Skip to content

crypto/x509: extension parsers ignore trailing data in extension values #81798

Closed

Description

@keithadler

Go version

go version go1.26.6 darwin/arm64

Output of go env in your module/workspace:

GOARCH='arm64'
GOOS='darwin'

What did you do?

ParseCertificate accepts certificates whose keyUsage, basicConstraints, subjectAltName, extKeyUsage or certificatePolicies extension value has bytes after the DER value, or, for basicConstraints, unexpected elements inside the SEQUENCE. The extension parsers read the leading value and drop the rest. For example, parseBasicConstraintsExtension does der.ReadASN1(&der, cryptobyte_asn1.SEQUENCE) and never checks that the input, or the SEQUENCE body after the optional BOOLEAN and INTEGER, is empty. parseSANExtension (via forEachSAN), parseExtKeyUsageExtension and parseCertificatePoliciesExtension follow the same pattern, and parseKeyUsageExtension does not check for data after the BIT STRING.

Reproducer:

package main

import (
	"crypto/ecdsa"
	"crypto/elliptic"
	"crypto/rand"
	"crypto/x509"
	"crypto/x509/pkix"
	"encoding/asn1"
	"fmt"
	"math/big"
	"time"
)

func main() {
	key, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
	for _, value := range [][]byte{
		{0x30, 0x03, 0x01, 0x01, 0xff},                               // SEQUENCE { TRUE }: well formed
		{0x30, 0x03, 0x01, 0x01, 0xff, 0x00},                         // a byte after the SEQUENCE
		{0x30, 0x06, 0x01, 0x01, 0xff, 0x04, 0x01, 0x00},             // an OCTET STRING inside it
		{0x30, 0x08, 0x01, 0x01, 0xff, 0x02, 0x01, 0x00, 0x05, 0x00}, // a NULL after pathLenConstraint
	} {
		tmpl := &x509.Certificate{
			SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "ca"},
			NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(time.Hour),
			KeyUsage:        x509.KeyUsageCertSign,
			ExtraExtensions: []pkix.Extension{{Id: asn1.ObjectIdentifier{2, 5, 29, 19}, Critical: true, Value: value}},
		}
		der, _ := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key)
		c, err := x509.ParseCertificate(der)
		fmt.Printf("% x: err=%v IsCA=%v\n", value, err, err == nil && c.IsCA)
	}
}

What did you see happen?

30 03 01 01 ff: err=<nil> IsCA=true
30 03 01 01 ff 00: err=<nil> IsCA=true
30 06 01 01 ff 04 01 00: err=<nil> IsCA=true
30 08 01 01 ff 02 01 00 05 00: err=<nil> IsCA=true

The same happens with a trailing 0x00 appended to a keyUsage, subjectAltName, extKeyUsage or certificatePolicies value.

What did you expect to see?

A parse error for the last three basicConstraints values, and for the other extensions with trailing data: an extension's extnValue is the DER encoding of exactly one value (RFC 5280 §4.1), and parseCertificate already rejects trailing data at the certificate and TBSCertificate level (x509: trailing data). Python cryptography rejects all of these ("extra data").

This isn't a security issue as far as I can tell, since the extra bytes are inside the signed TBSCertificate, so only the issuing CA can produce them. The practical effect is that one certificate's meaning can be written in several byte strings, which DER is meant to rule out. A fix would be an Empty() check after each extension value (and after the optional fields inside basicConstraints).

Found with a cross-library X.509 test corpus (https://github.com/keithadler/lean-x509/tree/main/test/crosscheck), where each certificate has one flaw and a valid signature. Written with the help of Claude.

Activity

  1. added
    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
    on Sep 29, 2026
  2. prattmic commented on Sep 29, 2026

    @prattmic
    Member

    cc @golang/security

  3. keithadler commented on Sep 29, 2026

    @keithadler
    Author

    Thanks. I missed #79860 when filing, and it overlaps: the basicConstraints-inside-SEQUENCE and keyUsage cases are already there, and I take @rolandshoemaker's point there about tolerating unknown elements at the end of a SEQUENCE.

    The part I think is distinct is bytes after the top-level value in extnValue, e.g. 30 03 01 01 ff 00 for basicConstraints, and the same trailing byte on subjectAltName, extKeyUsage and certificatePolicies. That isn't an extension of the SEQUENCE: extnValue is the DER encoding of a single value (RFC 5280 §4.2), and a lone 0x00 isn't a complete DER element. An Empty() check on the input after reading the outer value wouldn't touch the Appendix B extensibility case. Happy to have this folded into #79860 if that's easier to track.

  4. DeRuina commented on Oct 5, 2026

    @DeRuina
    Contributor

    @rolandshoemaker building on @keithadler distinction: this would be just an Empty() check on the extension input after reading the complete outer extnValue - rejecting a stray trailing byte like 30 03 01 01 ff 00 without touching the Appendix B extensibility case (extra elements inside the SEQUENCE stay tolerated). Happy to write that narrow fix + tests for keyUsage/basicConstraints/SAN/extKeyUsage/certificatePolicies — want a CL?

  5. rolandshoemaker commented on Oct 5, 2026

    @rolandshoemaker
    Member

    @DeRuina sure.

  6. gopherbot commented on Oct 6, 2026

    @gopherbot
    Contributor

    Change https://go.dev/cl/845765 mentions this issue: crypto/x509: reject trailing data in extension values

  7. DeRuina commented on Oct 6, 2026

    @DeRuina
    Contributor

    Hey, created the CL for this: https://go.dev/cl/845765. @rolandshoemaker added you as a reviewer :)

  8. added
    FixPendingIssues that have a fix which has not yet been reviewed or submitted.
    and removed
    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
    on Oct 7, 2026
  9. added this to the Go1.28 milestone on Oct 7, 2026
  10. added a commit that references this issue on Oct 7, 2026
    b724cdf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FixPendingIssues that have a fix which has not yet been reviewed or submitted.

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions