diff --git a/command/agent.go b/command/agent.go index 793f26a31694..018d01e41243 100644 --- a/command/agent.go +++ b/command/agent.go @@ -349,6 +349,7 @@ func (c *AgentCommand) Run(args []string) int { Client: client, WrapTTL: sc.WrapTTL, DHType: sc.DHType, + DeriveKey: sc.DeriveKey, DHPath: sc.DHPath, AAD: sc.AAD, } diff --git a/command/agent/cert_with_name_end_to_end_test.go b/command/agent/cert_with_name_end_to_end_test.go index 0500dc8997c9..fea2bce4a6e2 100644 --- a/command/agent/cert_with_name_end_to_end_test.go +++ b/command/agent/cert_with_name_end_to_end_test.go @@ -11,6 +11,9 @@ import ( hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/sdk/helper/jsonutil" + "github.com/hashicorp/vault/sdk/helper/logging" + "github.com/hashicorp/vault/sdk/logical" vaultcert "github.com/hashicorp/vault/builtin/credential/cert" "github.com/hashicorp/vault/command/agent/auth" agentcert "github.com/hashicorp/vault/command/agent/auth/cert" @@ -18,9 +21,6 @@ import ( "github.com/hashicorp/vault/command/agent/sink/file" "github.com/hashicorp/vault/helper/dhutil" vaulthttp "github.com/hashicorp/vault/http" - "github.com/hashicorp/vault/sdk/helper/jsonutil" - "github.com/hashicorp/vault/sdk/helper/logging" - "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault" ) @@ -137,6 +137,7 @@ func testCertWithNameEndToEnd(t *testing.T, ahWrapping bool) { AAD: "foobar", DHType: "curve25519", DHPath: dhpath, + DeriveKey: true, Config: map[string]interface{}{ "path": out, }, @@ -186,14 +187,17 @@ func testCertWithNameEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, resp.Curve25519PublicKey) + shared, err := dhutil.GenerateSharedSecret(pri, resp.Curve25519PublicKey) + if err != nil { + t.Fatal(err) + } + aesKey, err := dhutil.DeriveSharedKey(shared, pub, resp.Curve25519PublicKey) if err != nil { t.Fatal(err) } if len(aesKey) == 0 { t.Fatal("got empty aes key") } - val, err = dhutil.DecryptAES(aesKey, resp.EncryptedPayload, resp.Nonce, []byte("foobar")) if err != nil { t.Fatalf("error: %v\nresp: %v", err, string(val)) diff --git a/command/agent/cert_with_no_name_end_to_end_test.go b/command/agent/cert_with_no_name_end_to_end_test.go index d6394de37e85..da62ba5d3faa 100644 --- a/command/agent/cert_with_no_name_end_to_end_test.go +++ b/command/agent/cert_with_no_name_end_to_end_test.go @@ -183,7 +183,7 @@ func testCertWithNoNAmeEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, resp.Curve25519PublicKey) + aesKey, err := dhutil.GenerateSharedSecret(pri, resp.Curve25519PublicKey) if err != nil { t.Fatal(err) } diff --git a/command/agent/config/config.go b/command/agent/config/config.go index 2036eb8067e5..489cbb746fb4 100644 --- a/command/agent/config/config.go +++ b/command/agent/config/config.go @@ -75,6 +75,7 @@ type Sink struct { WrapTTLRaw interface{} `hcl:"wrap_ttl"` WrapTTL time.Duration `hcl:"-"` DHType string `hcl:"dh_type"` + DeriveKey bool `hcl:"derive_key"` DHPath string `hcl:"dh_path"` AAD string `hcl:"aad"` AADEnvVar string `hcl:"aad_env_var"` @@ -395,6 +396,9 @@ func parseSinks(result *Config, list *ast.ObjectList) error { if s.AAD != "" { return multierror.Prefix(errors.New("specifying AAD data without 'dh_type' does not make sense"), fmt.Sprintf("sink.%s", s.Type)) } + if s.DeriveKey { + return multierror.Prefix(errors.New("specifying 'derive_key' data without 'dh_type' does not make sense"), fmt.Sprintf("sink.%s", s.Type)) + } case s.DHPath != "" && s.DHType != "": default: return multierror.Prefix(errors.New("'dh_type' and 'dh_path' must be specified together"), fmt.Sprintf("sink.%s", s.Type)) diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index b5c1d2e0c728..a316a73eb916 100644 --- a/command/agent/config/config_test.go +++ b/command/agent/config/config_test.go @@ -143,6 +143,7 @@ func TestLoadConfigFile(t *testing.T) { DHType: "curve25519", DHPath: "/tmp/file-foo-dhpath2", AAD: "aad", + DeriveKey: true, Config: map[string]interface{}{ "path": "/tmp/file-bar", }, diff --git a/command/agent/config/test-fixtures/config-embedded-type.hcl b/command/agent/config/test-fixtures/config-embedded-type.hcl index f17336078a65..919bfd907757 100644 --- a/command/agent/config/test-fixtures/config-embedded-type.hcl +++ b/command/agent/config/test-fixtures/config-embedded-type.hcl @@ -23,6 +23,7 @@ auto_auth { aad_env_var = "TEST_AAD_ENV" dh_type = "curve25519" dh_path = "/tmp/file-foo-dhpath2" + derive_key = true config = { path = "/tmp/file-bar" } diff --git a/command/agent/config/test-fixtures/config.hcl b/command/agent/config/test-fixtures/config.hcl index 096190d04d84..b02170736a8e 100644 --- a/command/agent/config/test-fixtures/config.hcl +++ b/command/agent/config/test-fixtures/config.hcl @@ -25,6 +25,7 @@ auto_auth { aad_env_var = "TEST_AAD_ENV" dh_type = "curve25519" dh_path = "/tmp/file-foo-dhpath2" + derive_key = true config = { path = "/tmp/file-bar" } diff --git a/command/agent/jwt_end_to_end_test.go b/command/agent/jwt_end_to_end_test.go index f07a3f98442d..0cda243725cb 100644 --- a/command/agent/jwt_end_to_end_test.go +++ b/command/agent/jwt_end_to_end_test.go @@ -158,6 +158,7 @@ func testJWTEndToEnd(t *testing.T, ahWrapping bool) { AAD: "foobar", DHType: "curve25519", DHPath: dhpath, + DeriveKey: true, Config: map[string]interface{}{ "path": out, }, @@ -231,7 +232,11 @@ func testJWTEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, resp.Curve25519PublicKey) + shared, err := dhutil.GenerateSharedSecret(pri, resp.Curve25519PublicKey) + if err != nil { + t.Fatal(err) + } + aesKey, err := dhutil.DeriveSharedKey(shared, pub, resp.Curve25519PublicKey) if err != nil { t.Fatal(err) } diff --git a/command/agent/sink/sink.go b/command/agent/sink/sink.go index acc93b773b94..f8eb921b387e 100644 --- a/command/agent/sink/sink.go +++ b/command/agent/sink/sink.go @@ -32,6 +32,7 @@ type SinkConfig struct { WrapTTL time.Duration DHType string DHPath string + DeriveKey bool AAD string cachedRemotePubKey []byte cachedPubKey []byte @@ -205,7 +206,16 @@ func (s *SinkConfig) encryptToken(token string) (string, error) { resp.Curve25519PublicKey = s.cachedPubKey } - aesKey, err = dhutil.GenerateSharedKey(s.cachedPriKey, s.cachedRemotePubKey) + secret, err := dhutil.GenerateSharedSecret(s.cachedPriKey, s.cachedRemotePubKey) + if err != nil { + return "", errwrap.Wrapf("error calculating shared key: {{err}}", err) + } + if s.DeriveKey { + aesKey, err = dhutil.DeriveSharedKey(secret, s.cachedPubKey, s.cachedRemotePubKey) + } else { + aesKey = secret + } + if err != nil { return "", errwrap.Wrapf("error deriving shared key: {{err}}", err) } diff --git a/helper/dhutil/dhutil.go b/helper/dhutil/dhutil.go index 86e23298e037..23257cf91b9a 100644 --- a/helper/dhutil/dhutil.go +++ b/helper/dhutil/dhutil.go @@ -1,11 +1,15 @@ package dhutil import ( + "bytes" "crypto/aes" "crypto/cipher" "crypto/rand" + "crypto/sha256" + "encoding/hex" "errors" "fmt" + "golang.org/x/crypto/hkdf" "io" "golang.org/x/crypto/curve25519" @@ -34,9 +38,9 @@ func GeneratePublicPrivateKey() ([]byte, []byte, error) { return public[:], scalar[:], nil } -// generateSharedKey uses the private key and the other party's public key to +// GenerateSharedSecret uses the private key and the other party's public key to // generate the shared secret. -func GenerateSharedKey(ourPrivate, theirPublic []byte) ([]byte, error) { +func GenerateSharedSecret(ourPrivate, theirPublic []byte) ([]byte, error) { if len(ourPrivate) != 32 { return nil, fmt.Errorf("invalid private key length: %d", len(ourPrivate)) } @@ -44,13 +48,50 @@ func GenerateSharedKey(ourPrivate, theirPublic []byte) ([]byte, error) { return nil, fmt.Errorf("invalid public key length: %d", len(theirPublic)) } - var scalar, pub, secret [32]byte - copy(scalar[:], ourPrivate) - copy(pub[:], theirPublic) + return curve25519.X25519(ourPrivate, theirPublic) +} + +// DeriveSharedKey uses HKDF to derive a key from a shared secret and public keys +func DeriveSharedKey(secret, ourPublic, theirPublic []byte) ([]byte, error) { + // Derive the final key from the HKDF of the secret and public keys. + +/* + Internally, HKDF hashes the secret and two public keys. If Alice and Bob are doing DH key exchange, Alice calculates: + + HKDF(secret, A, B) since ourPublic is A. + + Bob calculates HKDF(secret, B, A), since Bob's ours is B. That produces a different value. Now we only care + that both public keys participate in the derivation, so simply sorting them so they are in a consistent + numerical order (either one would do) arrives at an agreed value. +*/ + + var pub1 []byte + var pub2 []byte + switch bytes.Compare(ourPublic, theirPublic) { + case 0: + return nil, errors.New("same public key supplied for both participants") + case -1: + pub1 = ourPublic + pub2 = theirPublic + case 1: + pub1 = theirPublic + pub2 = ourPublic + } + + kio := hkdf.New(sha256.New, secret, pub1, pub2) - curve25519.ScalarMult(&secret, &scalar, &pub) + var key [32]byte + n, err := io.ReadFull(kio, key[:]) + if err != nil { + // Don't return the key along with the error to prevent misuse + return nil, err + } + if n != 32 { + return nil, errors.New("short read from hkdf") + } + fmt.Printf("Key: %s\n", hex.EncodeToString(key[:])) - return secret[:], nil + return key[:], nil } // Use AES256-GCM to encrypt some plaintext with a provided key. The returned values are diff --git a/helper/dhutil/dhutil_test.go b/helper/dhutil/dhutil_test.go new file mode 100644 index 000000000000..46e90196d15e --- /dev/null +++ b/helper/dhutil/dhutil_test.go @@ -0,0 +1 @@ +package dhutil diff --git a/website/pages/docs/agent/autoauth/index.mdx b/website/pages/docs/agent/autoauth/index.mdx index eab9fc24e903..2f84c3976687 100644 --- a/website/pages/docs/agent/autoauth/index.mdx +++ b/website/pages/docs/agent/autoauth/index.mdx @@ -155,6 +155,11 @@ These configuration values are common to all Sinks: agent should read the client's initial parameters (e.g. curve25519 public key). +- `derive_key` `(bool: false)` - If specified, the final encryption key is + calculated by using HKDF-SHA256 to derive a key from the calculated shared + secret and the two public keys for enhanced security. This is recommended + if backward compatibility isn't a concern. + - `aad` `(string: optional)` - If specified, additional authenticated data to use with the AES-GCM encryption of the token. Can be any string, including serialized data.