Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VA: Make PerformValidation more like DoDCV #7828

Merged
merged 11 commits into from
Nov 20, 2024
18 changes: 0 additions & 18 deletions core/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,24 +150,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.
Expand Down
8 changes: 5 additions & 3 deletions grpc/pb-marshalling.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
4 changes: 3 additions & 1 deletion grpc/pb-marshalling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -1792,7 +1792,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
}
Expand Down
8 changes: 4 additions & 4 deletions va/caa.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,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)
Expand Down Expand Up @@ -76,7 +76,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)
}()
Expand Down
42 changes: 24 additions & 18 deletions va/va.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,15 +281,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
Expand Down Expand Up @@ -623,9 +624,14 @@ 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. The method returns a
// ValidationResult and an error if the validation fails. The ValidationResult
beautifulentropy marked this conversation as resolved.
Show resolved Hide resolved
// always includes a list of ValidationRecords, even when it also contains a
// Problem. PerformValidation 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")
Expand All @@ -648,10 +654,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 := ""
Expand All @@ -673,7 +679,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)
}()

Expand Down Expand Up @@ -701,7 +707,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
Expand All @@ -710,5 +716,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)
}
10 changes: 5 additions & 5 deletions va/va_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
}

Expand All @@ -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
Expand Down
Loading