From 305fc6d5a9315f27bca5aa0ba775becf9a44245d Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Thu, 6 Aug 2020 11:18:18 -0500 Subject: [PATCH 01/10] TOB-018 remediation --- .../agent/cert_with_name_end_to_end_test.go | 2 +- .../cert_with_no_name_end_to_end_test.go | 2 +- command/agent/jwt_end_to_end_test.go | 2 +- command/agent/sink/sink.go | 2 +- helper/dhutil/dhutil.go | 41 ++++++++++++++++--- 5 files changed, 39 insertions(+), 10 deletions(-) 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..4fbe9efa28b6 100644 --- a/command/agent/cert_with_name_end_to_end_test.go +++ b/command/agent/cert_with_name_end_to_end_test.go @@ -186,7 +186,7 @@ func testCertWithNameEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, resp.Curve25519PublicKey) + aesKey, err := dhutil.GenerateSharedKey(pri, pub, resp.Curve25519PublicKey) if err != nil { t.Fatal(err) } 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..c5347c4afb31 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.GenerateSharedKey(pri, pub, resp.Curve25519PublicKey) if err != nil { t.Fatal(err) } diff --git a/command/agent/jwt_end_to_end_test.go b/command/agent/jwt_end_to_end_test.go index f07a3f98442d..807ee3a5cfc7 100644 --- a/command/agent/jwt_end_to_end_test.go +++ b/command/agent/jwt_end_to_end_test.go @@ -231,7 +231,7 @@ func testJWTEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, resp.Curve25519PublicKey) + aesKey, err := dhutil.GenerateSharedKey(pri, 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..a1aa8431cc0e 100644 --- a/command/agent/sink/sink.go +++ b/command/agent/sink/sink.go @@ -205,7 +205,7 @@ func (s *SinkConfig) encryptToken(token string) (string, error) { resp.Curve25519PublicKey = s.cachedPubKey } - aesKey, err = dhutil.GenerateSharedKey(s.cachedPriKey, s.cachedRemotePubKey) + aesKey, err = dhutil.GenerateSharedKey(s.cachedPriKey, s.cachedPubKey, s.cachedRemotePubKey) 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..76cd85ea2bbf 100644 --- a/helper/dhutil/dhutil.go +++ b/helper/dhutil/dhutil.go @@ -1,11 +1,14 @@ package dhutil import ( + "bytes" "crypto/aes" "crypto/cipher" "crypto/rand" + "crypto/sha256" "errors" "fmt" + "golang.org/x/crypto/hkdf" "io" "golang.org/x/crypto/curve25519" @@ -36,7 +39,7 @@ func GeneratePublicPrivateKey() ([]byte, []byte, error) { // generateSharedKey uses the private key and the other party's public key to // generate the shared secret. -func GenerateSharedKey(ourPrivate, theirPublic []byte) ([]byte, error) { +func GenerateSharedKey(ourPrivate, ourPublic, theirPublic []byte) ([]byte, error) { if len(ourPrivate) != 32 { return nil, fmt.Errorf("invalid private key length: %d", len(ourPrivate)) } @@ -44,13 +47,39 @@ 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) + secret, err := curve25519.X25519(ourPrivate, theirPublic) + if err != nil { + return nil, err + } + + // Derive the final key from the HKDF of the secret and public keys. + + //These must be consistently ordered we're on "our" side of key negotiation or the other. + 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 + } - curve25519.ScalarMult(&secret, &scalar, &pub) + kio := hkdf.New(sha256.New, secret, pub1, pub2) - return secret[:], nil + var key [32]byte + n, err := io.ReadFull(kio, key[:]) + if n != 32 { + return nil, errors.New("short read from hkdf") + } + if err != nil { + // Don't return the key along with the error to prevent misuse + return nil, err + } + return key[:], nil } // Use AES256-GCM to encrypt some plaintext with a provided key. The returned values are From fcd4562d46d88ea68fa5aeca8ed6dfd7546a6019 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Thu, 6 Aug 2020 11:44:00 -0500 Subject: [PATCH 02/10] Make key derivation an optional config flag, off by default, for backwards compatibility --- command/agent.go | 1 + command/agent/cert_with_name_end_to_end_test.go | 6 +++++- command/agent/cert_with_no_name_end_to_end_test.go | 6 +++++- command/agent/config/config.go | 4 ++++ command/agent/config/config_test.go | 2 ++ command/agent/jwt_end_to_end_test.go | 6 +++++- command/agent/sink/sink.go | 12 +++++++++++- helper/dhutil/dhutil.go | 10 +++++----- website/pages/docs/agent/autoauth/index.mdx | 5 +++++ 9 files changed, 43 insertions(+), 9 deletions(-) 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 4fbe9efa28b6..af9e3cb0ae4f 100644 --- a/command/agent/cert_with_name_end_to_end_test.go +++ b/command/agent/cert_with_name_end_to_end_test.go @@ -186,7 +186,11 @@ func testCertWithNameEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, pub, 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/cert_with_no_name_end_to_end_test.go b/command/agent/cert_with_no_name_end_to_end_test.go index c5347c4afb31..9f14102acf2f 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,11 @@ func testCertWithNoNAmeEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, pub, 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/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..99106854d798 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", }, @@ -531,6 +532,7 @@ func TestLoadConfigFile_Template(t *testing.T) { DHType: "curve25519", DHPath: "/tmp/file-foo-dhpath", AAD: "foobar", + DeriveKey: true, Config: map[string]interface{}{ "path": "/tmp/file-foo", }, diff --git a/command/agent/jwt_end_to_end_test.go b/command/agent/jwt_end_to_end_test.go index 807ee3a5cfc7..a8a4e70a6476 100644 --- a/command/agent/jwt_end_to_end_test.go +++ b/command/agent/jwt_end_to_end_test.go @@ -231,7 +231,11 @@ func testJWTEndToEnd(t *testing.T, ahWrapping bool) { continue } - aesKey, err := dhutil.GenerateSharedKey(pri, pub, 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 a1aa8431cc0e..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.cachedPubKey, 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 76cd85ea2bbf..2727aafe83fd 100644 --- a/helper/dhutil/dhutil.go +++ b/helper/dhutil/dhutil.go @@ -39,7 +39,7 @@ func GeneratePublicPrivateKey() ([]byte, []byte, error) { // generateSharedKey uses the private key and the other party's public key to // generate the shared secret. -func GenerateSharedKey(ourPrivate, ourPublic, 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)) } @@ -47,11 +47,11 @@ func GenerateSharedKey(ourPrivate, ourPublic, theirPublic []byte) ([]byte, error return nil, fmt.Errorf("invalid public key length: %d", len(theirPublic)) } - secret, err := curve25519.X25519(ourPrivate, theirPublic) - if err != nil { - return nil, err - } + 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. //These must be consistently ordered we're on "our" side of key negotiation or the other. 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. From 3d55d9b87ee69f65a2d697a549c884287478305d Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Thu, 6 Aug 2020 11:59:23 -0500 Subject: [PATCH 03/10] Fix unit tests --- command/agent/config/config_test.go | 1 - command/agent/config/test-fixtures/config-embedded-type.hcl | 1 + command/agent/config/test-fixtures/config.hcl | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index 99106854d798..a316a73eb916 100644 --- a/command/agent/config/config_test.go +++ b/command/agent/config/config_test.go @@ -532,7 +532,6 @@ func TestLoadConfigFile_Template(t *testing.T) { DHType: "curve25519", DHPath: "/tmp/file-foo-dhpath", AAD: "foobar", - DeriveKey: true, Config: map[string]interface{}{ "path": "/tmp/file-foo", }, 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" } From 735b9733e6241c256688add625ccec748771ba95 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Thu, 6 Aug 2020 15:32:51 -0500 Subject: [PATCH 04/10] Address some feedback --- helper/dhutil/dhutil.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helper/dhutil/dhutil.go b/helper/dhutil/dhutil.go index 2727aafe83fd..25dedb027bb7 100644 --- a/helper/dhutil/dhutil.go +++ b/helper/dhutil/dhutil.go @@ -37,7 +37,7 @@ 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 GenerateSharedSecret(ourPrivate, theirPublic []byte) ([]byte, error) { if len(ourPrivate) != 32 { @@ -72,13 +72,13 @@ func DeriveSharedKey(secret, ourPublic, theirPublic []byte) ([]byte, error) { var key [32]byte n, err := io.ReadFull(kio, key[:]) - if n != 32 { - return nil, errors.New("short read from hkdf") - } 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") + } return key[:], nil } From f54fda3d894f1aa746d25482c179541b687613dd Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Fri, 7 Aug 2020 10:11:09 -0500 Subject: [PATCH 05/10] Set config on unit test --- command/agent/cert_with_name_end_to_end_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 af9e3cb0ae4f..f9668a0a168a 100644 --- a/command/agent/cert_with_name_end_to_end_test.go +++ b/command/agent/cert_with_name_end_to_end_test.go @@ -3,6 +3,10 @@ package agent import ( "context" "encoding/pem" + "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" "io/ioutil" "os" "testing" @@ -10,7 +14,6 @@ import ( hclog "github.com/hashicorp/go-hclog" - "github.com/hashicorp/vault/api" 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, }, @@ -197,7 +198,6 @@ func testCertWithNameEndToEnd(t *testing.T, ahWrapping bool) { 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)) From ee78637db525bc39e174f723ff1adc7fb26d3a88 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Fri, 7 Aug 2020 10:56:04 -0500 Subject: [PATCH 06/10] Fix another test failure --- command/agent/cert_with_no_name_end_to_end_test.go | 1 + 1 file changed, 1 insertion(+) 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 9f14102acf2f..5f65f53e1de8 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 @@ -134,6 +134,7 @@ func testCertWithNoNAmeEndToEnd(t *testing.T, ahWrapping bool) { AAD: "foobar", DHType: "curve25519", DHPath: dhpath, + DeriveKey: true, Config: map[string]interface{}{ "path": out, }, From 5b0be224c453c2bd02f5bef213557a0cb3d5895b Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Fri, 7 Aug 2020 11:04:42 -0500 Subject: [PATCH 07/10] One more conf fail --- command/agent/jwt_end_to_end_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command/agent/jwt_end_to_end_test.go b/command/agent/jwt_end_to_end_test.go index a8a4e70a6476..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, }, From 8577fc019dc8328915ca51a5285e99cb84eb6cf5 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Fri, 7 Aug 2020 11:22:03 -0500 Subject: [PATCH 08/10] Switch one of the test cases to not use a derive dkey --- command/agent/cert_with_no_name_end_to_end_test.go | 7 +------ helper/dhutil/dhutil_test.go | 1 + 2 files changed, 2 insertions(+), 6 deletions(-) create mode 100644 helper/dhutil/dhutil_test.go 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 5f65f53e1de8..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 @@ -134,7 +134,6 @@ func testCertWithNoNAmeEndToEnd(t *testing.T, ahWrapping bool) { AAD: "foobar", DHType: "curve25519", DHPath: dhpath, - DeriveKey: true, Config: map[string]interface{}{ "path": out, }, @@ -184,11 +183,7 @@ func testCertWithNoNAmeEndToEnd(t *testing.T, ahWrapping bool) { continue } - shared, err := dhutil.GenerateSharedSecret(pri, resp.Curve25519PublicKey) - if err != nil { - t.Fatal(err) - } - aesKey, err := dhutil.DeriveSharedKey(shared, pub, resp.Curve25519PublicKey) + aesKey, err := dhutil.GenerateSharedSecret(pri, resp.Curve25519PublicKey) if err != nil { t.Fatal(err) } 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 From 30b5238faccc7ba5acf251d075676a8275268227 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Mon, 10 Aug 2020 10:47:16 -0500 Subject: [PATCH 09/10] wip --- helper/dhutil/dhutil.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/helper/dhutil/dhutil.go b/helper/dhutil/dhutil.go index 25dedb027bb7..a1588a0843e4 100644 --- a/helper/dhutil/dhutil.go +++ b/helper/dhutil/dhutil.go @@ -6,6 +6,7 @@ import ( "crypto/cipher" "crypto/rand" "crypto/sha256" + "encoding/hex" "errors" "fmt" "golang.org/x/crypto/hkdf" @@ -79,6 +80,8 @@ func DeriveSharedKey(secret, ourPublic, theirPublic []byte) ([]byte, error) { if n != 32 { return nil, errors.New("short read from hkdf") } + fmt.Printf("Key: %s\n", hex.EncodeToString(key[:])) + return key[:], nil } From 4927f40441afe2a0801e18ec000981132c809925 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Mon, 17 Aug 2020 11:10:43 -0500 Subject: [PATCH 10/10] comments --- command/agent/cert_with_name_end_to_end_test.go | 8 ++++---- helper/dhutil/dhutil.go | 11 ++++++++++- 2 files changed, 14 insertions(+), 5 deletions(-) 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 f9668a0a168a..fea2bce4a6e2 100644 --- a/command/agent/cert_with_name_end_to_end_test.go +++ b/command/agent/cert_with_name_end_to_end_test.go @@ -3,10 +3,6 @@ package agent import ( "context" "encoding/pem" - "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" "io/ioutil" "os" "testing" @@ -14,6 +10,10 @@ 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" diff --git a/helper/dhutil/dhutil.go b/helper/dhutil/dhutil.go index a1588a0843e4..23257cf91b9a 100644 --- a/helper/dhutil/dhutil.go +++ b/helper/dhutil/dhutil.go @@ -55,7 +55,16 @@ func GenerateSharedSecret(ourPrivate, theirPublic []byte) ([]byte, error) { func DeriveSharedKey(secret, ourPublic, theirPublic []byte) ([]byte, error) { // Derive the final key from the HKDF of the secret and public keys. - //These must be consistently ordered we're on "our" side of key negotiation or the other. +/* + 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) {