Skip to content

Commit

Permalink
VA: Make performRemoteValidation more generic (#7847)
Browse files Browse the repository at this point in the history
- Make performRemoteValidation a more generic function that returns a
new remoteResult interface
- Modify the return value of IsCAAValid and PerformValidation to satisfy
the remoteResult interface
- Include compile time checks and tests that pass an arbitrary operation
  • Loading branch information
beautifulentropy authored Nov 27, 2024
1 parent ded2e5e commit 27a7714
Show file tree
Hide file tree
Showing 9 changed files with 215 additions and 100 deletions.
6 changes: 3 additions & 3 deletions grpc/pb-marshalling.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,13 @@ func ValidationResultToPB(records []core.ValidationRecord, prob *probs.ProblemDe
return nil, err
}
}
marshalledProbs, err := ProblemDetailsToPB(prob)
marshalledProb, err := ProblemDetailsToPB(prob)
if err != nil {
return nil, err
}
return &vapb.ValidationResult{
Records: recordAry,
Problems: marshalledProbs,
Problem: marshalledProb,
Perspective: perspective,
Rir: rir,
}, nil
Expand All @@ -212,7 +212,7 @@ func pbToValidationResult(in *vapb.ValidationResult) ([]core.ValidationRecord, *
return nil, nil, err
}
}
prob, err := PBToProblemDetails(in.Problems)
prob, err := PBToProblemDetails(in.Problem)
if err != nil {
return nil, nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -1762,7 +1762,7 @@ func (ra *RegistrationAuthorityImpl) recordValidation(ctx context.Context, authI
Attempted: string(challenge.Type),
AttemptedAt: validated,
ValidationRecords: vr.Records,
ValidationError: vr.Problems,
ValidationError: vr.Problem,
})
return err
}
Expand Down Expand Up @@ -1929,8 +1929,8 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
prob = probs.ServerInternal("Could not communicate with VA")
ra.log.AuditErrf("Could not communicate with VA: %s", err)
} else {
if res.Problems != nil {
prob, err = bgrpc.PBToProblemDetails(res.Problems)
if res.Problem != nil {
prob, err = bgrpc.PBToProblemDetails(res.Problem)
if err != nil {
prob = probs.ServerInternal("Could not communicate with VA")
ra.log.AuditErrf("Could not communicate with VA: %s", err)
Expand Down
12 changes: 6 additions & 6 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ func TestPerformValidationAlreadyValid(t *testing.T) {
Url: "http://example.com/",
},
},
Problems: nil,
Problem: nil,
}

// A subsequent call to perform validation should return nil due
Expand Down Expand Up @@ -758,7 +758,7 @@ func TestPerformValidationSuccess(t *testing.T) {
ResolverAddrs: []string{"rebound"},
},
},
Problems: nil,
Problem: nil,
}

now := fc.Now()
Expand Down Expand Up @@ -901,7 +901,7 @@ func TestPerformValidation_FailedValidationsTriggerPauseIdentifiersRatelimit(t *
ResolverAddrs: []string{"rebound"},
},
},
Problems: &corepb.ProblemDetails{
Problem: &corepb.ProblemDetails{
Detail: fmt.Sprintf("CAA invalid for %s", domain),
},
}
Expand Down Expand Up @@ -954,7 +954,7 @@ func TestPerformValidation_FailedValidationsTriggerPauseIdentifiersRatelimit(t *
ResolverAddrs: []string{"rebound"},
},
},
Problems: &corepb.ProblemDetails{
Problem: &corepb.ProblemDetails{
Detail: fmt.Sprintf("CAA invalid for %s", domain),
},
}
Expand Down Expand Up @@ -1034,7 +1034,7 @@ func TestPerformValidation_FailedThenSuccessfulValidationResetsPauseIdentifiersR
ResolverAddrs: []string{"rebound"},
},
},
Problems: &corepb.ProblemDetails{
Problem: &corepb.ProblemDetails{
Detail: fmt.Sprintf("CAA invalid for %s", domain),
},
}
Expand Down Expand Up @@ -1092,7 +1092,7 @@ func TestPerformValidation_FailedThenSuccessfulValidationResetsPauseIdentifiersR
ResolverAddrs: []string{"rebound"},
},
},
Problems: nil,
Problem: nil,
}

challIdx = dnsChallIdx(t, authzPB.Challenges)
Expand Down
4 changes: 2 additions & 2 deletions test/v2_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -1083,8 +1083,8 @@ def test_http_multiva_threshold_fail():
raise(Exception("no HTTP-01 challenge in failed authz"))
if httpChall.error.typ != "urn:ietf:params:acme:error:unauthorized":
raise(Exception("expected unauthorized prob, found {0}".format(httpChall.error.typ)))
if not httpChall.error.detail.startswith("During secondary domain validation: "):
raise(Exception("expected 'During secondary domain validation' problem detail, found {0}".format(httpChall.error.detail)))
if not httpChall.error.detail.startswith("During secondary validation: "):
raise(Exception("expected 'During secondary validation' problem detail, found {0}".format(httpChall.error.detail)))

class FakeH2ServerHandler(socketserver.BaseRequestHandler):
"""
Expand Down
4 changes: 2 additions & 2 deletions va/dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ func TestDNSValidationEmpty(t *testing.T) {
// metrics checked below are incremented.
req := createValidationRequest("empty-txts.com", core.ChallengeTypeDNS01)
res, _ := va.PerformValidation(context.Background(), req)
test.AssertEquals(t, res.Problems.ProblemType, "unauthorized")
test.AssertEquals(t, res.Problems.Detail, "No TXT record found at _acme-challenge.empty-txts.com")
test.AssertEquals(t, res.Problem.ProblemType, "unauthorized")
test.AssertEquals(t, res.Problem.Detail, "No TXT record found at _acme-challenge.empty-txts.com")

test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{
"operation": opChallAndCAA,
Expand Down
111 changes: 65 additions & 46 deletions va/proto/va.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion va/proto/va.proto
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ message IsCAAValidRequest {
// If CAA is valid for the requested domain, the problem will be empty
message IsCAAValidResponse {
core.ProblemDetails problem = 1;
string perspective = 3;
string rir = 4;
}

message PerformValidationRequest {
Expand All @@ -39,7 +41,7 @@ message AuthzMeta {

message ValidationResult {
repeated core.ValidationRecord records = 1;
core.ProblemDetails problems = 2;
core.ProblemDetails problem = 2;
string perspective = 3;
string rir = 4;
}
Loading

0 comments on commit 27a7714

Please sign in to comment.