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

Allow CA interface to control validation of Client Identifiers for device-attest-01 acme requests #1525

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions acme/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Authorization struct {
Wildcard bool `json:"wildcard"`
ExpiresAt time.Time `json:"expires"`
Error *Error `json:"error,omitempty"`
ExtraIdentifiers []string `json:"extraIdentifiers,omitempty"`
}

// ToLog enables response logging.
Expand Down
10 changes: 6 additions & 4 deletions acme/challenge.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose
// identifiers.
//
// Note: We might want to use an external service for this.
if data.UDID != ch.Value && data.SerialNumber != ch.Value {
if !SkipPermanentIdentiferValidation && (data.UDID != ch.Value) && (data.SerialNumber != ch.Value) {
subproblem := NewSubproblemWithIdentifier(
ErrorRejectedIdentifierType,
Identifier{Type: "permanent-identifier", Value: ch.Value},
Expand Down Expand Up @@ -494,7 +494,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose
// certificate with the challenged Order value.
//
// Note: We might want to use an external service for this.
if data.SerialNumber != ch.Value {
if !SkipPermanentIdentiferValidation && (data.SerialNumber != ch.Value) {
subproblem := NewSubproblemWithIdentifier(
ErrorRejectedIdentifierType,
Identifier{Type: "permanent-identifier", Value: ch.Value},
Expand All @@ -505,6 +505,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose

// Update attestation key fingerprint to compare against the CSR
az.Fingerprint = data.Fingerprint
az.ExtraIdentifiers = []string{data.SerialNumber, p.AttObj}

case "tpm":
data, err := doTPMAttestationFormat(ctx, prov, ch, jwk, &att)
Expand All @@ -524,7 +525,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose
// haven't implemented a way for AK certs requested by the CLI to always contain the requested
// PermanentIdentifier. Omitting the check below doesn't allow just any request, as the Order can
// still fail if the challenge value isn't equal to the CSR subject.
if len(data.PermanentIdentifiers) > 0 && !slices.Contains(data.PermanentIdentifiers, ch.Value) { // TODO(hs): add support for HardwareModuleName
if (!SkipPermanentIdentiferValidation) && (len(data.PermanentIdentifiers) > 0) && (!slices.Contains(data.PermanentIdentifiers, ch.Value)) { // TODO(hs): add support for HardwareModuleName
subproblem := NewSubproblemWithIdentifier(
ErrorRejectedIdentifierType,
Identifier{Type: "permanent-identifier", Value: ch.Value},
Expand All @@ -535,6 +536,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose

// Update attestation key fingerprint to compare against the CSR
az.Fingerprint = data.Fingerprint

default:
return storeError(ctx, db, ch, true, NewDetailedError(ErrorBadAttestationStatementType, "unsupported attestation object format %q", format))
}
Expand All @@ -547,7 +549,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose
// Store the fingerprint in the authorization.
//
// TODO: add method to update authorization and challenge atomically.
if az.Fingerprint != "" {
if az.Fingerprint != "" || az.ExtraIdentifiers != nil {
if err := db.UpdateAuthorization(ctx, az); err != nil {
return WrapErrorISE(err, "error updating authorization")
}
Expand Down
7 changes: 7 additions & 0 deletions acme/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ func NewContext(ctx context.Context, db DB, client Client, linker Linker, fn Pre
return ctx
}

// SkipPermanentIdentiferValidation allows one to bypass the code which validates
// that a) that the attested UUID or serial for "device-attest-01" request match
// the permanent identifier and b) that the CN presented in the final CSRs match the permanent
// identifier. The logic to bypass this is to allow external CAs to control the validation temporarily
// Setting this at true at this global level is extremely discouraged (unless you know what you are doing)
var SkipPermanentIdentiferValidation bool

// PrerequisitesChecker is a function that checks if all prerequisites for
// serving ACME are met by the CA configuration.
type PrerequisitesChecker func(ctx context.Context) (bool, error)
Expand Down
5 changes: 5 additions & 0 deletions acme/db/nosql/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type dbAuthz struct {
CreatedAt time.Time `json:"createdAt"`
ExpiresAt time.Time `json:"expiresAt"`
Error *acme.Error `json:"error"`
ExtraIdentifiers []string `json:"extraIdentifiers,omitempty"`
}

func (ba *dbAuthz) clone() *dbAuthz {
Expand Down Expand Up @@ -71,6 +72,7 @@ func (db *DB) GetAuthorization(ctx context.Context, id string) (*acme.Authorizat
ExpiresAt: dbaz.ExpiresAt,
Token: dbaz.Token,
Fingerprint: dbaz.Fingerprint,
ExtraIdentifiers: dbaz.ExtraIdentifiers,
Error: dbaz.Error,
}, nil
}
Expand Down Expand Up @@ -101,6 +103,7 @@ func (db *DB) CreateAuthorization(ctx context.Context, az *acme.Authorization) e
Token: az.Token,
Fingerprint: az.Fingerprint,
Wildcard: az.Wildcard,
ExtraIdentifiers: nil,
}

return db.save(ctx, az.ID, dbaz, nil, "authz", authzTable)
Expand All @@ -116,6 +119,7 @@ func (db *DB) UpdateAuthorization(ctx context.Context, az *acme.Authorization) e
nu := old.clone()
nu.Status = az.Status
nu.Fingerprint = az.Fingerprint
nu.ExtraIdentifiers = az.ExtraIdentifiers
nu.Error = az.Error
return db.save(ctx, old.ID, nu, old, "authz", authzTable)
}
Expand Down Expand Up @@ -149,6 +153,7 @@ func (db *DB) GetAuthorizationsByAccountID(_ context.Context, accountID string)
Token: dbaz.Token,
Fingerprint: dbaz.Fingerprint,
Error: dbaz.Error,
ExtraIdentifiers: dbaz.ExtraIdentifiers,
})
}

Expand Down
23 changes: 22 additions & 1 deletion acme/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,21 @@ func (o *Order) getAuthorizationFingerprint(ctx context.Context, db DB) (string,
return "", nil
}

func (o *Order) getAuthorizationExtraIdentifiers(ctx context.Context, db DB) ([]string, error) {
for _, azID := range o.AuthorizationIDs {
az, err := db.GetAuthorization(ctx, azID)
if err != nil {
return nil, WrapErrorISE(err, "error getting authorization %q", azID)
}
// There's no point on reading all the authorizations as there will
// be only one for a permanent identifier.
if az.ExtraIdentifiers != nil {
return az.ExtraIdentifiers, nil
}
}
return nil, nil
}

// Finalize signs a certificate if the necessary conditions for Order completion
// have been met.
//
Expand Down Expand Up @@ -191,6 +206,11 @@ func (o *Order) Finalize(ctx context.Context, db DB, csr *x509.CertificateReques
}
}

extraIdentifiers, err := o.getAuthorizationExtraIdentifiers(ctx, db)
if err != nil {
return err
}

// canonicalize the CSR to allow for comparison
csr = canonicalize(csr)

Expand All @@ -211,7 +231,7 @@ func (o *Order) Finalize(ctx context.Context, db DB, csr *x509.CertificateReques
// is rejected, because the Common Name hasn't been challenged in that case. This
// could result in unauthorized access if a relying system relies on the Common
// Name in its authorization logic.
if csr.Subject.CommonName != "" && csr.Subject.CommonName != permanentIdentifier {
if !SkipPermanentIdentiferValidation && (csr.Subject.CommonName != "") && (csr.Subject.CommonName != permanentIdentifier) {
return NewError(ErrorBadCSRType, "CSR Subject Common Name does not match identifiers exactly: "+
"CSR Subject Common Name = %s, Order Permanent Identifier = %s", csr.Subject.CommonName, permanentIdentifier)
}
Expand All @@ -228,6 +248,7 @@ func (o *Order) Finalize(ctx context.Context, db DB, csr *x509.CertificateReques
})
extraOptions = append(extraOptions, provisioner.AttestationData{
PermanentIdentifier: permanentIdentifier,
ExtraIdentifiers: extraIdentifiers,
})
} else {
defaultTemplate = x509util.DefaultLeafTemplate
Expand Down
3 changes: 2 additions & 1 deletion authority/provisioner/sign_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ func (fn CertificateEnforcerFunc) Enforce(cert *x509.Certificate) error {
// AttestationData is a SignOption used to pass attestation information to the
// sign methods.
type AttestationData struct {
PermanentIdentifier string
PermanentIdentifier string //
ExtraIdentifiers []string
}

// defaultPublicKeyValidator validates the public key of a certificate request.
Expand Down
13 changes: 13 additions & 0 deletions authority/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,17 @@ func (a *Authority) signX509(ctx context.Context, csr *x509.CertificateRequest,
)
}

var clientIdentifier string
var extraIdentifiers []string
if attData != nil {
if attData.PermanentIdentifier != "" {
clientIdentifier = attData.PermanentIdentifier
}
if attData.ExtraIdentifiers != nil {
extraIdentifiers = attData.ExtraIdentifiers
}
}

// Sign certificate
lifetime := leaf.NotAfter.Sub(leaf.NotBefore.Add(signOpts.Backdate))

Expand All @@ -305,6 +316,8 @@ func (a *Authority) signX509(ctx context.Context, csr *x509.CertificateRequest,
Lifetime: lifetime,
Backdate: signOpts.Backdate,
Provisioner: pInfo,
ClientIdentifier: clientIdentifier,
ExtraIdentifiers: extraIdentifiers,
})
if err != nil {
return nil, prov, errs.Wrap(http.StatusInternalServerError, err, "authority.Sign; error creating certificate", opts...)
Expand Down
2 changes: 2 additions & 0 deletions cas/apiv1/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ type CreateCertificateRequest struct {
RequestID string
Provisioner *ProvisionerInfo
IsCAServerCert bool
ClientIdentifier string // any identifier token (can be any arbitrary token/blob interpretable by the CA)
ExtraIdentifiers []string // any additional identifier ( interpretable by the CA such as serial, UDID of device)
}

// ProvisionerInfo contains information of the provisioner used to authorize a
Expand Down