diff --git a/cmd/boulder-va/main.go b/cmd/boulder-va/main.go index 72ab5dbd3eb..f2c2c848766 100644 --- a/cmd/boulder-va/main.go +++ b/cmd/boulder-va/main.go @@ -43,10 +43,10 @@ type RemoteVAGRPCClientConfig struct { // - RIPE // - APNIC // - LACNIC - // - AfriNIC + // - AFRINIC // // TODO(#7615): Make mandatory. - RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AfriNIC"` + RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AFRINIC"` } type Config struct { diff --git a/cmd/remoteva/main.go b/cmd/remoteva/main.go index 97320f9710f..fa83487503e 100644 --- a/cmd/remoteva/main.go +++ b/cmd/remoteva/main.go @@ -27,7 +27,7 @@ type Config struct { // of RVAs deployed in the same datacenter. // // TODO(#7615): Make mandatory. - Perspective string `validate:"omitempty"` + Perspective string `omitempty:"omitempty"` // RIR indicates the Regional Internet Registry where this RVA is // located. This field is used to identify the RIR region from which a @@ -38,10 +38,10 @@ type Config struct { // - RIPE // - APNIC // - LACNIC - // - AfriNIC + // - AFRINIC // // TODO(#7615): Make mandatory. - RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AfriNIC"` + RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AFRINIC"` // SkipGRPCClientCertVerification, when disabled as it should typically // be, will cause the remoteva server (which receives gRPCs from a diff --git a/test/config/remoteva-a.json b/test/config/remoteva-a.json index ca21d7c89ea..8725cda2f3c 100644 --- a/test/config/remoteva-a.json +++ b/test/config/remoteva-a.json @@ -38,7 +38,9 @@ "accountURIPrefixes": [ "http://boulder.service.consul:4000/acme/reg/", "http://boulder.service.consul:4001/acme/acct/" - ] + ], + "perspective": "dadaist", + "rir": "ARIN" }, "syslog": { "stdoutlevel": 4, diff --git a/test/config/remoteva-b.json b/test/config/remoteva-b.json index f49cd16c141..5089b7a452c 100644 --- a/test/config/remoteva-b.json +++ b/test/config/remoteva-b.json @@ -38,7 +38,9 @@ "accountURIPrefixes": [ "http://boulder.service.consul:4000/acme/reg/", "http://boulder.service.consul:4001/acme/acct/" - ] + ], + "perspective": "surrealist", + "rir": "RIPE" }, "syslog": { "stdoutlevel": 4, diff --git a/test/config/remoteva-c.json b/test/config/remoteva-c.json index 27dbd79563d..5234429f5b2 100644 --- a/test/config/remoteva-c.json +++ b/test/config/remoteva-c.json @@ -38,7 +38,9 @@ "accountURIPrefixes": [ "http://boulder.service.consul:4000/acme/reg/", "http://boulder.service.consul:4001/acme/acct/" - ] + ], + "perspective": "cubist", + "rir": "ARIN" }, "syslog": { "stdoutlevel": 4, diff --git a/test/config/va.json b/test/config/va.json index 36c8349e1c4..09ec1f634f4 100644 --- a/test/config/va.json +++ b/test/config/va.json @@ -43,17 +43,23 @@ { "serverAddress": "rva1.service.consul:9397", "timeout": "15s", - "hostOverride": "rva1.boulder" + "hostOverride": "rva1.boulder", + "perspective": "dadaist", + "rir": "ARIN" }, { "serverAddress": "rva1.service.consul:9498", "timeout": "15s", - "hostOverride": "rva1.boulder" + "hostOverride": "rva1.boulder", + "perspective": "surrealist", + "rir": "RIPE" }, { "serverAddress": "rva1.service.consul:9499", "timeout": "15s", - "hostOverride": "rva1.boulder" + "hostOverride": "rva1.boulder", + "perspective": "cubist", + "rir": "ARIN" } ], "maxRemoteValidationFailures": 1, diff --git a/va/caa.go b/va/caa.go index a604664f5f2..0b27c90cac1 100644 --- a/va/caa.go +++ b/va/caa.go @@ -112,12 +112,19 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC // It will also later be serialized in JSON, which defaults to UTF-8. Make // sure it is UTF-8 clean now. prob = filterProblemDetails(prob) - return &vapb.IsCAAValidResponse{Problem: &corepb.ProblemDetails{ - ProblemType: string(prob.Type), - Detail: replaceInvalidUTF8([]byte(prob.Detail)), - }}, nil + return &vapb.IsCAAValidResponse{ + Problem: &corepb.ProblemDetails{ + ProblemType: string(prob.Type), + Detail: replaceInvalidUTF8([]byte(prob.Detail)), + }, + Perspective: va.perspective, + Rir: va.rir, + }, nil } else { - return &vapb.IsCAAValidResponse{}, nil + return &vapb.IsCAAValidResponse{ + Perspective: va.perspective, + Rir: va.rir, + }, nil } } diff --git a/va/va.go b/va/va.go index f575992a548..d54f9214077 100644 --- a/va/va.go +++ b/va/va.go @@ -241,7 +241,7 @@ func NewValidationAuthorityImpl( for i, va1 := range remoteVAs { for j, va2 := range remoteVAs { - // TODO(#7819): Remove the != "" check once perspective is required. + // TODO(#7615): Remove the != "" check once perspective is required. if i != j && va1.Perspective == va2.Perspective && va1.Perspective != "" { return nil, fmt.Errorf("duplicate remote VA perspective %q", va1.Perspective) } @@ -504,6 +504,19 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o for _, i := range rand.Perm(remoteVACount) { go func(rva RemoteVA) { res, err := op(subCtx, rva, req) + if err != nil { + responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err} + return + } + // TODO(#7615): Remove the != "" checks once perspective and rir are required. + if (rva.Perspective != "" && res.GetPerspective() != "" && res.GetPerspective() != rva.Perspective) || + (rva.RIR != "" && res.GetRir() != "" && res.GetRir() != rva.RIR) { + err = fmt.Errorf( + "Expected perspective %q (%q) but got reply from %q (%q) - misconfiguration likely", rva.Perspective, rva.RIR, res.GetPerspective(), res.GetRir(), + ) + responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err} + return + } responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err} }(va.remoteVAs[i]) } @@ -538,7 +551,7 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o } if isCAACheck { // We're checking CAA, log the problem. - va.log.Errf("Operation on Remote VA (%s) returned a problem: %s", resp.addr, err) + va.log.Errf("Operation on Remote VA (%s) returned a problem: %s", resp.addr, currProb) } } else { // The remote VA returned a successful result. diff --git a/va/va_test.go b/va/va_test.go index a7d7e86c50f..b7925a3cc6c 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -160,7 +160,7 @@ func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride b rva.perspective = perspective rva.rir = rir - return RemoteClients{VAClient: &inMemVA{*rva}, CAAClient: &inMemVA{*rva}} + return RemoteClients{VAClient: &inMemVA{rva}, CAAClient: &inMemVA{rva}} } // RIRs @@ -296,14 +296,14 @@ func (b brokenRemoteVA) IsCAAValid(_ context.Context, _ *vapb.IsCAAValidRequest, // ValidationAuthorityImpl rather than over the network. This lets a local // in-memory mock VA act like a remote VA. type inMemVA struct { - rva ValidationAuthorityImpl + rva *ValidationAuthorityImpl } -func (inmem inMemVA) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { +func (inmem *inMemVA) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { return inmem.rva.PerformValidation(ctx, req) } -func (inmem inMemVA) IsCAAValid(ctx context.Context, req *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { +func (inmem *inMemVA) IsCAAValid(ctx context.Context, req *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { return inmem.rva.IsCAAValid(ctx, req) } @@ -333,6 +333,48 @@ func TestNewValidationAuthorityImplWithDuplicateRemotes(t *testing.T) { test.AssertContains(t, err.Error(), "duplicate remote VA perspective \"dadaist\"") } +func TestPerformValidationWithMismatchedRemoteVAPerspectives(t *testing.T) { + mismatched1 := RemoteVA{ + RemoteClients: setupRemote(nil, "", nil, "dadaist", arin), + Perspective: "baroque", + RIR: arin, + } + mismatched2 := RemoteVA{ + RemoteClients: setupRemote(nil, "", nil, "impressionist", ripe), + Perspective: "minimalist", + RIR: ripe, + } + remoteVAs := setupRemotes([]remoteConf{{rir: ripe}}, nil) + remoteVAs = append(remoteVAs, mismatched1, mismatched2) + va, mockLog := setup(nil, "", remoteVAs, nil) + + req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) + res, _ := va.PerformValidation(context.Background(), req) + test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") + test.AssertEquals(t, len(mockLog.GetAllMatching("Expected perspective")), 2) +} + +func TestPerformValidationWithMismatchedRemoteVARIRs(t *testing.T) { + mismatched1 := RemoteVA{ + RemoteClients: setupRemote(nil, "", nil, "dadaist", arin), + Perspective: "dadaist", + RIR: ripe, + } + mismatched2 := RemoteVA{ + RemoteClients: setupRemote(nil, "", nil, "impressionist", ripe), + Perspective: "impressionist", + RIR: arin, + } + remoteVAs := setupRemotes([]remoteConf{{rir: ripe}}, nil) + remoteVAs = append(remoteVAs, mismatched1, mismatched2) + va, mockLog := setup(nil, "", remoteVAs, nil) + + req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) + res, _ := va.PerformValidation(context.Background(), req) + test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") + test.AssertEquals(t, len(mockLog.GetAllMatching("Expected perspective")), 2) +} + func TestValidateMalformedChallenge(t *testing.T) { va, _ := setup(nil, "", nil, nil)