Skip to content

Commit

Permalink
fix: also validate DNS/SAN names when DNS resolution is bypassed
Browse files Browse the repository at this point in the history
closes issue #247

Signed-off-by: Clément Nussbaumer <[email protected]>
  • Loading branch information
clementnuss committed May 7, 2024
1 parent f91b9a1 commit f4654d3
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 14 deletions.
6 changes: 3 additions & 3 deletions internal/controller/csr_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ func TestNonMatchingCommonNameUsername(t *testing.T) {
func TestRegexCheckActiveWithBypass(t *testing.T) {
csrParams := CsrParams{
csrName: "csr-mismatch-SAN-hostname-with-bypass",
nodeName: testNodeName,
dnsName: "hostname-000.test.ch,auth.evil.io",
nodeName: "a",
dnsName: "a" + ".test.ch,auth.evil.io",
}

csrController.BypassDNSResolution = true
Expand All @@ -104,7 +104,7 @@ func TestRegexCheckActiveWithBypass(t *testing.T) {
require.Nil(t, err, "Could not create the CSR.")

approved, denied, reason, err := waitCsrApprovalStatus(csr.Name)
t.Log("CSR rejected with the following reason:" + reason)
t.Log(reason)
require.Nil(t, err, "Could not retrieve the CSR to check its approval status")
assert.False(t, approved)
assert.True(t, denied)
Expand Down
18 changes: 7 additions & 11 deletions internal/controller/regex_ip_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ func (r *CertificateSigningRequestReconciler) DNSCheck(ctx context.Context, csr
return valid, reason, nil
}

// bypassing DNS reslution - DNS check is approved
if r.BypassDNSResolution {
valid = true
return valid, reason, nil
}

dnsCtx, dnsCtxCancel := context.WithDeadline(ctx, time.Now().Add(time.Second)) // 1 second timeout for the dns request
defer dnsCtxCancel()

Expand All @@ -53,13 +47,15 @@ func (r *CertificateSigningRequestReconciler) DNSCheck(ctx context.Context, csr
return valid, reason, err
}

resolvedAddrs, err := r.DNSResolver.LookupHost(dnsCtx, sanDNSName)
if !r.BypassDNSResolution {
resolvedAddrs, err := r.DNSResolver.LookupHost(dnsCtx, sanDNSName)

if err != nil || len(resolvedAddrs) == 0 {
return false, "The SAN DNS Name could not be resolved, denying the CSR", nil
}
if err != nil || len(resolvedAddrs) == 0 {
return false, "The SAN DNS Name could not be resolved, denying the CSR", nil
}

allResolvedAddrs = append(allResolvedAddrs, resolvedAddrs...)
allResolvedAddrs = append(allResolvedAddrs, resolvedAddrs...)
}
}

var setBuilder netipx.IPSetBuilder
Expand Down

0 comments on commit f4654d3

Please sign in to comment.