Skip to content

Commit bae64fd

Browse files
dmavrodievdmavrodiev
authored
fix: dedupe SAN entries before checking algorithm uniqueness (#9472)
* fix: dedupe SAN entries before checking algorithm uniqueness Dedupe the certificate's domains with sets.New before checking them against the shared pkaSecretSet map, so a repeated SAN no longer conflicts with itself, while a genuine conflict across two different certificates for the same domain and algorithm is still caught. Add test cases to TestValidateTLSSecretsData: - valid-rsa-duplicate-san-domain: reproduces the bug against a new rsa-cert-dup-san.pem / rsa-pkcs8-dup-san.key fixture. - conflicting-rsa-algorithm-same-domain-different-secrets: guard rail confirming genuine cross-secret conflicts are still rejected. Fixes #9471 Signed-off-by: Dimitar Mavrodiev <dmavrodiev@gmail.com> * Apply codex review comment Signed-off-by: Dimitar Mavrodiev <dmavrodiev@gmail.com> * fix: address code review comment Link the SANs RFC section in the comments. Signed-off-by: Dimitar Mavrodiev <dmavrodiev@gmail.com> --------- Signed-off-by: Dimitar Mavrodiev <dmavrodiev@gmail.com>
1 parent 6fa5fbe commit bae64fd

5 files changed

Lines changed: 79 additions & 0 deletions

File tree

‎internal/gatewayapi/testdata/tls/gen-certs.sh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ openssl rsa -in rsa-pkcs8-san.key -out rsa-pkcs1-san.key
2020
openssl req -x509 -nodes -days $CERT_VALIDITY_DAYS -newkey rsa:2048 -keyout rsa-pkcs8-wildcard.key -out rsa-cert-wildcard.pem -subj "/CN=Test Inc" -addext "subjectAltName = DNS:*, DNS:*.example.com"
2121
openssl rsa -in rsa-pkcs8-wildcard.key -out rsa-pkcs1-wildcard.key
2222

23+
# RSA with a duplicate SAN entry
24+
25+
openssl req -x509 -nodes -days $CERT_VALIDITY_DAYS -newkey rsa:2048 -keyout rsa-pkcs8-dup-san.key -out rsa-cert-dup-san.pem -subj "/CN=Test Inc" -addext "subjectAltName = DNS:foo.bar.com, DNS:foo.bar.com"
26+
2327
# ECDSA-p256
2428

2529
openssl ecparam -name prime256v1 -genkey -noout -out ecdsa-p256.key

‎internal/gatewayapi/testdata/tls/rsa-cert-dup-san.pem‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIDLDCCAhSgAwIBAgIUVc3BBxB88/3dXu5GNXH/Rmz+kNkwDQYJKoZIhvcNAQEL
3+
BQAwEzERMA8GA1UEAwwIVGVzdCBJbmMwHhcNMjYwNzA5MTkyOTM0WhcNMzYwNzA2
4+
MTkyOTM0WjATMREwDwYDVQQDDAhUZXN0IEluYzCCASIwDQYJKoZIhvcNAQEBBQAD
5+
ggEPADCCAQoCggEBAJCEi+CScI2LitW3BMDNUOM+JPZT3wtaUsOFNxElz2S+5nQR
6+
s9jx63wAgNHDtsgMvQs+Xq9IC5quyn/TIWme3G7/9X8KsflNAsbbBCGKA0BeAc7n
7+
r25SZiWxY06hEeVnzxWcJdlR4eLZSzJihS8JQZmM6HVyr86P75B5jQP7+HFfp/LA
8+
JvNaILNsM0UjbnNqYxLGOEANZw5u56WjXUbhwySCybTWl4o3y0fWsvtSq4vIO1XK
9+
pU1QgDbLAVDNz3E4CXajD+gGcid4m2wzqVMiCvb7SwBa7a8TdipoQbwXaldh4l64
10+
o+FNVyI8Sp3ZMsNGoUq9oq/cU6O4MIwbFC/xqT8CAwEAAaN4MHYwHQYDVR0OBBYE
11+
FK6GC7CbB5XjcnxLI9Lzvs1CBz/MMB8GA1UdIwQYMBaAFK6GC7CbB5XjcnxLI9Lz
12+
vs1CBz/MMA8GA1UdEwEB/wQFMAMBAf8wIwYDVR0RBBwwGoILZm9vLmJhci5jb22C
13+
C2Zvby5iYXIuY29tMA0GCSqGSIb3DQEBCwUAA4IBAQBy24sXi/E3O83bwUogTan7
14+
2FV3EoFH2Y++uSeIDmtMxpZ/0NLWP57FohXRK/RV2aFp0uTcd8jBlf9elhYrPu9j
15+
UzlPbn8NygS11CrVGaJQ+2jHapmh4jfVjqkbB+VesCXXF9cF0sWkUcTthcaeYVYY
16+
4yThNZhovxN1n7lqmG7E3bsAV3//mSA+NQkdJ5FBUx4S7G5go9vHNUaUtc95lfwh
17+
GcRxMypBFXyZrgOdlWROIFzm9TZnvqBZDnv7gBtlkB/BqwJ2W8sSavVyNeY81MF0
18+
d0iuqmgR9+w1WjghDftCgEDh60a8dWO8NfBMx0N/OLpm/VRUOAKLlbA5Qg2TONhp
19+
-----END CERTIFICATE-----

‎internal/gatewayapi/testdata/tls/rsa-pkcs8-dup-san.key‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-----BEGIN PRIVATE KEY-----
2+
MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCQhIvgknCNi4rV
3+
twTAzVDjPiT2U98LWlLDhTcRJc9kvuZ0EbPY8et8AIDRw7bIDL0LPl6vSAuarsp/
4+
0yFpntxu//V/CrH5TQLG2wQhigNAXgHO569uUmYlsWNOoRHlZ88VnCXZUeHi2Usy
5+
YoUvCUGZjOh1cq/Oj++QeY0D+/hxX6fywCbzWiCzbDNFI25zamMSxjhADWcObuel
6+
o11G4cMkgsm01peKN8tH1rL7UquLyDtVyqVNUIA2ywFQzc9xOAl2ow/oBnIneJts
7+
M6lTIgr2+0sAWu2vE3YqaEG8F2pXYeJeuKPhTVciPEqd2TLDRqFKvaKv3FOjuDCM
8+
GxQv8ak/AgMBAAECggEAL/HxcA9VTPhbFp0R9B8Js2JmI9zedidApwI2qzc2j49v
9+
6FkJKDPWcry+ABmktcjYHPdTtWY7B1Xu86ppft+H9UFwwnWbZwCgJ7X4sGHXw06M
10+
3gZqYrjuj5nCvw7b35ZpkxtLSUaLoNWDR5N86QZyn40qf/CNGAQTsARLfuNk4MOi
11+
SNM/f8P23JgruBa2ceIC0r3BXQiUrKHwmIwSmEh2/x4jEhZ9+E5XOFpq3AoqVM8h
12+
Ccspyh61nJs+uY87mbE717XFq1Yq0XwotlbEgKzH8v9Uus1Et9GDboinkE/u9Eh4
13+
PUnDT/ya5yijN8FDms0ifAVbJnuQmxU4ZP5XeCr4hQKBgQDFz8LCmA+f2bYhyEL5
14+
2zNJgW78gbJnwyiZpcdEcv8KATbeq+2tha0piiph2sVQszxjswZav7E2htI3D4d2
15+
wEHmVf5cV7qG46BK1nFT3TTnbWP72byr/6cg1BEhpNYgqnpb4n8bGtASwQGt/ZpT
16+
qmGiLbV7PdbSPJ/Z3O6cVQ1K1QKBgQC7B3wmIsUAFl2QZIdRk7rTsOCAWSm4SZDn
17+
NgffJ1EVyzTcr3FpVZLZkN1oJWulnLjeWpg2r9UEPVWSkkIQjUCaQmABpI/Uo59N
18+
ScrZMlnarPOUqf0S40SduW5HMvu2B9TT4rmMPWpUY7sBOEqnN1t9DFOAOv+Ndq+m
19+
F7nAua2FwwKBgDMbLlJgPwkpkmi/+K3c+C8xhZ8vUwyD22V28zi4DTRkg+ybtthy
20+
BP8Kd1C42Om0pRGNG0Mu63YO9xjKplED8wKzjPgGomZfQPaU2Mq2CAkSthZHdvtp
21+
HaDZqWNr1vaxlNNQfU5fawqtWuW887ZR+s+Px6eDnpDKoPIEppE1WC3RAoGAXya5
22+
tLUvwJGgXFuotIoSHKz6KpIyNX3H6LmGW7Om/w15AWWIr2xH38RhwCB5mbIYI5e3
23+
pOrj1tpVdNJQJheW7GQkb/GG80mjPDD0sHd7W1NuQQ4SoM9bE1tJjZOUl9F4J6xL
24+
dduxAuoSM9attFDnjMD+olhht1jQmBGuASz16P0CgYBUj5F+va6MiZX91QzS3Jgf
25+
x7mxuuTPIs1kWMqiqZNMw3x3/FBQpgxYJtYy2si0T7y19WSeEcHU/h7Zret0tUEN
26+
zAomDKPWzQeF+lfpArUGN4A0WoMfVe0Z8HFHzbFakKfM9DpxpSvcA0Suocs2oZ86
27+
6S6A/ISFxW5kdRLPcpCMmA==
28+
-----END PRIVATE KEY-----

‎internal/gatewayapi/tls.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,16 @@ func parseCertsFromTLSSecretsData(secrets []*corev1.Secret) ([]*corev1.Secret, [
166166
}
167167

168168
// Check uniqueness for each domain in the certificate with this algorithm
169+
// Dedupe SANs within this single cert first - RFC 5280 4.2.1.6 permits
170+
// repeated entries in a GeneralNames sequence
171+
seenDomains := sets.New[string]()
169172
hasConflictDomainAlgorithm := false
170173
for _, domain := range certDomains {
174+
if seenDomains.Has(domain) {
175+
continue
176+
}
177+
seenDomains.Insert(domain)
178+
171179
pkaSecretKey := fmt.Sprintf("%s/%s", publicKeyAlgorithm, domain)
172180

173181
// Check whether the public key algorithm and certificate domain are unique

‎internal/gatewayapi/tls_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,26 @@ func TestValidateTLSSecretsData(t *testing.T) {
184184
ExpectedValidCount: 1,
185185
ExpectedCertsCount: 1,
186186
},
187+
{
188+
Name: "valid-rsa-duplicate-san-domain",
189+
CertFiles: []string{"rsa-cert-dup-san.pem"},
190+
KeyFiles: []string{"rsa-pkcs8-dup-san.key"},
191+
ExpectedErrMsg: "",
192+
ExpectedErrReason: "",
193+
ExpectedValidCount: 1,
194+
ExpectedCertsCount: 1,
195+
},
196+
{
197+
// Two distinct secrets legitimately claiming the same domain with the
198+
// same public key algorithm must still be rejected.
199+
Name: "conflicting-rsa-algorithm-same-domain-different-secrets",
200+
CertFiles: []string{"rsa-cert.pem", "rsa-cert.pem"},
201+
KeyFiles: []string{"rsa-pkcs1.key", "rsa-pkcs1.key"},
202+
ExpectedErrMsg: "test/secret public key algorithm must be unique, certificate domain foo.bar.com has a conflicting algorithm [RSA]",
203+
ExpectedErrReason: status.ListenerReasonPartiallyInvalidCertificateRef,
204+
ExpectedValidCount: 1,
205+
ExpectedCertsCount: 1,
206+
},
187207
{
188208
Name: "valid-ecdsa-p256",
189209
CertFiles: []string{"ecdsa-p256-cert.pem"},

0 commit comments

Comments
 (0)