letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reject all CSRs with an IP in the CN (#8282) 

Although https://github.com/letsencrypt/boulder/pull/8231 fixed
csr.CNFromCSR to ignore Common Names that are valid IPs, that didn't
fully solve our issue: identifier.FromCSR still extracts the CN and
assumes that it is a dnsName, leading to a mismatch between the CSR's
identifiers and the Order's identifiers.

Instead, let's outright reject all CSRs which carry an IP in their
Subject Common Name. Although this doesn't have the elegance of
rejecting such CNs on a profile-by-profile basis, it matches our ongoing
effort to do away with CNs entirely.
This commit is contained in:
Aaron Gable 2025-06-30 16:07:11 -07:00 committed by GitHub
parent 5493682a19
commit a1a7a7f7e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 21 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
csr/csr.go
View File

@ -37,6 +37,7 @@ var (
invalidEmailPresent = berrors.BadCSRError("CSR contains one or more email address fields")
invalidURIPresent = berrors.BadCSRError("CSR contains one or more URI fields")
invalidNoIdent = berrors.BadCSRError("at least one identifier is required")
invalidIPCN = berrors.BadCSRError("CSR contains IP address in Common Name")
)
// VerifyCSR checks the validity of a x509.CertificateRequest. It uses
@ -69,6 +70,16 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
return invalidURIPresent
}
// Reject all CSRs which have an IP address in the CN. We want to get rid of
// CNs entirely anyway, and IP addresses are a new feature, so don't let
// clients get in the habit of including them in the CN. We don't use
// CNFromCSR here because that also filters out IP address CNs, for defense
// in depth.
_, err = netip.ParseAddr(csr.Subject.CommonName)
if err == nil { // Inverted! Successful parsing is a bad thing in this case.
return invalidIPCN
}
// FromCSR also performs normalization, returning values that may not match
// the literal CSR contents.
idents := identifier.FromCSR(csr)
@ -98,7 +109,7 @@ func CNFromCSR(csr *x509.CertificateRequest) string {
if csr.Subject.CommonName != "" {
_, err := netip.ParseAddr(csr.Subject.CommonName)
if err == nil { // inverted; we're looking for successful parsing here
if err == nil { // Inverted! Successful parsing is a bad thing in this case.
return ""
}

9
csr/csr_test.go
View File

@ -70,6 +70,9 @@ func TestVerifyCSR(t *testing.T) {
signedReqWithIPAddress := new(x509.CertificateRequest)
*signedReqWithIPAddress = *signedReq
signedReqWithIPAddress.IPAddresses = []net.IP{net.IPv4(1, 2, 3, 4)}
signedReqWithIPCN := new(x509.CertificateRequest)
*signedReqWithIPCN = *signedReq
signedReqWithIPCN.Subject.CommonName = "1.2.3.4"
signedReqWithURI := new(x509.CertificateRequest)
*signedReqWithURI = *signedReq
testURI, _ := url.ParseRequestURI("https://example.com/")
@ -141,6 +144,12 @@ func TestVerifyCSR(t *testing.T) {
&mockPA{},
nil,
},
{
signedReqWithIPCN,
100,
&mockPA{},
invalidIPCN,
},
{
signedReqWithURI,
100,