Vault: Transit engine ecdsa-secp256k1 support

[john-osullivan]

I'm trying to use Vault to sign ethereum transactions and thought that the Vault engine would be a perfect fit. Unfortunately, ethereum's ECDSA uses the secp256k1 curve, rather than p256. My workaround is to roll my own plug-in which mostly behaves like Transit except it uses the key functions provided by go-ethereum/crypto/secp256k1. Given that the secp256k1 curve is also used by Bitcoin, there are a wide range of cryptographic applications which would benefit from having that curve built in to the engine.

Is there any reason this wouldn't be feasible? I'm going to be digging into this myself, would appreciate some guidance on any issues I'll run into in the conversion process. Thank you!

[jefferai]

Mostly the issue is that there isn't a pure-Go implementation (that I know of). Vault is distributed as a static binary, which means linking to C libs isn't feasible.

[john-osullivan]

Thanks for the info!

I imagine that supporting it isn't on the roadmap, then? Given that somebody first needs to open source that pure-Go secp256k1 implementation?

[jefferai]

It's not currently on the roadmap, no, but it could be added pretty easily if a library was available (and properly reviewed).

[jefferai]

As an aside, Vault plugins can be dyamically linked since they're separate processes, so someone could write a plugin to do secp256k1 operations. It won't be as fast as something natively compiled in, but it'd work.

[meirish]

@john-osullivan You might check out https://github.com/immutability-io/vault-ethereum which does support signing (via go-ethereum from the look of it).

[john-osullivan]

Thanks for the clarification about the separate Vault & plugin processes, @jefferai . Most of my experience is from web dev, still figuring things out this close to the metal.

And thanks for pointing me toward that lib, @meirish ! They seemed to store JSON-serialized keystores in Vault, just gotta make a clean driver for geth to use it directly.

[john-osullivan]

Heads up after doing a lot more digging, it looks like the btcd project built a pure-Go implementation of secp-256k1! It uses ISC, so it could be incorporated into for-profit code without issue. It's got test cases and has been in use since 2013, although I don't know if it's gotten an official independent review.

If this were in there, it seems like Vault would become a turn-key solution for secure crypto wallets.

[jefferai]

I took a look to see if I could slip this in for 0.10.2 but there are too many open questions so it'll have to wait until 0.10.3.

A few:

• Bitcoin uses double hashing, which we don't currently support as a hash algorithm. We could accept pre-hashed input only for this key type, or we could introduce a double hash algorithm (which just feels rather special-case but is otherwise not really an issue).

• Output format: Normally we'd take the raw signature bytes and base64 them to return to the user. The package you linked to has a serialization function which takes the raw output and puts it into DER format, but:

  • "does not include the appended hash type used in Bitcoin signature scripts" -- which suggests that maybe doublehashing isn't a requirement?
  • Doesn't explain why you would or would not want to serialize in DER format anyways
  • Uses some hacky code to work around encoding/asn1 bugs that may or may not still exist

so it's unclear to me what the right thing to do here is, and I'm leaning towards "return the raw bytes and let the user format as they wish"

[john-osullivan]

Thanks for taking a look, @jefferai ! Very excited to hear it's on the roadmap -- should make it even easier to keep all my keys in Vault going forward.

[smolaon]

Is this issue still on roadmap?

[vnbx]

Is this still on the roadmap?

[jefferai]

Same status as before. Open questions, uncertain crypto implementation.

[eximchain137]

I took a look to see if I could slip this in for 0.10.2 but there are too many open questions so it'll have to wait until 0.10.3.

A few:

• Bitcoin uses double hashing, which we don't currently support as a hash algorithm. We could accept pre-hashed input only for this key type, or we could introduce a double hash algorithm (which just feels rather special-case but is otherwise not really an issue).

You should accept pre-hashed inputs only, this leaves the implementation neutral for other chains that don't require this.

• Output format: Normally we'd take the raw signature bytes and base64 them to return to the user. The package you linked to has a serialization function which takes the raw output and puts it into DER format, but:

  • "does not include the appended hash type used in Bitcoin signature scripts" -- which suggests that maybe doublehashing isn't a requirement?
  • Doesn't explain why you would or would not want to serialize in DER format anyways
  • Uses some hacky code to work around encoding/asn1 bugs that may or may not still exist

so it's unclear to me what the right thing to do here is, and I'm leaning towards "return the raw bytes and let the user format as they wish"

I like your idea here, raw bytes is definitely the way to go, I would stay away from any implementations of particular clients, no offense to previous suggestions. the only function that need to be implemented in the plugin is: func Sign(rand io.Reader, priv *PrivateKey, hash []byte) (r, s *big.Int, err error) from the ecdsa library.
https://github.com/golang/go/blob/master/src/crypto/ecdsa/ecdsa.go#L156

so that clients just have to do a one line swap to use vault instead for example here

the referenced line would go from:

sig, err := crypto.Sign(h[:], prv)

to:

//sign using the new  secp256-k1 signer and public key/endpoint
req, err := http.NewRequest("POST", "http://127.0.0.1:8200/v1/transit/sign/my-key/secp256-k1", payload)
req.Header.Set("X-Vault-Token", "...")
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
sig, err := http.DefaultClient.Do(req) //maybe some type conversion here

[jefferai]

Those points all look fine but there's still the issue of a package. https://github.com/btcsuite/btcd/tree/master/btcec is the only pure-Go package I could find. They say it's under an ISC license, but the original work is under an unknown license -- possibly BSD, possibly not. I wouldn't be able to use it without first getting it cleared, and that's outside of the question of whether it can be trusted :-)

[eximchain137]

Is there any reason to not use the c-go secp256k1 wrappers? Just wondering if it's more of from a perspective of how you guys maintain the code base (wanting to keep everything for the native transit engine binaries in pure go makes sense). Or if there is some attack surface to building the secp256k1 signer as a separate process plugin and using the go package that wraps the bitcoin secp256k1 C library?

[jefferai]

In an external plugin someone could certainly wrap it. It just wouldn't be compiled in Vault proper, similar to our Oracle plugin.

[Legogris]

Has any work been done on this? Since this issue was opened, ecdsa-secp256k1's position as defacto standard in blockchain crypto has been solidified (Bitcoin, Ethereum, EOS, Tezos, etc etc).

To provide some context of why this is very relevant for vault, the rise of PoS and automated transactions to smart contracts or oracles makes Vault (if the schemes would be supported) a great fit for signing transactions for those who don't rely on HSMs.

From my perspective, chain-specific encoding, formatting, and double-hashing and so on shouldn't be a concern of Vault - just input and output raw bytes as with other algorithms and leave the rest up to users.

As for ready-made libraries, apart from the already mentioned implementations and the wrapper around the C-library, there is now also ecies/go (MIT license) and decred/dcrd/dcrec/secp25k1 (ISC license).

[eximchain137]

https://github.com/Eximchain/vault-guardian-plugin
https://github.com/Eximchain/terraform-aws-vault-guardian
https://github.com/Eximchain/vault-guardian-js-client

[kartikv11]

+1 to the ask of this feature as part of Transit-engine

[Konubinix]

At my company, we would like to use vault to sign using ecdsa-secp256k1.

As of today, would you recommand us to contribute to the effort of adding it to transit (and submitting a pull request to vault) or forking https://github.com/immutability-io/vault-ethereum to make it focus on the signing part (removing the part playing with ethereum) ?