diff --git a/csr/csr.go b/csr/csr.go index 730bb9a9f..34b84c39b 100644 --- a/csr/csr.go +++ b/csr/csr.go @@ -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 "" } diff --git a/csr/csr_test.go b/csr/csr_test.go index 1aabc3cb8..d3d3d1bc4 100644 --- a/csr/csr_test.go +++ b/csr/csr_test.go @@ -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,