crypto/x509: ParseCRL allows revocation serial number that is a non-positive integer

[onepeople158]

Go version

go version go1.24.2 linux/amd64

Output of go env in your module/workspace:

CRL Issuer: CN=My Root CA,OU=My Root CA,O=My Company,L=San Francisco,ST=California,C=US
This Update Time: 2025-01-01 08:00:00 +0800 CST
Next Update Time: 2025-12-01 08:00:00 +0800 CST
Signature Algorithm: SHA256-RSA
Number of Revoked Certificates: 1

Revoked Entry Details:
============================
Serial Number: -24
Revocation Time: 2025-04-14 20:00:00 +0800 CST
  Extensions:
    2.5.29.21 (CRL Reason): KeyCompromise
----------------------------
CRL Issuer: CN=My Root CA,OU=My Root CA,O=My Company,L=San Francisco,ST=California,C=US
This Update Time: 2025-01-01 08:00:00 +0800 CST
Next Update Time: 2025-12-01 08:00:00 +0800 CST
Signature Algorithm: SHA256-RSA
Number of Revoked Certificates: 1

Revoked Entry Details:
============================
Serial Number: 0
Revocation Time: 2025-04-14 20:00:00 +0800 CST
  Extensions:
    2.5.29.21 (CRL Reason): KeyCompromise
----------------------------

What did you do?

Hello developer, Go successfully parsed a CRL file with revoked certificate serial numbers 0 and -36 without any errors. According to RFC5280, the certificate serial number must be a positive integer.

What did you see happen?

Go successfully parsed a CRL file with revoked certificate serial numbers 0 and -36 without any errors.

What did you expect to see?

Test Case:

go_certs.zip

[gabyhelp]

Related Issues

• crypto/x509: ParseRevocationList incorrect handling of the CRL Number field #73029
• crypto/x509: ParseRevocationList accepts authorityCertSerialNumber set to 0 #73293
• affected/package: x509 CreateRevocationList incorrect serial? #53923 (closed)
• crypto/x509: wrong value of RevocationList.AuthorityKeyId #67571 (closed)
• crypto/x509: Verify allows serialNumber larger than 20 octets #72076
• crypto/x509: the ParseRevocationList() doesn't seem to populate the AuthorityKeyId correctly #57461 (closed)
• crypto/x509: CreateRevocationList() does not enforce that the CRL Number is at most 20 octets #53543 (closed)
• crypto/x509: RevocationList.CheckSignatureFrom doesn't check Subject #60728
• crypto/x509: ParseRevocationList accepts invalid AKI extension in CRL #73030

Related Code Changes

• crypto/x509: don't create certs with negative serials

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[prattmic]

cc @FiloSottile @rolandshoemaker @cpu @golang/security

[FiloSottile]

Is this different from #73029?

[onepeople158]

Is this different from #73029?

Hello, developer. I initiated this report because I am unsure whether the CRL Number and Revoked serial use the same parameter type.

[cpu]

I think it's reasonable to tighten this up, but ParseRevocationList should probably mimick x509.ParseCertificate and respect the x509negativeserial godebug flag. As long as there's an escape hatch to parse a cert with a negative serial I think it's reasonable to allow the same cert to appear in a CRL.

[cpu]

Without any deep attachment to a particular outcome, here's a WIP in case it helps discussion: https://go-review.googlesource.com/c/go/+/666695

ParseRevocationList should probably mimick x509.ParseCertificate and respect the x509negativeserial godebug flag.

I convinced myself a new GODEBUG is required here in order to properly reflect the version that the behavior changed, and to not muddy the waters with the metric associated for the Certificate context. I also think it's sensible to make CreateRevocationList err for negative serials unless toggled otherwise.

[rolandshoemaker]

I convinced myself a new GODEBUG is required here in order to properly reflect the version that the behavior changed, and to not muddy the waters with the metric associated for the Certificate context. I also think it's sensible to make CreateRevocationList err for negative serials unless toggled otherwise.

This seems correct to me.

[AGWA]

This seems like a bad idea, as it would put CAs who misissue a certificate with a non-positive serial number between a rock and a hard place. If they revoke the certificate, their CRL won't be parseable by a major implementation by default. If they don't revoke the certificate, they would be violating the BRs and may also be putting RPs which accept certificates with non-positive serial numbers at risk (consider that the certificate may also have more serious problems, like improper DCV).

Given that the prohibition against non-positive serial numbers is in Section 4, which describes certificates, and there is no corresponding prohibition in Section 5, which describes CRLs, it's not clear to me that it's even a violation of RFC 5280 for CAs to put non-positive serial numbers in a CRL's revokedCertificates sequence. (It's true that Section 5 refers to "the ASN.1 in Section 4.1" for the definition of CertificateSerialNumber but the ASN.1 just defines CertificateSerialNumber as an INTEGER; the prohibition against non-positive serial numbers in Section 4.1.2.2 applies to just the TBSCertificate field rather than all instances of the CertificateSerialNumber type.)

Even if it were a violation of RFC 5280, I don't think the usual arguments in favor of strict parsing apply here. Non-positive serial numbers are still syntactically valid ASN.1 INTEGERs, so it's unlikely Go would interpret them differently from other implementations. Preventing badness from spreading in the ecosystem is better enforced during certificate parsing. Enforcing it in CRL parsing doesn't discourage CAs from issuing certificates with non-positive serial numbers; it just discourages CAs from revoking them.

Are there arguments in favor of this that I'm overlooking?

[rolandshoemaker]

Hm, this is a good point that I didn't consider.

We don't really support doing anything with CRLs currently (as in, using them for revocation checking in crypto/x509 itself) but plausibly other people could be implementing that themselves. We probably shouldn't be preventing malformed certificates from being included in CRLs. Similarly we probably shouldn't be preventing CRLs from being created with said serials.

Really wish CRLs hadn't used serial as the identifier, but whatever.