validate name constraint before signing CSR

Signed-off-by: tanujd11 <dwiveditanuj41@gmail.com>
This commit is contained in:
tanujd11 2023-11-20 23:31:59 +05:30
parent 50d84c1bbc
commit adb9311f56
11 changed files with 68 additions and 45 deletions

View File

@ -544,8 +544,8 @@ type NameConstraints struct {
//
// +optional
Permitted *NameConstraintItem
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
// of information appearing in the permitted
//
// +optional

View File

@ -513,9 +513,9 @@ type NameConstraints struct {
// Permitted contains the constraints in which the names must be located.
//
// +optional
Permitted *NameConstraintItem`json:"permitted,omitempty"`
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
Permitted *NameConstraintItem `json:"permitted,omitempty"`
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
// of information appearing in the permitted
//
// +optional

View File

@ -520,9 +520,9 @@ type NameConstraints struct {
// Permitted contains the constraints in which the names must be located.
//
// +optional
Permitted *NameConstraintItem`json:"permitted,omitempty"`
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
Permitted *NameConstraintItem `json:"permitted,omitempty"`
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
// of information appearing in the permitted
//
// +optional

View File

@ -518,9 +518,9 @@ type NameConstraints struct {
// Permitted contains the constraints in which the names must be located.
//
// +optional
Permitted *NameConstraintItem`json:"permitted,omitempty"`
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
Permitted *NameConstraintItem `json:"permitted,omitempty"`
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
// of information appearing in the permitted
//
// +optional

View File

@ -684,7 +684,7 @@ func TestValidateCertificate(t *testing.T) {
Spec: internalcmapi.CertificateSpec{
CommonName: "testcn",
SecretName: "abc",
IsCA: true,
IsCA: true,
NameConstraints: &internalcmapi.NameConstraints{
Permitted: &internalcmapi.NameConstraintItem{
DNSDomains: []string{"example.com"},
@ -700,9 +700,9 @@ func TestValidateCertificate(t *testing.T) {
"invalid with name constraints": {
cfg: &internalcmapi.Certificate{
Spec: internalcmapi.CertificateSpec{
CommonName: "testcn",
SecretName: "abc",
IsCA: true,
CommonName: "testcn",
SecretName: "abc",
IsCA: true,
NameConstraints: &internalcmapi.NameConstraints{},
IssuerRef: cmmeta.ObjectReference{
Name: "valid",

View File

@ -99,7 +99,7 @@ const (
UseCertificateRequestBasicConstraints featuregate.Feature = "UseCertificateRequestBasicConstraints"
// Owner: @tanujd11
// Alpha: v1.13
// Alpha: v1.14
//
// UseCertificateRequestNameConstraints will add Name Constraints section in the Extension Request of the Certificate Signing Request
// This feature will add NameConstraints section in CSR with CA field as true
@ -147,5 +147,5 @@ var defaultCertManagerFeatureGates = map[featuregate.Feature]featuregate.Feature
ServerSideApply: {Default: false, PreRelease: featuregate.Alpha},
LiteralCertificateSubject: {Default: false, PreRelease: featuregate.Alpha},
UseCertificateRequestBasicConstraints: {Default: false, PreRelease: featuregate.Alpha},
UseCertificateRequestNameConstraints: {Default: false, PreRelease: featuregate.Alpha},
UseCertificateRequestNameConstraints: {Default: false, PreRelease: featuregate.Alpha},
}

View File

@ -609,8 +609,8 @@ type NameConstraints struct {
//
// +optional
Permitted *NameConstraintItem `json:"permitted,omitempty"`
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
// Excluded contains the constraints which must be disallowed. Any name matching a
// restriction in the excluded field is invalid regardless
// of information appearing in the permitted
//
// +optional

View File

@ -198,16 +198,16 @@ func CertificateTemplateFromCSR(csr *x509.CertificateRequest, validatorMutators
if err != nil {
return err
}
template.PermittedDNSDomainsCritical = nameConstraints.PermittedDNSDomainsCritical
template.PermittedDNSDomains = nameConstraints.PermittedDNSDomains
template.ExcludedDNSDomains = nameConstraints.ExcludedDNSDomains
template.ExcludedDNSDomains = nameConstraints.ExcludedDNSDomains
template.PermittedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.PermittedIPRanges)
template.ExcludedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.ExcludedIPRanges)
template.ExcludedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.ExcludedIPRanges)
template.PermittedEmailAddresses = nameConstraints.PermittedEmailAddresses
template.ExcludedEmailAddresses = nameConstraints.ExcludedEmailAddresses
template.PermittedURIDomains = nameConstraints.PermittedURIDomains
template.ExcludedURIDomains = nameConstraints.ExcludedEmailAddresses
template.ExcludedURIDomains = nameConstraints.ExcludedEmailAddresses
}
// RFC 5280, 4.2.1.3

View File

@ -207,7 +207,6 @@ func WithEncodeNameConstraintsInRequest(encode bool) GenerateCSROption {
}
}
func WithUseLiteralSubject(useLiteralSubject bool) GenerateCSROption {
return func(o *generateCSROptions) {
o.UseLiteralSubject = useLiteralSubject
@ -382,6 +381,10 @@ func SignCSRTemplate(caCerts []*x509.Certificate, caKey crypto.Signer, template
}
issuingCACert := caCerts[0]
err := validateNameConstraints(issuingCACert, template)
if err != nil {
return PEMBundle{}, fmt.Errorf("cert not present in the namesConstraints specified by cacert: %s", err.Error())
}
_, cert, err := SignCertificate(template, issuingCACert, template.PublicKey, caKey)
if err != nil {

View File

@ -359,9 +359,9 @@ func TestGenerateCSR(t *testing.T) {
nameConstraints := NameConstraints{
PermittedDNSDomainsCritical: true,
PermittedDNSDomains: []string{"example.org"},
PermittedIPRanges: []net.IPNet{*permittedIPNet},
PermittedEmailAddresses: []string{"email@email.org"},
ExcludedIPRanges: []net.IPNet{*excludedIPNet},
PermittedIPRanges: []net.IPNet{*permittedIPNet},
PermittedEmailAddresses: []string{"email@email.org"},
ExcludedIPRanges: []net.IPNet{*excludedIPNet},
}
asn1NameConstraints, err := asn1.Marshal(nameConstraints)
if err != nil {
@ -685,9 +685,13 @@ func TestSignCSRTemplate(t *testing.T) {
// for that, we construct a chain of four certificates:
// a root CA, two intermediate CA, and a leaf certificate.
mustCreatePair := func(issuerCert *x509.Certificate, issuerPK crypto.Signer, name string, isCA bool) ([]byte, *x509.Certificate, *x509.Certificate, crypto.Signer) {
mustCreatePair := func(issuerCert *x509.Certificate, issuerPK crypto.Signer, name string, isCA bool, nameConstraints *NameConstraints) ([]byte, *x509.Certificate, *x509.Certificate, crypto.Signer) {
pk, err := GenerateECPrivateKey(256)
require.NoError(t, err)
var permittedIPRanges []*net.IPNet
if nameConstraints != nil {
permittedIPRanges = ConvertIPNeSliceToIPNetPointerSlice(nameConstraints.PermittedIPRanges)
}
tmpl := &x509.Certificate{
Version: 3,
BasicConstraintsValid: true,
@ -701,6 +705,7 @@ func TestSignCSRTemplate(t *testing.T) {
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
PublicKey: pk.Public(),
IsCA: isCA,
PermittedIPRanges: permittedIPRanges,
}
if isCA {
@ -719,10 +724,16 @@ func TestSignCSRTemplate(t *testing.T) {
return pem, cert, tmpl, pk
}
rootPEM, rootCert, rootTmpl, rootPK := mustCreatePair(nil, nil, "root", true)
int1PEM, int1Cert, int1Tmpl, int1PK := mustCreatePair(rootCert, rootPK, "int1", true)
int2PEM, int2Cert, int2Tmpl, int2PK := mustCreatePair(int1Cert, int1PK, "int2", true)
leafPEM, _, leafTmpl, _ := mustCreatePair(int2Cert, int2PK, "leaf", false)
rootPEM, rootCert, rootTmpl, rootPK := mustCreatePair(nil, nil, "root", true, nil)
int1PEM, int1Cert, int1Tmpl, int1PK := mustCreatePair(rootCert, rootPK, "int1", true, nil)
int2PEM, int2Cert, int2Tmpl, int2PK := mustCreatePair(int1Cert, int1PK, "int2", true, nil)
leafPEM, _, leafTmpl, _ := mustCreatePair(int2Cert, int2PK, "leaf", false, nil)
// vars for testing name constraints
_, permittedIPNet, _ := net.ParseCIDR("10.10.0.0/16")
_, ncRootCert, _, ncRootPK := mustCreatePair(nil, nil, "ncroot", true, &NameConstraints{PermittedIPRanges: []net.IPNet{*permittedIPNet}})
_, _, ncLeafTmpl, _ := mustCreatePair(ncRootCert, ncRootPK, "ncleaf", false, nil)
ncLeafTmpl.IPAddresses = []net.IP{net.ParseIP("10.20.0.5")}
tests := map[string]struct {
caCerts []*x509.Certificate
@ -769,6 +780,12 @@ func TestSignCSRTemplate(t *testing.T) {
template: rootTmpl,
wantErr: true,
},
"Error: Non-compliance with NameConstraints in the leaf certificate.": {
caCerts: []*x509.Certificate{ncRootCert},
caKey: ncRootPK,
template: ncLeafTmpl,
wantErr: true,
},
}
for name, test := range tests {

View File

@ -32,15 +32,15 @@ var (
// NameConstraints represents the NameConstraints extension.
type NameConstraints struct {
PermittedDNSDomainsCritical bool `asn1:"optional,explicit,tag:0"`
PermittedDNSDomains []string `asn1:"optional,explicit,tag:1"`
ExcludedDNSDomains []string `asn1:"optional,explicit,tag:2"`
PermittedDNSDomainsCritical bool `asn1:"optional,explicit,tag:0"`
PermittedDNSDomains []string `asn1:"optional,explicit,tag:1"`
ExcludedDNSDomains []string `asn1:"optional,explicit,tag:2"`
PermittedIPRanges []net.IPNet `asn1:"optional,explicit,tag:3"`
ExcludedIPRanges []net.IPNet `asn1:"optional,explicit,tag:4"`
PermittedEmailAddresses []string `asn1:"optional,explicit,tag:5"`
ExcludedEmailAddresses []string `asn1:"optional,explicit,tag:6"`
PermittedURIDomains []string `asn1:"optional,explicit,tag:7"`
ExcludedURIDomains []string `asn1:"optional,explicit,tag:8"`
PermittedEmailAddresses []string `asn1:"optional,explicit,tag:5"`
ExcludedEmailAddresses []string `asn1:"optional,explicit,tag:6"`
PermittedURIDomains []string `asn1:"optional,explicit,tag:7"`
ExcludedURIDomains []string `asn1:"optional,explicit,tag:8"`
}
// Adapted from x509.go
@ -54,10 +54,10 @@ func MarshalNameConstraints(nameConstraints *v1.NameConstraints) (pkix.Extension
}
nameConstraintsForMarshalling = NameConstraints{
PermittedDNSDomainsCritical: nameConstraints.Permitted.Critical,
PermittedDNSDomains: nameConstraints.Permitted.DNSDomains,
PermittedIPRanges: permittedIPRanges,
PermittedEmailAddresses: nameConstraints.Permitted.EmailAddresses,
PermittedURIDomains: nameConstraints.Permitted.URIDomains,
PermittedDNSDomains: nameConstraints.Permitted.DNSDomains,
PermittedIPRanges: permittedIPRanges,
PermittedEmailAddresses: nameConstraints.Permitted.EmailAddresses,
PermittedURIDomains: nameConstraints.Permitted.URIDomains,
}
}
@ -78,7 +78,7 @@ func MarshalNameConstraints(nameConstraints *v1.NameConstraints) (pkix.Extension
func parseCIDRs(cidrs []string) ([]net.IPNet, error) {
ipRanges := []net.IPNet{}
for _, cidr := range(cidrs) {
for _, cidr := range cidrs {
_, ipNet, err := net.ParseCIDR(cidr)
ipRanges = append(ipRanges, net.IPNet{
IP: ipNet.IP,
@ -105,7 +105,10 @@ func UnmarshalNameConstraints(value []byte) (NameConstraints, error) {
}
// ConvertIPNeSliceToIPNetPointerSlice converts []net.IPNet to []*net.IPNet.
func ConvertIPNeSliceToIPNetPointerSlice(ipNetPointerSlice []net.IPNet) ([]*net.IPNet) {
func ConvertIPNeSliceToIPNetPointerSlice(ipNetPointerSlice []net.IPNet) []*net.IPNet {
if ipNetPointerSlice == nil {
return nil
}
var ipNets []*net.IPNet
for _, ipNet := range ipNetPointerSlice {
ipNets = append(ipNets, &ipNet)