From 6c67032722d6e31b957517f411e3694b626bb93f Mon Sep 17 00:00:00 2001 From: James Renken Date: Thu, 7 Nov 2024 22:15:53 -0800 Subject: [PATCH 01/10] WFE/nonce: Add NonceHMACKey field --- cmd/boulder-wfe2/main.go | 22 +++++++++++-- cmd/config.go | 2 +- cmd/nonce-service/main.go | 36 +++++++++++++-------- test/config-next/nonce-a.json | 4 +-- test/config-next/nonce-b.json | 4 +-- test/config-next/wfe2.json | 4 +-- test/config/nonce-a.json | 1 - test/config/nonce-b.json | 1 - test/integration/nonce_test.go | 11 +++++-- test/integration/testdata/nonce-client.json | 4 +-- test/secrets/nonce_prefix_key | 2 +- 11 files changed, 60 insertions(+), 31 deletions(-) diff --git a/cmd/boulder-wfe2/main.go b/cmd/boulder-wfe2/main.go index 61698d16cfe..d395e67c8ac 100644 --- a/cmd/boulder-wfe2/main.go +++ b/cmd/boulder-wfe2/main.go @@ -71,13 +71,23 @@ type Config struct { // local and remote nonce-service instances. RedeemNonceService *cmd.GRPCClientConfig `validate:"required"` + // NonceHMACKey is a path to a file containing an HMAC key which is a + // secret used for deriving the prefix of each nonce instance. It should + // contain 256 bits (32 bytes) of random data to be suitable as an + // HMAC-SHA256 key (e.g. the output of `openssl rand -hex 32`). In a + // multi-DC deployment this value should be the same across all + // boulder-wfe and nonce-service instances. + NonceHMACKey cmd.HMACKeyConfig `validate:"-"` + // NoncePrefixKey is a secret used for deriving the prefix of each nonce // instance. It should contain 256 bits of random data to be suitable as // an HMAC-SHA256 key (e.g. the output of `openssl rand -hex 32`). In a // multi-DC deployment this value should be the same across all // boulder-wfe and nonce-service instances. // - // TODO(#7632) Update this to use the new HMACKeyConfig. + // TODO(#7632): Remove this. + // + // Deprecated: Use NonceHMACKey instead. NoncePrefixKey cmd.PasswordConfig `validate:"-"` // Chains is a list of lists of certificate filenames. Each inner list is @@ -295,9 +305,15 @@ func main() { } var noncePrefixKey string - if c.WFE.NoncePrefixKey.PasswordFile != "" { + if c.WFE.NonceHMACKey.KeyFile != "" { + keyByte, err := c.WFE.NonceHMACKey.Load() + cmd.FailOnError(err, "Failed to load nonceHMACKey file") + noncePrefixKey = string(keyByte) + } else if c.WFE.NoncePrefixKey.PasswordFile != "" { noncePrefixKey, err = c.WFE.NoncePrefixKey.Pass() - cmd.FailOnError(err, "Failed to load noncePrefixKey") + cmd.FailOnError(err, "Failed to load noncePrefixKey file") + } else { + cmd.Fail("NonceHMACKey KeyFile or NoncePrefixKey PasswordFile must be set") } getNonceConn, err := bgrpc.ClientSetup(c.WFE.GetNonceService, tlsConfig, stats, clk) diff --git a/cmd/config.go b/cmd/config.go index 3264ea1cce2..9aaa6836f52 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -572,7 +572,7 @@ func (hc *HMACKeyConfig) Load() ([]byte, error) { if len(trimmed) != 32 { return nil, fmt.Errorf( - "validating unpauseHMACKey, length must be 32 alphanumeric characters, got %d", + "validating HMAC key, length must be 32 alphanumeric characters, got %d", len(trimmed), ) } diff --git a/cmd/nonce-service/main.go b/cmd/nonce-service/main.go index af90de5ac33..75c9cb94b06 100644 --- a/cmd/nonce-service/main.go +++ b/cmd/nonce-service/main.go @@ -19,14 +19,13 @@ type Config struct { MaxUsed int - // UseDerivablePrefix indicates whether to use a nonce prefix derived - // from the gRPC listening address. If this is false, the nonce prefix - // will be the value of the NoncePrefix field. If this is true, the - // NoncePrefixKey field is required. - // TODO(#6610): Remove this. - // - // Deprecated: this value is ignored, and treated as though it is always true. - UseDerivablePrefix bool `validate:"-"` + // NonceHMACKey is a path to a file containing an HMAC key which is a + // secret used for deriving the prefix of each nonce instance. It should + // contain 256 bits (32 bytes) of random data to be suitable as an + // HMAC-SHA256 key (e.g. the output of `openssl rand -hex 32`). In a + // multi-DC deployment this value should be the same across all + // boulder-wfe and nonce-service instances. + NonceHMACKey cmd.HMACKeyConfig `validate:"required_without_all=NoncePrefixKey,structonly"` // NoncePrefixKey is a secret used for deriving the prefix of each nonce // instance. It should contain 256 bits (32 bytes) of random data to be @@ -34,8 +33,11 @@ type Config struct { // 32`). In a multi-DC deployment this value should be the same across // all boulder-wfe and nonce-service instances. // - // TODO(#7632) Update this to use the new HMACKeyConfig. - NoncePrefixKey cmd.PasswordConfig `validate:"required"` + // TODO(#7632): Remove this and change `NonceHMACKey`'s validation to + // just `required.` + // + // Deprecated: Use NonceHMACKey instead. + NoncePrefixKey cmd.PasswordConfig `validate:"required_without_all=NonceHMACKey,structonly"` Syslog cmd.SyslogConfig OpenTelemetry cmd.OpenTelemetryConfig @@ -84,12 +86,18 @@ func main() { c.NonceService.DebugAddr = *debugAddr } - if c.NonceService.NoncePrefixKey.PasswordFile == "" { - cmd.Fail("NoncePrefixKey PasswordFile must be set") + var key string + if c.NonceService.NonceHMACKey.KeyFile != "" { + keyByte, err := c.NonceService.NonceHMACKey.Load() + cmd.FailOnError(err, "Failed to load 'nonceHMACKey' file.") + key = string(keyByte) + } else if c.NonceService.NoncePrefixKey.PasswordFile != "" { + key, err = c.NonceService.NoncePrefixKey.Pass() + cmd.FailOnError(err, "Failed to load 'noncePrefixKey' file.") + } else { + cmd.Fail("NonceHMACKey KeyFile or NoncePrefixKey PasswordFile must be set") } - key, err := c.NonceService.NoncePrefixKey.Pass() - cmd.FailOnError(err, "Failed to load 'noncePrefixKey' file.") noncePrefix, err := derivePrefix(key, c.NonceService.GRPC.Address) cmd.FailOnError(err, "Failed to derive nonce prefix") diff --git a/test/config-next/nonce-a.json b/test/config-next/nonce-a.json index 75df81b6ed9..d14b44063f2 100644 --- a/test/config-next/nonce-a.json +++ b/test/config-next/nonce-a.json @@ -1,8 +1,8 @@ { "NonceService": { "maxUsed": 131072, - "noncePrefixKey": { - "passwordFile": "test/secrets/nonce_prefix_key" + "nonceHMACKey": { + "keyFile": "test/secrets/nonce_prefix_key" }, "syslog": { "stdoutLevel": 6, diff --git a/test/config-next/nonce-b.json b/test/config-next/nonce-b.json index 75df81b6ed9..d14b44063f2 100644 --- a/test/config-next/nonce-b.json +++ b/test/config-next/nonce-b.json @@ -1,8 +1,8 @@ { "NonceService": { "maxUsed": 131072, - "noncePrefixKey": { - "passwordFile": "test/secrets/nonce_prefix_key" + "nonceHMACKey": { + "keyFile": "test/secrets/nonce_prefix_key" }, "syslog": { "stdoutLevel": 6, diff --git a/test/config-next/wfe2.json b/test/config-next/wfe2.json index 1b697d99376..27ba88ac57b 100644 --- a/test/config-next/wfe2.json +++ b/test/config-next/wfe2.json @@ -68,8 +68,8 @@ "noWaitForReady": true, "hostOverride": "nonce.boulder" }, - "noncePrefixKey": { - "passwordFile": "test/secrets/nonce_prefix_key" + "nonceHMACKey": { + "keyFile": "test/secrets/nonce_prefix_key" }, "chains": [ [ diff --git a/test/config/nonce-a.json b/test/config/nonce-a.json index c2dd9765c85..6bbe2a0c53c 100644 --- a/test/config/nonce-a.json +++ b/test/config/nonce-a.json @@ -1,7 +1,6 @@ { "NonceService": { "maxUsed": 131072, - "useDerivablePrefix": true, "noncePrefixKey": { "passwordFile": "test/secrets/nonce_prefix_key" }, diff --git a/test/config/nonce-b.json b/test/config/nonce-b.json index c2dd9765c85..6bbe2a0c53c 100644 --- a/test/config/nonce-b.json +++ b/test/config/nonce-b.json @@ -1,7 +1,6 @@ { "NonceService": { "maxUsed": 131072, - "useDerivablePrefix": true, "noncePrefixKey": { "passwordFile": "test/secrets/nonce_prefix_key" }, diff --git a/test/integration/nonce_test.go b/test/integration/nonce_test.go index 5df9cb986a6..2aa49ae1438 100644 --- a/test/integration/nonce_test.go +++ b/test/integration/nonce_test.go @@ -24,6 +24,7 @@ type nonceBalancerTestConfig struct { GetNonceService *cmd.GRPCClientConfig RedeemNonceService *cmd.GRPCClientConfig NoncePrefixKey cmd.PasswordConfig + NonceHMACKey cmd.HMACKeyConfig } } @@ -41,8 +42,14 @@ func TestNonceBalancer_NoBackendMatchingPrefix(t *testing.T) { tlsConfig, err := c.NotWFE.TLS.Load(metrics.NoopRegisterer) test.AssertNotError(t, err, "Could not load TLS config") - rncKey, err := c.NotWFE.NoncePrefixKey.Pass() - test.AssertNotError(t, err, "Failed to load noncePrefixKey") + var rncKey string + rncKeyByte, err := c.NotWFE.NonceHMACKey.Load() + if err != nil { + rncKey, err = c.NotWFE.NoncePrefixKey.Pass() + test.AssertNotError(t, err, "Failed to load nonceHMACKey or noncePrefixKey") + } else { + rncKey = string(rncKeyByte) + } clk := clock.New() diff --git a/test/integration/testdata/nonce-client.json b/test/integration/testdata/nonce-client.json index 90e84706b02..a66077e2690 100644 --- a/test/integration/testdata/nonce-client.json +++ b/test/integration/testdata/nonce-client.json @@ -32,8 +32,8 @@ "noWaitForReady": true, "hostOverride": "nonce.boulder" }, - "noncePrefixKey": { - "passwordFile": "test/secrets/nonce_prefix_key" + "nonceHMACKey": { + "keyFile": "test/secrets/nonce_prefix_key" } } } diff --git a/test/secrets/nonce_prefix_key b/test/secrets/nonce_prefix_key index d65802423de..96bb4c2ed30 100644 --- a/test/secrets/nonce_prefix_key +++ b/test/secrets/nonce_prefix_key @@ -1 +1 @@ -3b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f +3b8c758dd85e113ea340ce0b3a99f389 From 9d3a34f8020ce3a01d15da4c01342eb851b00567 Mon Sep 17 00:00:00 2001 From: James Renken Date: Fri, 8 Nov 2024 14:33:49 -0800 Subject: [PATCH 02/10] Change nonce prefix key from string to byteslice --- cmd/boulder-wfe2/main.go | 8 ++++---- cmd/nonce-service/main.go | 10 +++++----- nonce/nonce.go | 4 ++-- test/integration/nonce_test.go | 9 ++++----- wfe2/wfe.go | 4 ++-- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/cmd/boulder-wfe2/main.go b/cmd/boulder-wfe2/main.go index d395e67c8ac..699ed0d787d 100644 --- a/cmd/boulder-wfe2/main.go +++ b/cmd/boulder-wfe2/main.go @@ -304,14 +304,14 @@ func main() { cmd.Fail("'getNonceService' must be configured") } - var noncePrefixKey string + var noncePrefixKey []byte if c.WFE.NonceHMACKey.KeyFile != "" { - keyByte, err := c.WFE.NonceHMACKey.Load() + noncePrefixKey, err = c.WFE.NonceHMACKey.Load() cmd.FailOnError(err, "Failed to load nonceHMACKey file") - noncePrefixKey = string(keyByte) } else if c.WFE.NoncePrefixKey.PasswordFile != "" { - noncePrefixKey, err = c.WFE.NoncePrefixKey.Pass() + keyString, err := c.WFE.NoncePrefixKey.Pass() cmd.FailOnError(err, "Failed to load noncePrefixKey file") + noncePrefixKey = []byte(keyString) } else { cmd.Fail("NonceHMACKey KeyFile or NoncePrefixKey PasswordFile must be set") } diff --git a/cmd/nonce-service/main.go b/cmd/nonce-service/main.go index 75c9cb94b06..1b7cf7f421c 100644 --- a/cmd/nonce-service/main.go +++ b/cmd/nonce-service/main.go @@ -44,7 +44,7 @@ type Config struct { } } -func derivePrefix(key string, grpcAddr string) (string, error) { +func derivePrefix(key []byte, grpcAddr string) (string, error) { host, port, err := net.SplitHostPort(grpcAddr) if err != nil { return "", fmt.Errorf("parsing gRPC listen address: %w", err) @@ -86,14 +86,14 @@ func main() { c.NonceService.DebugAddr = *debugAddr } - var key string + var key []byte if c.NonceService.NonceHMACKey.KeyFile != "" { - keyByte, err := c.NonceService.NonceHMACKey.Load() + key, err = c.NonceService.NonceHMACKey.Load() cmd.FailOnError(err, "Failed to load 'nonceHMACKey' file.") - key = string(keyByte) } else if c.NonceService.NoncePrefixKey.PasswordFile != "" { - key, err = c.NonceService.NoncePrefixKey.Pass() + keyString, err := c.NonceService.NoncePrefixKey.Pass() cmd.FailOnError(err, "Failed to load 'noncePrefixKey' file.") + key = []byte(keyString) } else { cmd.Fail("NonceHMACKey KeyFile or NoncePrefixKey PasswordFile must be set") } diff --git a/nonce/nonce.go b/nonce/nonce.go index 388ab62d050..dae37ba3e2a 100644 --- a/nonce/nonce.go +++ b/nonce/nonce.go @@ -55,8 +55,8 @@ type HMACKeyCtxKey struct{} // DerivePrefix derives a nonce prefix from the provided listening address and // key. The prefix is derived by take the first 8 characters of the base64url // encoded HMAC-SHA256 hash of the listening address using the provided key. -func DerivePrefix(grpcAddr, key string) string { - h := hmac.New(sha256.New, []byte(key)) +func DerivePrefix(grpcAddr string, key []byte) string { + h := hmac.New(sha256.New, key) h.Write([]byte(grpcAddr)) return base64.RawURLEncoding.EncodeToString(h.Sum(nil))[:PrefixLen] } diff --git a/test/integration/nonce_test.go b/test/integration/nonce_test.go index 2aa49ae1438..5beaea6190d 100644 --- a/test/integration/nonce_test.go +++ b/test/integration/nonce_test.go @@ -42,13 +42,12 @@ func TestNonceBalancer_NoBackendMatchingPrefix(t *testing.T) { tlsConfig, err := c.NotWFE.TLS.Load(metrics.NoopRegisterer) test.AssertNotError(t, err, "Could not load TLS config") - var rncKey string - rncKeyByte, err := c.NotWFE.NonceHMACKey.Load() + var rncKey []byte + rncKey, err = c.NotWFE.NonceHMACKey.Load() if err != nil { - rncKey, err = c.NotWFE.NoncePrefixKey.Pass() + rncKeyString, err := c.NotWFE.NoncePrefixKey.Pass() test.AssertNotError(t, err, "Failed to load nonceHMACKey or noncePrefixKey") - } else { - rncKey = string(rncKeyByte) + rncKey = []byte(rncKeyString) } clk := clock.New() diff --git a/wfe2/wfe.go b/wfe2/wfe.go index b4c60c084b5..751c1bf551e 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -109,7 +109,7 @@ type WebFrontEndImpl struct { rnc nonce.Redeemer // rncKey is the HMAC key used to derive the prefix of nonce backends used // for nonce redemption. - rncKey string + rncKey []byte accountGetter AccountGetter log blog.Logger clk clock.Clock @@ -194,7 +194,7 @@ func NewWebFrontEndImpl( sac sapb.StorageAuthorityReadOnlyClient, gnc nonce.Getter, rnc nonce.Redeemer, - rncKey string, + rncKey []byte, accountGetter AccountGetter, limiter *ratelimits.Limiter, txnBuilder *ratelimits.TransactionBuilder, From 800e8f0a4132c894d5f84fafad1356daa548f0d0 Mon Sep 17 00:00:00 2001 From: James Renken Date: Fri, 8 Nov 2024 14:36:16 -0800 Subject: [PATCH 03/10] Fix in noncebalancer too --- grpc/noncebalancer/noncebalancer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/noncebalancer/noncebalancer.go b/grpc/noncebalancer/noncebalancer.go index cf4e566714a..83d73254b41 100644 --- a/grpc/noncebalancer/noncebalancer.go +++ b/grpc/noncebalancer/noncebalancer.go @@ -84,7 +84,7 @@ func (p *Picker) Pick(info balancer.PickInfo) (balancer.PickResult, error) { // This should never happen. return balancer.PickResult{}, errMissingHMACKeyCtxKey } - hmacKey, ok := hmacKeyVal.(string) + hmacKey, ok := hmacKeyVal.([]byte) if !ok { // This should never happen. return balancer.PickResult{}, errInvalidHMACKeyCtxKeyType From f0b270f3a2df5b86b107118444be2d7654b7bac9 Mon Sep 17 00:00:00 2001 From: James Renken Date: Fri, 8 Nov 2024 14:54:54 -0800 Subject: [PATCH 04/10] Also fix more tests --- grpc/noncebalancer/noncebalancer_test.go | 11 ++++++----- nonce/nonce_test.go | 2 +- wfe2/wfe_test.go | 5 +++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/grpc/noncebalancer/noncebalancer_test.go b/grpc/noncebalancer/noncebalancer_test.go index ce7a05649ed..5b6298ac37c 100644 --- a/grpc/noncebalancer/noncebalancer_test.go +++ b/grpc/noncebalancer/noncebalancer_test.go @@ -4,16 +4,17 @@ import ( "context" "testing" - "github.com/letsencrypt/boulder/nonce" - "github.com/letsencrypt/boulder/test" "google.golang.org/grpc/balancer" "google.golang.org/grpc/balancer/base" "google.golang.org/grpc/resolver" + + "github.com/letsencrypt/boulder/nonce" + "github.com/letsencrypt/boulder/test" ) func TestPickerPicksCorrectBackend(t *testing.T) { _, p, subConns := setupTest(false) - prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, "Kala namak") + prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, []byte("Kala namak")) testCtx := context.WithValue(context.Background(), nonce.PrefixCtxKey{}, "HNmOnt8w") testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, prefix) @@ -26,7 +27,7 @@ func TestPickerPicksCorrectBackend(t *testing.T) { func TestPickerMissingPrefixInCtx(t *testing.T) { _, p, subConns := setupTest(false) - prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, "Kala namak") + prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, []byte("Kala namak")) testCtx := context.WithValue(context.Background(), nonce.HMACKeyCtxKey{}, prefix) info := balancer.PickInfo{Ctx: testCtx} @@ -73,7 +74,7 @@ func TestPickerInvalidHMACKeyInCtx(t *testing.T) { func TestPickerNoMatchingSubConnAvailable(t *testing.T) { _, p, subConns := setupTest(false) - prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, "Kala namak") + prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, []byte("Kala namak")) testCtx := context.WithValue(context.Background(), nonce.PrefixCtxKey{}, "rUsTrUin") testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, prefix) diff --git a/nonce/nonce_test.go b/nonce/nonce_test.go index db515d2a32d..77f497ef986 100644 --- a/nonce/nonce_test.go +++ b/nonce/nonce_test.go @@ -147,6 +147,6 @@ func TestNoncePrefixValidation(t *testing.T) { } func TestDerivePrefix(t *testing.T) { - prefix := DerivePrefix("192.168.1.1:8080", "3b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f") + prefix := DerivePrefix("192.168.1.1:8080", []byte("3b8c758dd85e113ea340ce0b3a99f389")) test.AssertEquals(t, prefix, "P9qQaK4o") } diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go index e9b179cf282..f0ae8e8cdb7 100644 --- a/wfe2/wfe_test.go +++ b/wfe2/wfe_test.go @@ -397,7 +397,8 @@ func setupWFE(t *testing.T) (WebFrontEndImpl, clock.FakeClock, requestSigner) { log := blog.NewMock() // Use derived nonces. - noncePrefix := nonce.DerivePrefix("192.168.1.1:8080", "b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f") + rncKey := []byte("b8c758dd85e113ea340ce0b3a99f3894") + noncePrefix := nonce.DerivePrefix("192.168.1.1:8080", rncKey) nonceService, err := nonce.NewNonceService(metrics.NoopRegisterer, 100, noncePrefix) test.AssertNotError(t, err, "making nonceService") @@ -458,7 +459,7 @@ func setupWFE(t *testing.T) (WebFrontEndImpl, clock.FakeClock, requestSigner) { mockSA, gnc, rnc, - "rncKey", + rncKey, mockSA, limiter, txnBuilder, From a1450c3084c02baf066d2bf38ce3acbd0ecdd88e Mon Sep 17 00:00:00 2001 From: James Renken Date: Fri, 8 Nov 2024 15:12:54 -0800 Subject: [PATCH 05/10] Fix interaction with gRPC & update test nonce prefix --- grpc/noncebalancer/noncebalancer.go | 4 ++-- nonce/nonce_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/grpc/noncebalancer/noncebalancer.go b/grpc/noncebalancer/noncebalancer.go index 83d73254b41..d4841a11f81 100644 --- a/grpc/noncebalancer/noncebalancer.go +++ b/grpc/noncebalancer/noncebalancer.go @@ -84,7 +84,7 @@ func (p *Picker) Pick(info balancer.PickInfo) (balancer.PickResult, error) { // This should never happen. return balancer.PickResult{}, errMissingHMACKeyCtxKey } - hmacKey, ok := hmacKeyVal.([]byte) + hmacKey, ok := hmacKeyVal.(string) if !ok { // This should never happen. return balancer.PickResult{}, errInvalidHMACKeyCtxKeyType @@ -94,7 +94,7 @@ func (p *Picker) Pick(info balancer.PickInfo) (balancer.PickResult, error) { // First call to Pick with a new Picker. prefixToBackend := make(map[string]balancer.SubConn) for sc, scInfo := range p.backends { - scPrefix := nonce.DerivePrefix(scInfo.Address.Addr, hmacKey) + scPrefix := nonce.DerivePrefix(scInfo.Address.Addr, []byte(hmacKey)) prefixToBackend[scPrefix] = sc } p.prefixToBackend = prefixToBackend diff --git a/nonce/nonce_test.go b/nonce/nonce_test.go index 77f497ef986..c7100fe4308 100644 --- a/nonce/nonce_test.go +++ b/nonce/nonce_test.go @@ -148,5 +148,5 @@ func TestNoncePrefixValidation(t *testing.T) { func TestDerivePrefix(t *testing.T) { prefix := DerivePrefix("192.168.1.1:8080", []byte("3b8c758dd85e113ea340ce0b3a99f389")) - test.AssertEquals(t, prefix, "P9qQaK4o") + test.AssertEquals(t, prefix, "FxMmnuMq") } From cde1e2135aa856df972c9ae3ed537056009f0336 Mon Sep 17 00:00:00 2001 From: James Renken Date: Fri, 8 Nov 2024 16:47:11 -0800 Subject: [PATCH 06/10] Really fix noncebalancer, thanks jsha! --- grpc/noncebalancer/noncebalancer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/grpc/noncebalancer/noncebalancer.go b/grpc/noncebalancer/noncebalancer.go index d4841a11f81..90027e7f834 100644 --- a/grpc/noncebalancer/noncebalancer.go +++ b/grpc/noncebalancer/noncebalancer.go @@ -35,7 +35,7 @@ var ErrNoBackendsMatchPrefix = status.New(codes.Unavailable, "no backends match var errMissingPrefixCtxKey = errors.New("nonce.PrefixCtxKey value required in RPC context") var errMissingHMACKeyCtxKey = errors.New("nonce.HMACKeyCtxKey value required in RPC context") var errInvalidPrefixCtxKeyType = errors.New("nonce.PrefixCtxKey value in RPC context must be a string") -var errInvalidHMACKeyCtxKeyType = errors.New("nonce.HMACKeyCtxKey value in RPC context must be a string") +var errInvalidHMACKeyCtxKeyType = errors.New("nonce.HMACKeyCtxKey value in RPC context must be a byteslice") // Balancer implements the base.PickerBuilder interface. It's used to create new // balancer.Picker instances. It should only be used by nonce-service clients. @@ -84,7 +84,7 @@ func (p *Picker) Pick(info balancer.PickInfo) (balancer.PickResult, error) { // This should never happen. return balancer.PickResult{}, errMissingHMACKeyCtxKey } - hmacKey, ok := hmacKeyVal.(string) + hmacKey, ok := hmacKeyVal.([]byte) if !ok { // This should never happen. return balancer.PickResult{}, errInvalidHMACKeyCtxKeyType @@ -94,7 +94,7 @@ func (p *Picker) Pick(info balancer.PickInfo) (balancer.PickResult, error) { // First call to Pick with a new Picker. prefixToBackend := make(map[string]balancer.SubConn) for sc, scInfo := range p.backends { - scPrefix := nonce.DerivePrefix(scInfo.Address.Addr, []byte(hmacKey)) + scPrefix := nonce.DerivePrefix(scInfo.Address.Addr, hmacKey) prefixToBackend[scPrefix] = sc } p.prefixToBackend = prefixToBackend From 5da194c96427790fe280a1a4c69dcfc01eee4916 Mon Sep 17 00:00:00 2001 From: James Renken Date: Fri, 8 Nov 2024 17:04:46 -0800 Subject: [PATCH 07/10] Revert test prefix truncation (it's hex!) & fix more tests --- grpc/noncebalancer/noncebalancer.go | 2 +- grpc/noncebalancer/noncebalancer_test.go | 8 ++++---- nonce/nonce_test.go | 4 ++-- test/secrets/nonce_prefix_key | 2 +- wfe2/wfe_test.go | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/grpc/noncebalancer/noncebalancer.go b/grpc/noncebalancer/noncebalancer.go index 90027e7f834..525e663cb76 100644 --- a/grpc/noncebalancer/noncebalancer.go +++ b/grpc/noncebalancer/noncebalancer.go @@ -35,7 +35,7 @@ var ErrNoBackendsMatchPrefix = status.New(codes.Unavailable, "no backends match var errMissingPrefixCtxKey = errors.New("nonce.PrefixCtxKey value required in RPC context") var errMissingHMACKeyCtxKey = errors.New("nonce.HMACKeyCtxKey value required in RPC context") var errInvalidPrefixCtxKeyType = errors.New("nonce.PrefixCtxKey value in RPC context must be a string") -var errInvalidHMACKeyCtxKeyType = errors.New("nonce.HMACKeyCtxKey value in RPC context must be a byteslice") +var errInvalidHMACKeyCtxKeyType = errors.New("nonce.HMACKeyCtxKey value in RPC context must be a byte slice") // Balancer implements the base.PickerBuilder interface. It's used to create new // balancer.Picker instances. It should only be used by nonce-service clients. diff --git a/grpc/noncebalancer/noncebalancer_test.go b/grpc/noncebalancer/noncebalancer_test.go index 5b6298ac37c..33155dc361e 100644 --- a/grpc/noncebalancer/noncebalancer_test.go +++ b/grpc/noncebalancer/noncebalancer_test.go @@ -17,7 +17,7 @@ func TestPickerPicksCorrectBackend(t *testing.T) { prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, []byte("Kala namak")) testCtx := context.WithValue(context.Background(), nonce.PrefixCtxKey{}, "HNmOnt8w") - testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, prefix) + testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, []byte(prefix)) info := balancer.PickInfo{Ctx: testCtx} gotPick, err := p.Pick(info) @@ -29,7 +29,7 @@ func TestPickerMissingPrefixInCtx(t *testing.T) { _, p, subConns := setupTest(false) prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, []byte("Kala namak")) - testCtx := context.WithValue(context.Background(), nonce.HMACKeyCtxKey{}, prefix) + testCtx := context.WithValue(context.Background(), nonce.HMACKeyCtxKey{}, []byte(prefix)) info := balancer.PickInfo{Ctx: testCtx} gotPick, err := p.Pick(info) @@ -41,7 +41,7 @@ func TestPickerInvalidPrefixInCtx(t *testing.T) { _, p, _ := setupTest(false) testCtx := context.WithValue(context.Background(), nonce.PrefixCtxKey{}, 9) - testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, "foobar") + testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, []byte("foobar")) info := balancer.PickInfo{Ctx: testCtx} gotPick, err := p.Pick(info) @@ -77,7 +77,7 @@ func TestPickerNoMatchingSubConnAvailable(t *testing.T) { prefix := nonce.DerivePrefix(subConns[0].addrs[0].Addr, []byte("Kala namak")) testCtx := context.WithValue(context.Background(), nonce.PrefixCtxKey{}, "rUsTrUin") - testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, prefix) + testCtx = context.WithValue(testCtx, nonce.HMACKeyCtxKey{}, []byte(prefix)) info := balancer.PickInfo{Ctx: testCtx} gotPick, err := p.Pick(info) diff --git a/nonce/nonce_test.go b/nonce/nonce_test.go index c7100fe4308..42b43649117 100644 --- a/nonce/nonce_test.go +++ b/nonce/nonce_test.go @@ -147,6 +147,6 @@ func TestNoncePrefixValidation(t *testing.T) { } func TestDerivePrefix(t *testing.T) { - prefix := DerivePrefix("192.168.1.1:8080", []byte("3b8c758dd85e113ea340ce0b3a99f389")) - test.AssertEquals(t, prefix, "FxMmnuMq") + prefix := DerivePrefix("192.168.1.1:8080", []byte("3b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f")) + test.AssertEquals(t, prefix, "P9qQaK4o") } diff --git a/test/secrets/nonce_prefix_key b/test/secrets/nonce_prefix_key index 96bb4c2ed30..d65802423de 100644 --- a/test/secrets/nonce_prefix_key +++ b/test/secrets/nonce_prefix_key @@ -1 +1 @@ -3b8c758dd85e113ea340ce0b3a99f389 +3b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go index f0ae8e8cdb7..2d3150870ec 100644 --- a/wfe2/wfe_test.go +++ b/wfe2/wfe_test.go @@ -397,7 +397,7 @@ func setupWFE(t *testing.T) (WebFrontEndImpl, clock.FakeClock, requestSigner) { log := blog.NewMock() // Use derived nonces. - rncKey := []byte("b8c758dd85e113ea340ce0b3a99f3894") + rncKey := []byte("b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f") noncePrefix := nonce.DerivePrefix("192.168.1.1:8080", rncKey) nonceService, err := nonce.NewNonceService(metrics.NoopRegisterer, 100, noncePrefix) test.AssertNotError(t, err, "making nonceService") From 5f574c5d680d4fbc5cf33654bae8374e550bf221 Mon Sep 17 00:00:00 2001 From: James Renken Date: Fri, 8 Nov 2024 17:12:22 -0800 Subject: [PATCH 08/10] Okay I was right about truncation after all --- test/secrets/nonce_prefix_key | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/secrets/nonce_prefix_key b/test/secrets/nonce_prefix_key index d65802423de..96bb4c2ed30 100644 --- a/test/secrets/nonce_prefix_key +++ b/test/secrets/nonce_prefix_key @@ -1 +1 @@ -3b8c758dd85e113ea340ce0b3a99f389d40a308548af94d1730a7692c1874f1f +3b8c758dd85e113ea340ce0b3a99f389 From 0c27488301201d979d87ef816b49cb4e8671c39c Mon Sep 17 00:00:00 2001 From: James Renken Date: Tue, 12 Nov 2024 13:00:22 -0800 Subject: [PATCH 09/10] Remove deprecated noncePrefixKey from integration test --- test/integration/nonce_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/integration/nonce_test.go b/test/integration/nonce_test.go index 5beaea6190d..8475463aff4 100644 --- a/test/integration/nonce_test.go +++ b/test/integration/nonce_test.go @@ -23,7 +23,6 @@ type nonceBalancerTestConfig struct { TLS cmd.TLSConfig GetNonceService *cmd.GRPCClientConfig RedeemNonceService *cmd.GRPCClientConfig - NoncePrefixKey cmd.PasswordConfig NonceHMACKey cmd.HMACKeyConfig } } @@ -42,13 +41,8 @@ func TestNonceBalancer_NoBackendMatchingPrefix(t *testing.T) { tlsConfig, err := c.NotWFE.TLS.Load(metrics.NoopRegisterer) test.AssertNotError(t, err, "Could not load TLS config") - var rncKey []byte - rncKey, err = c.NotWFE.NonceHMACKey.Load() - if err != nil { - rncKeyString, err := c.NotWFE.NoncePrefixKey.Pass() - test.AssertNotError(t, err, "Failed to load nonceHMACKey or noncePrefixKey") - rncKey = []byte(rncKeyString) - } + rncKey, err := c.NotWFE.NonceHMACKey.Load() + test.AssertNotError(t, err, "Failed to load nonceHMACKey") clk := clock.New() From ada6bee5cc79e90b810289e7e36957fc133c1a85 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 11 Nov 2024 08:07:20 -0800 Subject: [PATCH 10/10] ra: remove special case for empty DNSNames (#7795) This case was added to work around a test case that didn't fill it out; instead, fill DNSNames for that test case. --- ra/ra.go | 13 ++++--------- ra/ra_test.go | 11 ++++++++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/ra/ra.go b/ra/ra.go index 7fe7c5d916d..e1b375d7a17 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1336,16 +1336,11 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner( return nil, nil, wrapError(err, "getting SCTs") } - var isRenewal bool - if len(parsedPrecert.DNSNames) > 0 { - // This should never happen under normal operation, but it sometimes - // occurs under test. - exists, err := ra.SA.FQDNSetExists(ctx, &sapb.FQDNSetExistsRequest{DnsNames: parsedPrecert.DNSNames}) - if err != nil { - return nil, nil, wrapError(err, "checking if certificate is a renewal") - } - isRenewal = exists.Exists + exists, err := ra.SA.FQDNSetExists(ctx, &sapb.FQDNSetExistsRequest{DnsNames: parsedPrecert.DNSNames}) + if err != nil { + return nil, nil, wrapError(err, "checking if certificate is a renewal") } + isRenewal := exists.Exists cert, err := ra.CA.IssueCertificateForPrecertificate(ctx, &capb.IssueCertificateForPrecertificateRequest{ DER: precert.DER, diff --git a/ra/ra_test.go b/ra/ra_test.go index 7cb1a2cf842..fa0bfe3f4bb 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -3755,15 +3755,20 @@ type mockCAFailCertForPrecert struct { // IssuePrecertificate needs to be mocked for mockCAFailCertForPrecert's `IssueCertificateForPrecertificate` to get called. func (ca *mockCAFailCertForPrecert) IssuePrecertificate( - context.Context, - *capb.IssueCertificateRequest, - ...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) { + ctx context.Context, + req *capb.IssueCertificateRequest, + opts ...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) { k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { return nil, err } + parsedCSR, err := x509.ParseCertificateRequest(req.Csr) + if err != nil { + return nil, err + } tmpl := &ctx509.Certificate{ SerialNumber: big.NewInt(1), + DNSNames: parsedCSR.DNSNames, ExtraExtensions: []ctpkix.Extension{ { Id: ctx509.OIDExtensionCTPoison,