Skip to content

crl: reject invalid serial numbers in gencrl input - #1459

Open
RaphaelFakhri wants to merge 1 commit into
cloudflare:masterfrom
RaphaelFakhri:crl-reject-invalid-serials
Open

RaphaelFakhri wants to merge 1 commit into
cloudflare:masterfrom
RaphaelFakhri:crl-reject-invalid-serials

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

NewCRLFromFile parses each line of the cfssl gencrl serial list with big.Int.SetString(value, 10) but ignores whether parsing succeeded. A line that is not plain decimal produces a CRL entry for a different serial number, and the command exits 0:

The certificate you meant to revoke stays valid.

Changes

  • Trim whitespace around each line before parsing, so CRLF files and padded lines keep working.
  • Return an error such as invalid serial number "1a2b": serial numbers must be decimal when a line does not parse.
  • Add TestNewCRLFromFileInvalidSerial and TestNewCRLFromFileTrimsWhitespace.

Input format is unchanged: serial numbers are still decimal.

Testing

go test ./crl/ ./cli/gencrl/ ./api/crl/
golangci-lint run ./crl/...

Without the crl/crl.go change, both new tests fail (five invalid-serial cases accepted, padded serials parsed as 0). With it, all tests pass.

Fixes #1458

NewCRLFromFile ignored the result of big.Int.SetString, so a serial
number that is not plain decimal (for example the hex form printed by
openssl, or a line with leading whitespace) was added to the CRL as a
different serial number without any error. A hex serial such as 1a2b
revoked serial 1.

Trim whitespace around each line and return an error when a line is not
a valid decimal serial number.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gencrl silently revokes the wrong serial number when a line is not plain decimal

1 participant