Webhook accepts duplicate dnsNames and ipAddresses, which fail ACME issuance with an opaque badCSR error
Describe the bug:
The webhook accepts a Certificate whose dnsNames or ipAddresses contain duplicates. With an ACME issuer such a Certificate never issues. The CSR carries one SAN per entry, the Order carries one identifier per distinct value, and the ACME server rejects the mismatch with badCSR / Unexpected number of identifiers found in CSR. Nothing tells the user which field is wrong.
#8978 fixed this for Certificates that ingress-shim generates. Every other producer is still affected: hand-written Certificates, Helm charts, GitOps templates and third-party controllers.
EvidenceGenerateCSRcopiesSpec.DNSNamesinto the CSR verbatim.- The ACME order controller collapses
DNSNamesandIPAddressesinto sets before building the order. validateIPAddresseschecks each address parses, but not that it is unique. There is no per-entry check fordnsNamesat all. The validation package already usesfield.DuplicateforadditionalOutputFormats, so there is a precedent to follow.
Expected behaviour:
The webhook should reject duplicate dnsNames and ipAddresses at admission with a field.Duplicate error, so the user sees the problem when they apply the object. IP addresses should be compared by parsed value so that two spellings of one IPv6 address count as a duplicate.
Please do not fix this by de-duplicating inside GenerateCSR alone. RequestMatchesSpec compares the CSR against the spec with EqualUnsorted, which is length-sensitive. A CSR with fewer SANs than the spec would never match, and the controller would loop recreating CertificateRequests.
Steps to reproduce the bug:
- Apply a Certificate with
dnsNames: [example.com, example.com]and an ACME issuer. - The webhook accepts it.
- The Order fails with
Unexpected number of identifiers found in CSR.
Anything else we need to know?:
Found during the pre-backport safety review of #8978: https://github.com/cert-manager/cert-manager/pull/8978#issuecomment-5497143708
Environment details:
- cert-manager version: master at f94d3ea.
/kind bug
[Claude Fable 5.1]
Source: cert-manager/cert-manager