Skip to content

va: Use Identifier in IsCAAValidRequest #8153

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

va: Use Identifier in IsCAAValidRequest #8153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a4d0cc
va: Use Identifier in IsCAAValidRequest
jprenken May 1, 2025
661c161
Address feedback
jprenken May 1, 2025
215333d
Re-add va.pb.go with new protoc-gen
jprenken May 2, 2025
dbdccf6
va: Use Identifier in IsCAAValidRequest
jprenken May 1, 2025
03e37f4
Address feedback
jprenken May 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions ra/ra.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,8 +837,6 @@ func (ra *RegistrationAuthorityImpl) recheckCAA(ctx context.Context, authzs []*c
ch := make(chan authzCAAResult, len(authzs))
for _, authz := range authzs {
go func(authz *core.Authorization) {
name := authz.Identifier.Value

// If an authorization has multiple valid challenges,
// the type of the first valid challenge is used for
// the purposes of CAA rechecking.
Expand All @@ -854,22 +852,23 @@ func (ra *RegistrationAuthorityImpl) recheckCAA(ctx context.Context, authzs []*c
authz: authz,
err: berrors.InternalServerError(
"Internal error determining validation method for authorization ID %v (%v)",
authz.ID, name),
authz.ID, authz.Identifier.Value),
}
return
}
var resp *vapb.IsCAAValidResponse
var err error
resp, err = ra.VA.DoCAA(ctx, &vapb.IsCAAValidRequest{
Domain: name,
Identifier: authz.Identifier.ToProto(),
Domain: authz.Identifier.Value,
ValidationMethod: method,
AccountURIID: authz.RegistrationID,
})
if err != nil {
ra.log.AuditErrf("Rechecking CAA: %s", err)
err = berrors.InternalServerError(
"Internal error rechecking CAA for authorization ID %v (%v)",
authz.ID, name,
authz.ID, authz.Identifier.Value,
)
} else if resp.Problem != nil {
err = berrors.CAAError("rechecking caa: %s", resp.Problem.Detail)
Expand Down Expand Up @@ -1634,6 +1633,7 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
ExpectedKeyAuthorization: expectedKeyAuthorization,
},
&vapb.IsCAAValidRequest{
Identifier: authz.Identifier.ToProto(),
Domain: authz.Identifier.Value,
ValidationMethod: chall.Type,
AccountURIID: authz.RegistrationID,
Expand Down
10 changes: 5 additions & 5 deletions ra/ra_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (dva *DummyValidationAuthority) PerformValidation(ctx context.Context, req
return dcvRes, nil
}
caaResp, err := dva.DoCAA(ctx, &vapb.IsCAAValidRequest{
Domain: req.DnsName,
Identifier: req.Identifier,
ValidationMethod: req.Challenge.Type,
AccountURIID: req.Authz.RegID,
AuthzID: req.Authz.Id,
Expand Down Expand Up @@ -1033,7 +1033,7 @@ func (cr *caaRecorder) IsCAAValid(
) (*vapb.IsCAAValidResponse, error) {
cr.Lock()
defer cr.Unlock()
cr.names[in.Domain] = true
cr.names[in.Identifier.Value] = true
return &vapb.IsCAAValidResponse{}, nil
}

Expand All @@ -1044,7 +1044,7 @@ func (cr *caaRecorder) DoCAA(
) (*vapb.IsCAAValidResponse, error) {
cr.Lock()
defer cr.Unlock()
cr.names[in.Domain] = true
cr.names[in.Identifier.Value] = true
return &vapb.IsCAAValidResponse{}, nil
}

Expand Down Expand Up @@ -1225,7 +1225,7 @@ func (cf *caaFailer) IsCAAValid(
opts ...grpc.CallOption,
) (*vapb.IsCAAValidResponse, error) {
cvrpb := &vapb.IsCAAValidResponse{}
switch in.Domain {
switch in.Identifier.Value {
case "a.com":
cvrpb.Problem = &corepb.ProblemDetails{
Detail: "CAA invalid for a.com",
Expand All @@ -1246,7 +1246,7 @@ func (cf *caaFailer) DoCAA(
opts ...grpc.CallOption,
) (*vapb.IsCAAValidResponse, error) {
cvrpb := &vapb.IsCAAValidResponse{}
switch in.Domain {
switch in.Identifier.Value {
case "a.com":
cvrpb.Problem = &corepb.ProblemDetails{
Detail: "CAA invalid for a.com",
Expand Down
35 changes: 23 additions & 12 deletions va/caa.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,32 @@ type caaParams struct {
// implements the CAA portion of Multi-Perspective Issuance Corroboration as
// defined in BRs Sections 3.2.2.9 and 5.4.1.
func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAValidRequest) (*vapb.IsCAAValidResponse, error) {
if core.IsAnyNilOrZero(req.Domain, req.ValidationMethod, req.AccountURIID) {
// TODO(#8023): Once Domain is no longer used, remove the conditional, use
// req.Identifier directly, and include it in the IsAnyNilOrZero check.
if core.IsAnyNilOrZero(req.ValidationMethod, req.AccountURIID) {
return nil, berrors.InternalServerError("incomplete IsCAAValid request")
}

ident := identifier.NewDNS(req.Domain)
if req.GetIdentifier() != nil {
ident = identifier.FromProto(req.GetIdentifier())
}

if ident.Type != identifier.TypeDNS {
return nil, berrors.MalformedError("Identifier type for CAA check was not DNS")
}

logEvent := validationLogEvent{
AuthzID: req.AuthzID,
Requester: req.AccountURIID,
Identifier: identifier.NewDNS(req.Domain),
Identifier: ident,
}

challType := core.AcmeChallenge(req.ValidationMethod)
if !challType.IsValid() {
return nil, berrors.InternalServerError("unrecognized validation method %q", req.ValidationMethod)
}

acmeID := identifier.NewDNS(req.Domain)
params := &caaParams{
accountURIID: req.AccountURIID,
validationMethod: challType,
Expand Down Expand Up @@ -89,15 +100,15 @@ func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAVal
va.log.AuditObject("CAA check result", logEvent)
}()

internalErr = va.checkCAA(ctx, acmeID, params)
internalErr = va.checkCAA(ctx, ident, params)

// Stop the clock for local check latency.
localLatency = va.clk.Since(start)

if internalErr != nil {
logEvent.InternalError = internalErr.Error()
prob = detailedError(internalErr)
prob.Detail = fmt.Sprintf("While processing CAA for %s: %s", req.Domain, prob.Detail)
prob.Detail = fmt.Sprintf("While processing CAA for %s: %s", req.Identifier.Value, prob.Detail)
}

if va.isPrimaryVA() {
Expand All @@ -114,7 +125,7 @@ func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAVal
if remoteProb != nil {
prob = remoteProb
va.log.Infof("CAA check failed due to remote failures: identifier=%v err=%s",
req.Domain, remoteProb)
req.Identifier.Value, remoteProb)
}
}

Expand Down Expand Up @@ -143,19 +154,19 @@ func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAVal
// the CAA lookup & validation fail a problem is returned.
func (va *ValidationAuthorityImpl) checkCAA(
ctx context.Context,
identifier identifier.ACMEIdentifier,
ident identifier.ACMEIdentifier,
params *caaParams) error {
if core.IsAnyNilOrZero(params, params.validationMethod, params.accountURIID) {
return errors.New("expected validationMethod or accountURIID not provided to checkCAA")
}

foundAt, valid, response, err := va.checkCAARecords(ctx, identifier, params)
foundAt, valid, response, err := va.checkCAARecords(ctx, ident, params)
if err != nil {
return berrors.DNSError("%s", err)
}

va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %d, Challenge: %s, Valid for issuance: %t, Found at: %q] Response=%q",
identifier.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt, response)
ident.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt, response)
if !valid {
return berrors.CAAError("CAA record for %s prevents issuance", foundAt)
}
Expand Down Expand Up @@ -299,13 +310,13 @@ func (va *ValidationAuthorityImpl) getCAA(ctx context.Context, hostname string)
// value (or nil).
func (va *ValidationAuthorityImpl) checkCAARecords(
ctx context.Context,
identifier identifier.ACMEIdentifier,
ident identifier.ACMEIdentifier,
params *caaParams) (string, bool, string, error) {
hostname := strings.ToLower(identifier.Value)
hostname := strings.ToLower(ident.Value)
// If this is a wildcard name, remove the prefix
var wildcard bool
if strings.HasPrefix(hostname, `*.`) {
hostname = strings.TrimPrefix(identifier.Value, `*.`)
hostname = strings.TrimPrefix(ident.Value, `*.`)
wildcard = true
}
caaSet, err := va.getCAA(ctx, hostname)
Expand Down
Loading