feat: dns-resolved IPs must fall within whitelisted IP set

README.md

@@ -109,13 +109,11 @@ we check the following criteria:
   `system:node:` prefix)
 * CSR SAN IP Addresses must all be part of the set of IP addresses resolved
   from the SAN DNS Name
-* ⚠ the CSR SAN DNS Name (if specified) must resolve to IP address(es) that
+* the CSR SAN DNS Name (if specified) must resolve to IP address(es) that
   fall within the set of provider-specified IP ranges.
 * the CSR SAN IP Address(es) must fall within a set of provider-specified IP
   ranges
 
-⚠ == not yet implemented
-
 With those verifications in place, it makes it quite hard for an attacker to
 get a forged hostname to be signed, it would indeed require:
 

internal/controller/csr_controller_test.go

@@ -30,6 +30,7 @@ func TestValidCsrApproved(t *testing.T) {
 	csrParams := CsrParams{
 		ipAddresses: testNodeIpAddresses,
 		nodeName:    testNodeName,
+		dnsName:     testNodeName + ".test.ch",
 	}
 	validCsr := createCsr(t, csrParams)
 
@@ -48,6 +49,7 @@ func TestWrongSignerCsr(t *testing.T) {
 		csrName:     "csr-wrong-signer",
 		ipAddresses: testNodeIpAddresses,
 		nodeName:    testNodeName,
+		dnsName:     testNodeName + ".test.ch",
 	}
 	csr := createCsr(t, csrParams)
 	csr.Spec.SignerName = "example.com/not-kubelet-serving"
@@ -67,7 +69,9 @@ func TestNonMatchingCommonNameUsername(t *testing.T) {
 		csrName:     "csr-non-matching",
 		commonName:  "funny-common-name",
 		ipAddresses: testNodeIpAddresses,
-		nodeName:    testNodeName}
+		nodeName:    testNodeName,
+		dnsName:     testNodeName + ".test.ch",
+	}
 	csr := createCsr(t, csrParams)
 	_, nodeClientSet, _ := createControlPlaneUser(t, csr.Spec.Username, []string{"system:masters"})
 
@@ -143,6 +147,7 @@ func TestMismatchedResolvedIpsSANIps(t *testing.T) {
 	csrParams := CsrParams{
 		csrName:     "mismatched-san-ip-resolved-dns-ip",
 		nodeName:    testNodeName,
+		dnsName:     testNodeName + ".test.ch",
 		ipAddresses: []net.IP{{9, 9, 9, 9}},
 	}
 	csr := createCsr(t, csrParams)
@@ -163,6 +168,7 @@ func TestExpirationSecondsTooLarge(t *testing.T) {
 		csrName:           "expiration-seconds",
 		expirationSeconds: 368 * 24 * 3600, // one day more than the maximum of 367
 		nodeName:          testNodeName,
+		dnsName:           testNodeName + ".test.ch",
 	}
 	csr := createCsr(t, csrParams)
 	_, nodeClientSet, _ := createControlPlaneUser(t, csr.Spec.Username, []string{"system:masters"})
@@ -245,3 +251,26 @@ func TestIPv6NotWhitelisted(t *testing.T) {
 	assert.False(t, approved)
 	assert.True(t, denied)
 }
+
+func TestIPv6WithoutDNSNotWhitelisted(t *testing.T) {
+	csrParams := CsrParams{
+		csrName:     "ipv6-noDNS-non-whitelisted",
+		nodeName:    testNodeName,
+		ipAddresses: []net.IP{{0x20, 0x01, 0xc0, 0xfe, 0xbe, 0xef, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x14}},
+	}
+	dnsResolver.Zones[csrParams.dnsName+"."] = mockdns.Zone{
+		AAAA: []string{"2001:c0fe:beef::14"},
+	}
+
+	csr := createCsr(t, csrParams)
+	_, nodeClientSet, _ := createControlPlaneUser(t, csr.Spec.Username, []string{"system:masters"})
+
+	_, err := nodeClientSet.CertificatesV1().CertificateSigningRequests().Create(
+		testContext, &csr, metav1.CreateOptions{})
+	require.Nil(t, err, "Could not create the CSR.")
+
+	approved, denied, err := waitCsrApprovalStatus(csr.Name)
+	require.Nil(t, err, "Could not retrieve the CSR to check its approval status")
+	assert.False(t, approved)
+	assert.True(t, denied)
+}

internal/controller/regex_ip_checks.go

@@ -65,6 +65,12 @@ func (r *CertificateSigningRequestReconciler) DNSCheck(ctx context.Context, csr
 		}
 
 		setBuilder.Add(ipa)
+
+		if !r.ProviderIPSet.Contains(ipa) {
+			return false, fmt.Sprintf("One of the resolved IP addresses, %s,"+
+				"isn't part of the provider-specified set of whitelisted IP. denying the certificate",
+				ipa), nil
+		}
 	}
 
 	resolvedIPSet, _ := setBuilder.IPSet()
@@ -77,14 +83,15 @@ func (r *CertificateSigningRequestReconciler) DNSCheck(ctx context.Context, csr
 		}
 
 		if !resolvedIPSet.Contains(ipa) {
-			return false, fmt.Sprintf("One of the SAN IP addresses, %s, is not contained in the set of resolved IP addresses, denying the CSR.", ipa), nil
+			return false, fmt.Sprintf("One of the SAN IP addresses, %s, "+
+				"is not contained in the set of resolved IP addresses, denying the CSR.", ipa), nil
 		}
 	}
 
 	return valid, reason, nil
 }
 
-// WhitelistedIPCheck verifies that the x509cr IP Addresses are contained in the
+// WhitelistedIPCheck verifies that the x509cr SAN IP Addresses are contained in the
 // set of ProviderSpecified IP addresses
 func (r *CertificateSigningRequestReconciler) WhitelistedIPCheck(csr *certificatesv1.CertificateSigningRequest, x509cr *x509.CertificateRequest) (valid bool, reason string, err error) {
 	sanIPAddrs := x509cr.IPAddresses

internal/controller/testenv_setup_test.go

@@ -98,10 +98,6 @@ func createCsr(t *testing.T, params CsrParams) certificates_v1.CertificateSignin
 		params.nodeName = randstr.String(4, "0123456789abcdefghijklmnopqrstuvwxyz")
 	}
 
-	if len(params.dnsName) == 0 {
-		params.dnsName = params.nodeName + ".test.ch"
-	}
-
 	csr.Spec.SignerName = certificates_v1.KubeletServingSignerName
 	csr.Spec.Usages = append(csr.Spec.Usages,
 		certificates_v1.UsageDigitalSignature,
@@ -124,9 +120,12 @@ func createCsr(t *testing.T, params CsrParams) certificates_v1.CertificateSignin
 			Organization: []string{"system:nodes"},
 			CommonName:   params.commonName,
 		},
-		DNSNames:    []string{params.dnsName},
 		IPAddresses: params.ipAddresses,
 	}
+	if len(params.dnsName) > 0 {
+		x509RequestTemplate.DNSNames = []string{params.dnsName}
+	}
+
 	x509Request, _ := x509.CreateCertificateRequest(rand.Reader, &x509RequestTemplate, priv)
 	pemRequest := pem.EncodeToMemory(&pem.Block{
 		Type:  "CERTIFICATE REQUEST",