From 094f0521e23ed1a8d3de98412fe872c032bc8f4f Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 24 Mar 2023 12:55:42 +0100 Subject: [PATCH] Remove check for `PermanentIdentifier` from `tpm` format validation --- acme/challenge.go | 29 +++++++++++++++++------------ go.mod | 3 +++ go.sum | 2 ++ 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/acme/challenge.go b/acme/challenge.go index 29113b03..54a7d7b9 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -28,8 +28,8 @@ import ( "github.com/fxamacker/cbor/v2" "github.com/google/go-attestation/attest" - x509ext "github.com/google/go-attestation/x509" "github.com/google/go-tpm/tpm2" + "github.com/ryboe/q" "golang.org/x/exp/slices" "go.step.sm/crypto/jose" @@ -420,6 +420,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose case "step": data, err := doStepAttestationFormat(ctx, prov, ch, jwk, &att) if err != nil { + q.Q(err) var acmeError *Error if errors.As(err, &acmeError) { if acmeError.Status == 500 { @@ -451,6 +452,7 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose if err != nil { // TODO(hs): we should provide more details in the error reported to the client; // "Attestation statement cannot be verified" is VERY generic. Also holds true for the other formats. + q.Q(err) var acmeError *Error if errors.As(err, &acmeError) { if acmeError.Status == 500 { @@ -598,6 +600,7 @@ func doTPMAttestationFormat(ctx context.Context, ch *Challenge, db DB, jwk *jose for _, ext := range akCert.Extensions { if ext.Id.Equal(oidSubjectAlternativeName) { sanExtension = ext + break } } @@ -605,15 +608,17 @@ func doTPMAttestationFormat(ctx context.Context, ch *Challenge, db DB, jwk *jose return nil, NewError(ErrorBadAttestationStatementType, "AK certificate is missing Subject Alternative Name extension") } - san, err := x509ext.ParseSubjectAltName(sanExtension) // TODO(hs): move to a package under our control? - if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "failed parsing Subject Alternative Name extension") - } + // TODO(hs): below code fails if there's a URI SAN, for example. Needs more complete parsing of SANS, + // or skip ASN1 tags that can't be parsed. + // san, err := x509ext.ParseSubjectAltName(sanExtension) // TODO(hs): move to a package under our control? + // if err != nil { + // return nil, WrapError(ErrorBadAttestationStatementType, err, "failed parsing Subject Alternative Name extension") + // } - var permanentIdentifiers = make([]string, len(san.PermanentIdentifiers)) - for i, p := range san.PermanentIdentifiers { - permanentIdentifiers[i] = p.IdentifierValue - } + // var permanentIdentifiers = make([]string, len(san.PermanentIdentifiers)) + // for i, p := range san.PermanentIdentifiers { + // permanentIdentifiers[i] = p.IdentifierValue + // } // TODO(hs): reenable this check when we want to enforce a PermanentIdentifier to be present in // the AK certificate. @@ -708,9 +713,9 @@ func doTPMAttestationFormat(ctx context.Context, ch *Challenge, db DB, jwk *jose } data := &tpmAttestationData{ - Certificate: akCert, - VerifiedChains: verifiedChains, - PermanentIdentifiers: permanentIdentifiers, + Certificate: akCert, + VerifiedChains: verifiedChains, + //PermanentIdentifiers: permanentIdentifiers, } if data.Fingerprint, err = keyutil.Fingerprint(publicKey); err != nil { diff --git a/go.mod b/go.mod index 5c8fb824..5b3374a4 100644 --- a/go.mod +++ b/go.mod @@ -53,6 +53,7 @@ require ( require ( github.com/google/go-attestation v0.4.4-0.20220404204839-8820d49b18d9 + github.com/ryboe/q v1.0.19 golang.org/x/exp v0.0.0-20230310171629-522b1b587ee0 ) @@ -111,6 +112,7 @@ require ( github.com/jackc/pgx/v4 v4.18.0 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/klauspost/compress v1.15.11 // indirect + github.com/kr/text v0.2.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/manifoldco/promptui v0.9.0 // indirect github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect @@ -121,6 +123,7 @@ require ( github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/rogpeppe/go-internal v1.9.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/ryanuber/go-glob v1.0.0 // indirect github.com/shopspring/decimal v1.2.0 // indirect diff --git a/go.sum b/go.sum index 965d1e69..81d934ec 100644 --- a/go.sum +++ b/go.sum @@ -845,6 +845,8 @@ github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= +github.com/ryboe/q v1.0.19 h1:1dO1anK4gorZRpXBD/edBZkMxIC1tFIwN03nfyOV13A= +github.com/ryboe/q v1.0.19/go.mod h1:IoEB3Q2/p6n1qbhIQVuNyakxtnV4rNJ/XJPK+jsEa0M= github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da/go.mod h1:gi+0XIa01GRL2eRQVjQkKGqKF3SF9vZR/HnPullcV2E= github.com/sassoftware/go-rpmutils v0.0.0-20190420191620-a8f1baeba37b/go.mod h1:am+Fp8Bt506lA3Rk3QCmSqmYmLMnPDhdDUcosQCAx+I= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=