VA: Make PerformValidation more like DoDCV (#7828)

- Remove Perspective and RIR from ValidationRecords
- Make ValidationResultToPB Perspective and RIR aware
- Update comment for VA.PerformValidation
- Make verificationRequestEvent more like doDCVAuditLog
- Update language used in problems created by performRemoteValidation to
be more like doRemoteDCV.
This commit is contained in:
Samantha Frank 2024-11-20 14:13:55 -05:00 committed by GitHub
parent c2632585f5
commit 8bf13a90f4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 59 additions and 69 deletions

View File

@ -147,24 +147,6 @@ type ValidationRecord struct {
// lookup for AddressUsed. During recursive A and AAAA lookups, a record may
// instead look like A:host:port or AAAA:host:port
ResolverAddrs []string `json:"resolverAddrs,omitempty"`
// Perspective uniquely identifies the Network Perspective used to perform
// the validation, as specified in BRs Section 5.4.1, Requirement 2.7
// ("Multi-Perspective Issuance Corroboration attempts from each Network
// Perspective"). It should uniquely identify either the Primary Perspective
// (VA) or a group of RVAs deployed in the same datacenter.
Perspective string `json:"perspective,omitempty"`
// RIR indicates the Regional Internet Registry where this RVA is located.
// This field is used to identify the RIR region from which a given
// validation was performed, as specified in the "Phased Implementation
// Timeline" in BRs Section 3.2.2.9. It must be one of the following values:
// - ARIN
// - RIPE
// - APNIC
// - LACNIC
// - AfriNIC
RIR string `json:"rir,omitempty"`
}
// Challenge is an aggregate of all data needed for any challenges.

View File

@ -179,7 +179,7 @@ func PBToValidationRecord(in *corepb.ValidationRecord) (record core.ValidationRe
}, nil
}
func ValidationResultToPB(records []core.ValidationRecord, prob *probs.ProblemDetails) (*vapb.ValidationResult, error) {
func ValidationResultToPB(records []core.ValidationRecord, prob *probs.ProblemDetails, perspective, rir string) (*vapb.ValidationResult, error) {
recordAry := make([]*corepb.ValidationRecord, len(records))
var err error
for i, v := range records {
@ -193,8 +193,10 @@ func ValidationResultToPB(records []core.ValidationRecord, prob *probs.ProblemDe
return nil, err
}
return &vapb.ValidationResult{
Records: recordAry,
Problems: marshalledProbs,
Records: recordAry,
Problems: marshalledProbs,
Perspective: perspective,
Rir: rir,
}, nil
}

View File

@ -154,9 +154,11 @@ func TestValidationResult(t *testing.T) {
result := []core.ValidationRecord{vrA, vrB}
prob := &probs.ProblemDetails{Type: probs.TLSProblem, Detail: "asd", HTTPStatus: 200}
pb, err := ValidationResultToPB(result, prob)
pb, err := ValidationResultToPB(result, prob, "surreal", "ARIN")
test.AssertNotError(t, err, "ValidationResultToPB failed")
test.Assert(t, pb != nil, "Returned vapb.ValidationResult is nil")
test.AssertEquals(t, pb.Perspective, "surreal")
test.AssertEquals(t, pb.Rir, "ARIN")
reconResult, reconProb, err := pbToValidationResult(pb)
test.AssertNotError(t, err, "pbToValidationResult failed")

View File

@ -1753,7 +1753,7 @@ func (ra *RegistrationAuthorityImpl) recordValidation(ctx context.Context, authI
} else {
expires = ra.clk.Now().Add(ra.authorizationLifetime)
}
vr, err := bgrpc.ValidationResultToPB(challenge.ValidationRecord, challenge.Error)
vr, err := bgrpc.ValidationResultToPB(challenge.ValidationRecord, challenge.Error, "", "")
if err != nil {
return err
}

View File

@ -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 validation: "):
raise(Exception("expected 'During secondary validation' problem detail, found {0}".format(httpChall.error.detail)))
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)))
class FakeH2ServerHandler(socketserver.BaseRequestHandler):
"""

View File

@ -35,10 +35,10 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC
return nil, berrors.InternalServerError("incomplete IsCAAValid request")
}
logEvent := verificationRequestEvent{
// TODO(#7061) Plumb req.Authz.Id as "ID:" through from the RA to
// TODO(#7061) Plumb req.Authz.Id as "AuthzID:" through from the RA to
// correlate which authz triggered this request.
Requester: req.AccountURIID,
Hostname: req.Domain,
Requester: req.AccountURIID,
Identifier: req.Domain,
}
challType := core.AcmeChallenge(req.ValidationMethod)
@ -75,7 +75,7 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC
va.observeLatency(opCAA, allPerspectives, string(challType), probType, outcome, va.clk.Since(start))
}
// Log the total check latency.
logEvent.ValidationLatency = va.clk.Since(start).Round(time.Millisecond).Seconds()
logEvent.Latency = va.clk.Since(start).Round(time.Millisecond).Seconds()
va.log.AuditObject("CAA check result", logEvent)
}()

View File

@ -280,15 +280,16 @@ func maxAllowedFailures(perspectiveCount int) int {
return 2
}
// Used for audit logging
// verificationRequestEvent is logged once for each validation attempt. Its
// fields are exported for logging purposes.
type verificationRequestEvent struct {
ID string `json:",omitempty"`
Requester int64 `json:",omitempty"`
Hostname string `json:",omitempty"`
Challenge core.Challenge `json:",omitempty"`
ValidationLatency float64
Error string `json:",omitempty"`
InternalError string `json:",omitempty"`
AuthzID string
Requester int64
Identifier string
Challenge core.Challenge
Error string `json:",omitempty"`
InternalError string `json:",omitempty"`
Latency float64
}
// ipError is an error type used to pass though the IP address of the remote
@ -462,14 +463,10 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
responses := make(chan *response, remoteVACount)
for _, i := range rand.Perm(remoteVACount) {
go func(rva RemoteVA, out chan<- *response) {
go func(rva RemoteVA) {
res, err := rva.PerformValidation(ctx, req)
out <- &response{
addr: rva.Address,
result: res,
err: err,
}
}(va.remoteVAs[i], responses)
responses <- &response{rva.Address, res, err}
}(va.remoteVAs[i])
}
required := remoteVACount - va.maxRemoteFailures
@ -485,10 +482,10 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
failed = append(failed, resp.addr)
if core.IsCanceled(resp.err) {
currProb = probs.ServerInternal("Remote PerformValidation RPC canceled")
currProb = probs.ServerInternal("Secondary domain validation RPC canceled")
} else {
va.log.Errf("Remote VA %q.PerformValidation failed: %s", resp.addr, resp.err)
currProb = probs.ServerInternal("Remote PerformValidation RPC failed")
currProb = probs.ServerInternal("Secondary domain validation RPC failed")
}
} else if resp.result.Problems != nil {
// The remote VA returned a problem.
@ -498,7 +495,7 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
currProb, err = bgrpc.PBToProblemDetails(resp.result.Problems)
if err != nil {
va.log.Errf("Remote VA %q.PerformValidation returned malformed problem: %s", resp.addr, err)
currProb = probs.ServerInternal("Remote PerformValidation RPC returned malformed result")
currProb = probs.ServerInternal("Secondary domain validation RPC returned malformed result")
}
} else {
// The remote VA returned a successful result.
@ -516,7 +513,7 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
}
if len(failed) > va.maxRemoteFailures {
// Too many failed responses to reach quorum.
firstProb.Detail = fmt.Sprintf("During secondary validation: %s", firstProb.Detail)
firstProb.Detail = fmt.Sprintf("During secondary domain validation: %s", firstProb.Detail)
return firstProb
}
@ -622,9 +619,16 @@ func (va *ValidationAuthorityImpl) performLocalValidation(
return records, nil
}
// PerformValidation validates the challenge for the domain in the request.
// The returned result will always contain a list of validation records, even
// when it also contains a problem.
// PerformValidation conducts a local Domain Control Validation (DCV) and CAA
// check for the specified challenge and dnsName. When invoked on the primary
// Validation Authority (VA) and the local validation succeeds, it also performs
// DCV and CAA checks using the configured remote VAs. Failed validations are
// indicated by a non-nil Problems in the returned ValidationResult.
// PerformValidation returns error only for internal logic errors (and the
// client may receive errors from gRPC in the event of a communication problem).
// ValidationResult always includes a list of ValidationRecords, even when it
// also contains Problems. This method does NOT implement Multi-Perspective
// Issuance Corroboration as defined in BRs Sections 3.2.2.9 and 5.4.1.
func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) {
if core.IsAnyNilOrZero(req, req.DnsName, req.Challenge, req.Authz, req.ExpectedKeyAuthorization) {
return nil, berrors.InternalServerError("Incomplete validation request")
@ -647,10 +651,10 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
var localLatency time.Duration
start := va.clk.Now()
logEvent := verificationRequestEvent{
ID: req.Authz.Id,
Requester: req.Authz.RegID,
Hostname: req.DnsName,
Challenge: chall,
AuthzID: req.Authz.Id,
Requester: req.Authz.RegID,
Identifier: req.DnsName,
Challenge: chall,
}
defer func() {
probType := ""
@ -672,7 +676,7 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
}
// Log the total validation latency.
logEvent.ValidationLatency = time.Since(start).Round(time.Millisecond).Seconds()
logEvent.Latency = time.Since(start).Round(time.Millisecond).Seconds()
va.log.AuditObject("Validation result", logEvent)
}()
@ -700,7 +704,7 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
if err != nil {
logEvent.InternalError = err.Error()
prob = detailedError(err)
return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob))
return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir)
}
// Do remote validation. We do this after local validation is complete to
@ -709,5 +713,5 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
// own validation records, and it's not helpful to present multiple large
// errors to the end user.
prob = va.performRemoteValidation(ctx, req)
return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob))
return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir)
}

View File

@ -304,8 +304,8 @@ func TestPerformValidationValid(t *testing.T) {
if len(resultLog) != 1 {
t.Fatalf("Wrong number of matching lines for 'Validation result'")
}
if !strings.Contains(resultLog[0], `"Hostname":"good-dns01.com"`) {
t.Error("PerformValidation didn't log validation hostname.")
if !strings.Contains(resultLog[0], `"Identifier":"good-dns01.com"`) {
t.Error("PerformValidation didn't log validation identifier.")
}
}
@ -332,9 +332,9 @@ func TestPerformValidationWildcard(t *testing.T) {
t.Fatalf("Wrong number of matching lines for 'Validation result'")
}
// We expect that the top level Hostname reflect the wildcard name
if !strings.Contains(resultLog[0], `"Hostname":"*.good-dns01.com"`) {
t.Errorf("PerformValidation didn't log correct validation hostname.")
// We expect that the top level Identifier reflect the wildcard name
if !strings.Contains(resultLog[0], `"Identifier":"*.good-dns01.com"`) {
t.Errorf("PerformValidation didn't log correct validation identifier.")
}
// We expect that the ValidationRecord contain the correct non-wildcard
// hostname that was validated
@ -451,7 +451,7 @@ func TestMultiVA(t *testing.T) {
{brokenVA, "broken"},
},
AllowedUAs: allowedUAs,
ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC failed"),
// The real failure cause should be logged
ExpectedLog: expectedInternalErrLine,
},
@ -478,7 +478,7 @@ func TestMultiVA(t *testing.T) {
{brokenVA, "broken"},
},
AllowedUAs: allowedUAs,
ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC failed"),
// The real failure cause should be logged
ExpectedLog: expectedInternalErrLine,
},
@ -507,7 +507,7 @@ func TestMultiVA(t *testing.T) {
{brokenVA, "broken"},
},
AllowedUAs: allowedUAs,
ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC failed"),
// The real failure cause should be logged
ExpectedLog: expectedInternalErrLine,
},
@ -517,7 +517,7 @@ func TestMultiVA(t *testing.T) {
RemoteVAs: remoteVAs,
AllowedUAs: map[string]bool{localUA: true, remoteUA2: true},
ExpectedProb: probs.Unauthorized(fmt.Sprintf(
`During secondary validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
`During secondary domain validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
expectedKeyAuthorization)),
},
{
@ -539,7 +539,7 @@ func TestMultiVA(t *testing.T) {
{cancelledVA, remoteUA3},
},
AllowedUAs: allowedUAs,
ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC canceled"),
ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC canceled"),
},
{
// With the local and remote VAs seeing diff problems, we expect a problem.
@ -547,7 +547,7 @@ func TestMultiVA(t *testing.T) {
RemoteVAs: remoteVAs,
AllowedUAs: map[string]bool{localUA: true},
ExpectedProb: probs.Unauthorized(fmt.Sprintf(
`During secondary validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
`During secondary domain validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
expectedKeyAuthorization)),
},
}