diff --git a/api/next/50674.txt b/api/next/50674.txt new file mode 100644 index 0000000000..6b5bca3a9d --- /dev/null +++ b/api/next/50674.txt @@ -0,0 +1,9 @@ +pkg crypto/x509, func ParseRevocationList([]uint8) (*RevocationList, error) #50674 +pkg crypto/x509, method (*RevocationList) CheckSignatureFrom(*Certificate) error #50674 +pkg crypto/x509, type RevocationList struct, AuthorityKeyId []uint8 #50674 +pkg crypto/x509, type RevocationList struct, Extensions []pkix.Extension #50674 +pkg crypto/x509, type RevocationList struct, Issuer pkix.Name #50674 +pkg crypto/x509, type RevocationList struct, Raw []uint8 #50674 +pkg crypto/x509, type RevocationList struct, RawIssuer []uint8 #50674 +pkg crypto/x509, type RevocationList struct, RawTBSRevocationList []uint8 #50674 +pkg crypto/x509, type RevocationList struct, Signature []uint8 #50674 diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go index 333991bf14..9dd7c2564e 100644 --- a/src/crypto/x509/parser.go +++ b/src/crypto/x509/parser.go @@ -164,53 +164,29 @@ func parseAI(der cryptobyte.String) (pkix.AlgorithmIdentifier, error) { return ai, nil } -func parseValidity(der cryptobyte.String) (time.Time, time.Time, error) { - extract := func() (time.Time, error) { - var t time.Time - switch { - case der.PeekASN1Tag(cryptobyte_asn1.UTCTime): - // TODO(rolandshoemaker): once #45411 is fixed, the following code - // should be replaced with a call to der.ReadASN1UTCTime. - var utc cryptobyte.String - if !der.ReadASN1(&utc, cryptobyte_asn1.UTCTime) { - return t, errors.New("x509: malformed UTCTime") - } - s := string(utc) - - formatStr := "0601021504Z0700" - var err error - t, err = time.Parse(formatStr, s) - if err != nil { - formatStr = "060102150405Z0700" - t, err = time.Parse(formatStr, s) - } - if err != nil { - return t, err - } - - if serialized := t.Format(formatStr); serialized != s { - return t, errors.New("x509: malformed UTCTime") - } - - if t.Year() >= 2050 { - // UTCTime only encodes times prior to 2050. See https://tools.ietf.org/html/rfc5280#section-4.1.2.5.1 - t = t.AddDate(-100, 0, 0) - } - case der.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime): - if !der.ReadASN1GeneralizedTime(&t) { - return t, errors.New("x509: malformed GeneralizedTime") - } - default: - return t, errors.New("x509: unsupported time format") +func parseTime(der *cryptobyte.String) (time.Time, error) { + var t time.Time + switch { + case der.PeekASN1Tag(cryptobyte_asn1.UTCTime): + if !der.ReadASN1UTCTime(&t) { + return t, errors.New("x509: malformed UTCTime") } - return t, nil + case der.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime): + if !der.ReadASN1GeneralizedTime(&t) { + return t, errors.New("x509: malformed GeneralizedTime") + } + default: + return t, errors.New("x509: unsupported time format") } + return t, nil +} - notBefore, err := extract() +func parseValidity(der cryptobyte.String) (time.Time, time.Time, error) { + notBefore, err := parseTime(&der) if err != nil { return time.Time{}, time.Time{}, err } - notAfter, err := extract() + notAfter, err := parseTime(&der) if err != nil { return time.Time{}, time.Time{}, err } @@ -1011,3 +987,164 @@ func ParseCertificates(der []byte) ([]*Certificate, error) { } return certs, nil } + +// The X.509 standards confusingly 1-indexed the version names, but 0-indexed +// the actual encoded version, so the version for X.509v2 is 1. +const x509v2Version = 1 + +// ParseRevocationList parses a X509 v2 Certificate Revocation List from the given +// ASN.1 DER data. +func ParseRevocationList(der []byte) (*RevocationList, error) { + rl := &RevocationList{} + + input := cryptobyte.String(der) + // we read the SEQUENCE including length and tag bytes so that + // we can populate RevocationList.Raw, before unwrapping the + // SEQUENCE so it can be operated on + if !input.ReadASN1Element(&input, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed certificate") + } + rl.Raw = input + if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed certificate") + } + + var tbs cryptobyte.String + // do the same trick again as above to extract the raw + // bytes for Certificate.RawTBSCertificate + if !input.ReadASN1Element(&tbs, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed tbs certificate") + } + rl.RawTBSRevocationList = tbs + if !tbs.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed tbs certificate") + } + + var version int + if !tbs.PeekASN1Tag(cryptobyte_asn1.INTEGER) { + return nil, errors.New("x509: unsupported crl version") + } + if !tbs.ReadASN1Integer(&version) { + return nil, errors.New("x509: malformed crl") + } + if version != x509v2Version { + return nil, fmt.Errorf("x509: unsupported crl version: %d", version) + } + + var sigAISeq cryptobyte.String + if !tbs.ReadASN1(&sigAISeq, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed signature algorithm identifier") + } + // Before parsing the inner algorithm identifier, extract + // the outer algorithm identifier and make sure that they + // match. + var outerSigAISeq cryptobyte.String + if !input.ReadASN1(&outerSigAISeq, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed algorithm identifier") + } + if !bytes.Equal(outerSigAISeq, sigAISeq) { + return nil, errors.New("x509: inner and outer signature algorithm identifiers don't match") + } + sigAI, err := parseAI(sigAISeq) + if err != nil { + return nil, err + } + rl.SignatureAlgorithm = getSignatureAlgorithmFromAI(sigAI) + + var signature asn1.BitString + if !input.ReadASN1BitString(&signature) { + return nil, errors.New("x509: malformed signature") + } + rl.Signature = signature.RightAlign() + + var issuerSeq cryptobyte.String + if !tbs.ReadASN1Element(&issuerSeq, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed issuer") + } + rl.RawIssuer = issuerSeq + issuerRDNs, err := parseName(issuerSeq) + if err != nil { + return nil, err + } + rl.Issuer.FillFromRDNSequence(issuerRDNs) + + rl.ThisUpdate, err = parseTime(&tbs) + if err != nil { + return nil, err + } + if tbs.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime) || tbs.PeekASN1Tag(cryptobyte_asn1.UTCTime) { + rl.NextUpdate, err = parseTime(&tbs) + if err != nil { + return nil, err + } + } + + if tbs.PeekASN1Tag(cryptobyte_asn1.SEQUENCE) { + var revokedSeq cryptobyte.String + if !tbs.ReadASN1(&revokedSeq, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed crl") + } + for !revokedSeq.Empty() { + var certSeq cryptobyte.String + if !revokedSeq.ReadASN1(&certSeq, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed crl") + } + rc := pkix.RevokedCertificate{} + rc.SerialNumber = new(big.Int) + if !certSeq.ReadASN1Integer(rc.SerialNumber) { + return nil, errors.New("x509: malformed serial number") + } + rc.RevocationTime, err = parseTime(&certSeq) + if err != nil { + return nil, err + } + var extensions cryptobyte.String + var present bool + if !tbs.ReadOptionalASN1(&extensions, &present, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed extensions") + } + if present { + if !extensions.ReadASN1(&extensions, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed extensions") + } + for !extensions.Empty() { + var extension cryptobyte.String + if !extensions.ReadASN1(&extension, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed extension") + } + ext, err := parseExtension(extension) + if err != nil { + return nil, err + } + rc.Extensions = append(rc.Extensions, ext) + } + } + + rl.RevokedCertificates = append(rl.RevokedCertificates, rc) + } + } + + var extensions cryptobyte.String + var present bool + if !tbs.ReadOptionalASN1(&extensions, &present, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific()) { + return nil, errors.New("x509: malformed extensions") + } + if present { + if !extensions.ReadASN1(&extensions, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed extensions") + } + for !extensions.Empty() { + var extension cryptobyte.String + if !extensions.ReadASN1(&extension, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed extension") + } + ext, err := parseExtension(extension) + if err != nil { + return nil, err + } + rl.Extensions = append(rl.Extensions, ext) + } + } + + return rl, nil +} diff --git a/src/crypto/x509/pkix/pkix.go b/src/crypto/x509/pkix/pkix.go index e9179ed067..da57b66831 100644 --- a/src/crypto/x509/pkix/pkix.go +++ b/src/crypto/x509/pkix/pkix.go @@ -296,6 +296,8 @@ func (certList *CertificateList) HasExpired(now time.Time) bool { // TBSCertificateList represents the ASN.1 structure of the same name. See RFC // 5280, section 5.1. +// +// Deprecated: x509.RevocationList should be used instead. type TBSCertificateList struct { Raw asn1.RawContent Version int `asn1:"optional,default:0"` @@ -309,6 +311,8 @@ type TBSCertificateList struct { // RevokedCertificate represents the ASN.1 structure of the same name. See RFC // 5280, section 5.1. +// +// Deprecated: x509.RevocationList should be used instead. type RevokedCertificate struct { SerialNumber *big.Int RevocationTime time.Time diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index cb43079a9c..41d85c458d 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -880,6 +880,8 @@ func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey } // CheckCRLSignature checks that the signature in crl is from c. +// +// Deprecated: Use RevocationList.CheckSignatureFrom instead. func (c *Certificate) CheckCRLSignature(crl *pkix.CertificateList) error { algo := getSignatureAlgorithmFromAI(crl.SignatureAlgorithm) return c.CheckSignature(algo, crl.TBSCertList.Raw, crl.SignatureValue.RightAlign()) @@ -1607,6 +1609,8 @@ var pemType = "X509 CRL" // encoded CRLs will appear where they should be DER encoded, so this function // will transparently handle PEM encoding as long as there isn't any leading // garbage. +// +// Deprecated: Use ParseRevocationList instead. func ParseCRL(crlBytes []byte) (*pkix.CertificateList, error) { if bytes.HasPrefix(crlBytes, pemCRLPrefix) { block, _ := pem.Decode(crlBytes) @@ -1618,6 +1622,8 @@ func ParseCRL(crlBytes []byte) (*pkix.CertificateList, error) { } // ParseDERCRL parses a DER encoded CRL from the given bytes. +// +// Deprecated: Use ParseRevocationList instead. func ParseDERCRL(derBytes []byte) (*pkix.CertificateList, error) { certList := new(pkix.CertificateList) if rest, err := asn1.Unmarshal(derBytes, certList); err != nil { @@ -1631,7 +1637,7 @@ func ParseDERCRL(derBytes []byte) (*pkix.CertificateList, error) { // CreateCRL returns a DER encoded CRL, signed by this Certificate, that // contains the given list of revoked certificates. // -// Note: this method does not generate an RFC 5280 conformant X.509 v2 CRL. +// Deprecated: this method does not generate an RFC 5280 conformant X.509 v2 CRL. // To generate a standards compliant CRL, use CreateRevocationList instead. func (c *Certificate) CreateCRL(rand io.Reader, priv any, revokedCerts []pkix.RevokedCertificate, now, expiry time.Time) (crlBytes []byte, err error) { key, ok := priv.(crypto.Signer) @@ -2073,6 +2079,14 @@ func (c *CertificateRequest) CheckSignature() error { // RevocationList contains the fields used to create an X.509 v2 Certificate // Revocation list with CreateRevocationList. type RevocationList struct { + Raw []byte + RawTBSRevocationList []byte + RawIssuer []byte + + Issuer pkix.Name + AuthorityKeyId []byte + + Signature []byte // SignatureAlgorithm is used to determine the signature algorithm to be // used when signing the CRL. If 0 the default algorithm for the signing // key will be used. @@ -2087,6 +2101,7 @@ type RevocationList struct { // which should be a monotonically increasing sequence number for a given // CRL scope and CRL issuer. Number *big.Int + // ThisUpdate is used to populate the thisUpdate field in the CRL, which // indicates the issuance date of the CRL. ThisUpdate time.Time @@ -2094,6 +2109,11 @@ type RevocationList struct { // indicates the date by which the next CRL will be issued. NextUpdate // must be greater than ThisUpdate. NextUpdate time.Time + + // Extensions contains raw X.509 extensions. When creating a CRL, + // the Extensions field is ignored, see ExtraExtensions. + Extensions []pkix.Extension + // ExtraExtensions contains any additional extensions to add directly to // the CRL. ExtraExtensions []pkix.Extension @@ -2207,3 +2227,22 @@ func CreateRevocationList(rand io.Reader, template *RevocationList, issuer *Cert SignatureValue: asn1.BitString{Bytes: signature, BitLength: len(signature) * 8}, }) } + +// CheckSignatureFrom verifies that the signature on rl is a valid signature +// from issuer. +func (rl *RevocationList) CheckSignatureFrom(parent *Certificate) error { + if parent.Version == 3 && !parent.BasicConstraintsValid || + parent.BasicConstraintsValid && !parent.IsCA { + return ConstraintViolationError{} + } + + if parent.KeyUsage != 0 && parent.KeyUsage&KeyUsageCRLSign == 0 { + return ConstraintViolationError{} + } + + if parent.PublicKeyAlgorithm == UnknownPublicKeyAlgorithm { + return ErrUnsupportedAlgorithm + } + + return parent.CheckSignature(rl.SignatureAlgorithm, rl.RawTBSRevocationList, rl.Signature) +} diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index d2889fc1d7..b26ace89e7 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -2631,24 +2631,24 @@ func TestCreateRevocationList(t *testing.T) { return } - parsedCRL, err := ParseDERCRL(crl) + parsedCRL, err := ParseRevocationList(crl) if err != nil { t.Fatalf("Failed to parse generated CRL: %s", err) } if tc.template.SignatureAlgorithm != UnknownSignatureAlgorithm && - parsedCRL.SignatureAlgorithm.Algorithm.Equal(signatureAlgorithmDetails[tc.template.SignatureAlgorithm].oid) { + parsedCRL.SignatureAlgorithm != tc.template.SignatureAlgorithm { t.Fatalf("SignatureAlgorithm mismatch: got %v; want %v.", parsedCRL.SignatureAlgorithm, tc.template.SignatureAlgorithm) } - if !reflect.DeepEqual(parsedCRL.TBSCertList.RevokedCertificates, tc.template.RevokedCertificates) { + if !reflect.DeepEqual(parsedCRL.RevokedCertificates, tc.template.RevokedCertificates) { t.Fatalf("RevokedCertificates mismatch: got %v; want %v.", - parsedCRL.TBSCertList.RevokedCertificates, tc.template.RevokedCertificates) + parsedCRL.RevokedCertificates, tc.template.RevokedCertificates) } - if len(parsedCRL.TBSCertList.Extensions) != 2+len(tc.template.ExtraExtensions) { - t.Fatalf("Generated CRL has wrong number of extensions, wanted: %d, got: %d", 2+len(tc.template.ExtraExtensions), len(parsedCRL.TBSCertList.Extensions)) + if len(parsedCRL.Extensions) != 2+len(tc.template.ExtraExtensions) { + t.Fatalf("Generated CRL has wrong number of extensions, wanted: %d, got: %d", 2+len(tc.template.ExtraExtensions), len(parsedCRL.Extensions)) } expectedAKI, err := asn1.Marshal(authKeyId{Id: tc.issuer.SubjectKeyId}) if err != nil { @@ -2658,9 +2658,9 @@ func TestCreateRevocationList(t *testing.T) { Id: oidExtensionAuthorityKeyId, Value: expectedAKI, } - if !reflect.DeepEqual(parsedCRL.TBSCertList.Extensions[0], akiExt) { + if !reflect.DeepEqual(parsedCRL.Extensions[0], akiExt) { t.Fatalf("Unexpected first extension: got %v, want %v", - parsedCRL.TBSCertList.Extensions[0], akiExt) + parsedCRL.Extensions[0], akiExt) } expectedNum, err := asn1.Marshal(tc.template.Number) if err != nil { @@ -2670,18 +2670,18 @@ func TestCreateRevocationList(t *testing.T) { Id: oidExtensionCRLNumber, Value: expectedNum, } - if !reflect.DeepEqual(parsedCRL.TBSCertList.Extensions[1], crlExt) { + if !reflect.DeepEqual(parsedCRL.Extensions[1], crlExt) { t.Fatalf("Unexpected second extension: got %v, want %v", - parsedCRL.TBSCertList.Extensions[1], crlExt) + parsedCRL.Extensions[1], crlExt) } - if len(parsedCRL.TBSCertList.Extensions[2:]) == 0 && len(tc.template.ExtraExtensions) == 0 { + if len(parsedCRL.Extensions[2:]) == 0 && len(tc.template.ExtraExtensions) == 0 { // If we don't have anything to check return early so we don't // hit a [] != nil false positive below. return } - if !reflect.DeepEqual(parsedCRL.TBSCertList.Extensions[2:], tc.template.ExtraExtensions) { + if !reflect.DeepEqual(parsedCRL.Extensions[2:], tc.template.ExtraExtensions) { t.Fatalf("Extensions mismatch: got %v; want %v.", - parsedCRL.TBSCertList.Extensions[2:], tc.template.ExtraExtensions) + parsedCRL.Extensions[2:], tc.template.ExtraExtensions) } }) } @@ -3444,3 +3444,125 @@ func TestDisableSHA1ForCertOnly(t *testing.T) { t.Errorf("unexpected error: %s", err) } } + +func TestParseRevocationList(t *testing.T) { + derBytes := fromBase64(derCRLBase64) + certList, err := ParseRevocationList(derBytes) + if err != nil { + t.Errorf("error parsing: %s", err) + return + } + numCerts := len(certList.RevokedCertificates) + expected := 88 + if numCerts != expected { + t.Errorf("bad number of revoked certificates. got: %d want: %d", numCerts, expected) + } +} + +func TestRevocationListCheckSignatureFrom(t *testing.T) { + goodKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate test key: %s", err) + } + badKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate test key: %s", err) + } + tests := []struct { + name string + issuer *Certificate + err string + }{ + { + name: "valid", + issuer: &Certificate{ + Version: 3, + BasicConstraintsValid: true, + IsCA: true, + PublicKeyAlgorithm: ECDSA, + PublicKey: goodKey.Public(), + }, + }, + { + name: "valid, key usage set", + issuer: &Certificate{ + Version: 3, + BasicConstraintsValid: true, + IsCA: true, + PublicKeyAlgorithm: ECDSA, + PublicKey: goodKey.Public(), + KeyUsage: KeyUsageCRLSign, + }, + }, + { + name: "invalid issuer, wrong key usage", + issuer: &Certificate{ + Version: 3, + BasicConstraintsValid: true, + IsCA: true, + PublicKeyAlgorithm: ECDSA, + PublicKey: goodKey.Public(), + KeyUsage: KeyUsageCertSign, + }, + err: "x509: invalid signature: parent certificate cannot sign this kind of certificate", + }, + { + name: "invalid issuer, no basic constraints/ca", + issuer: &Certificate{ + Version: 3, + PublicKeyAlgorithm: ECDSA, + PublicKey: goodKey.Public(), + }, + err: "x509: invalid signature: parent certificate cannot sign this kind of certificate", + }, + { + name: "invalid issuer, unsupported public key type", + issuer: &Certificate{ + Version: 3, + BasicConstraintsValid: true, + IsCA: true, + PublicKeyAlgorithm: UnknownPublicKeyAlgorithm, + PublicKey: goodKey.Public(), + }, + err: "x509: cannot verify signature: algorithm unimplemented", + }, + { + name: "wrong key", + issuer: &Certificate{ + Version: 3, + BasicConstraintsValid: true, + IsCA: true, + PublicKeyAlgorithm: ECDSA, + PublicKey: badKey.Public(), + }, + err: "x509: ECDSA verification failure", + }, + } + + crlIssuer := &Certificate{ + BasicConstraintsValid: true, + IsCA: true, + PublicKeyAlgorithm: ECDSA, + PublicKey: goodKey.Public(), + KeyUsage: KeyUsageCRLSign, + SubjectKeyId: []byte{1, 2, 3}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + crlDER, err := CreateRevocationList(rand.Reader, &RevocationList{Number: big.NewInt(1)}, crlIssuer, goodKey) + if err != nil { + t.Fatalf("failed to generate CRL: %s", err) + } + crl, err := ParseRevocationList(crlDER) + if err != nil { + t.Fatalf("failed to parse test CRL: %s", err) + } + err = crl.CheckSignatureFrom(tc.issuer) + if err != nil && err.Error() != tc.err { + t.Errorf("unexpected error: got %s, want %s", err, tc.err) + } else if err == nil && tc.err != "" { + t.Errorf("CheckSignatureFrom did not fail: want %s", tc.err) + } + }) + } +}