diff --git a/go.mod b/go.mod index db5a4d7..79c5c62 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +4,7 @@ go 1.17 require ( github.com/foxcpp/go-mockdns v0.0.0-20210729171921-fb145fc6f897 - github.com/go-logr/logr v1.2.2 - github.com/go-logr/zapr v1.2.2 + github.com/go-logr/zapr v1.2.0 github.com/postfinance/flash v0.2.0 github.com/stretchr/testify v1.7.0 github.com/thanhpk/randstr v1.0.4 @@ -31,6 +30,7 @@ require ( github.com/evanphx/json-patch v4.12.0+incompatible // indirect github.com/form3tech-oss/jwt-go v3.2.3+incompatible // indirect github.com/fsnotify/fsnotify v1.5.1 // indirect + github.com/go-logr/logr v1.2.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.2 // indirect diff --git a/go.sum b/go.sum index e807f0f..dd36609 100644 --- a/go.sum +++ b/go.sum @@ -188,12 +188,10 @@ github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= github.com/go-logr/logr v0.2.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= +github.com/go-logr/logr v1.2.0 h1:QK40JKJyMdUDz+h+xvCsru/bJhvG0UxvePV0ufL/AcE= github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.2.2 h1:ahHml/yUpnlb96Rp8HCvtYVPY8ZYpxq3g7UYchIYwbs= -github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/zapr v1.2.0 h1:n4JnPI1T3Qq1SFEi/F8rwLrZERp2bso19PJZDB9dayk= github.com/go-logr/zapr v1.2.0/go.mod h1:Qa4Bsj2Vb+FAVeAKsLD8RLQ+YRJB8YDmOAKxaBQf7Ro= -github.com/go-logr/zapr v1.2.2 h1:5YNlIL6oZLydaV4dOFjL8YpgXF/tPeTbnpatnu3cq6o= -github.com/go-logr/zapr v1.2.2/go.mod h1:eIauM6P8qSvTw5o2ez6UEAfGjQKrxQTl5EoK+Qa2oG4= github.com/go-openapi/jsonpointer v0.19.3/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= github.com/go-openapi/jsonpointer v0.19.5/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= github.com/go-openapi/jsonreference v0.19.3/go.mod h1:rjx6GuL8TTa9VaixXglHmQmIL98+wF9xc8zWvFonSJ8= diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 7524ca3..f013eda 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -35,13 +35,14 @@ var ( // Config stores all parameters needed to configure a controller-manager type Config struct { - logLevel int - metricsAddr string - probeAddr string - RegexStr string - MaxSec int - K8sConfig *rest.Config - DNSResolver controller.HostResolver + logLevel int + metricsAddr string + probeAddr string + RegexStr string + MaxSec int + K8sConfig *rest.Config + DNSResolver controller.HostResolver + BypassDNSResolution bool } // Run will start the controller with the default settings @@ -113,6 +114,7 @@ func CreateControllerManager(config *Config) (mgr ctrl.Manager, code int) { ProviderRegexp: providerRegexp.MatchString, MaxExpirationSeconds: int32(config.MaxSec), Resolver: config.DNSResolver, + BypassDNSResolution: config.BypassDNSResolution, } if err = csrController.SetupWithManager(mgr); err != nil { @@ -132,11 +134,12 @@ func prepareCmdlineConfig() *Config { fs := flag.NewFlagSet("kubelet-csr-approver", flag.ExitOnError) var ( - logLevel = fs.Int("level", 0, "level ranges from -5 (Fatal) to 10 (Verbose)") - metricsAddr = fs.String("metrics-bind-address", ":8080", "address the metric endpoint binds to.") - probeAddr = fs.String("health-probe-bind-address", ":8081", "address the probe endpoint binds to.") - regexStr = fs.String("provider-regex", "", "provider-specified regex to validate CSR SAN names against") - maxSec = fs.Int("max-expiration-sec", 367*24*3600, "maximum seconds a CSR can request a cerficate for. defaults to 367 days") + logLevel = fs.Int("level", 0, "level ranges from -5 (Fatal) to 10 (Verbose)") + metricsAddr = fs.String("metrics-bind-address", ":8080", "address the metric endpoint binds to.") + probeAddr = fs.String("health-probe-bind-address", ":8081", "address the probe endpoint binds to.") + regexStr = fs.String("provider-regex", "", "provider-specified regex to validate CSR SAN names against") + maxSec = fs.Int("max-expiration-sec", 367*24*3600, "maximum seconds a CSR can request a cerficate for. defaults to 367 days") + bypassDNSResolution = fs.Bool("bypass-dns-resolution", false, "set this parameter to true to bypass DNS resolution checks") ) err := ff.Parse(fs, os.Args[1:], ff.WithEnvVarNoPrefix()) @@ -147,11 +150,12 @@ func prepareCmdlineConfig() *Config { } config := Config{ - logLevel: *logLevel, - metricsAddr: *metricsAddr, - probeAddr: *probeAddr, - RegexStr: *regexStr, - MaxSec: *maxSec, + logLevel: *logLevel, + metricsAddr: *metricsAddr, + probeAddr: *probeAddr, + RegexStr: *regexStr, + BypassDNSResolution: *bypassDNSResolution, + MaxSec: *maxSec, } config.DNSResolver = net.DefaultResolver diff --git a/internal/controller/csr_controller.go b/internal/controller/csr_controller.go index bc0f8d5..9dc1939 100644 --- a/internal/controller/csr_controller.go +++ b/internal/controller/csr_controller.go @@ -22,8 +22,6 @@ import ( "fmt" "strings" - "github.com/go-logr/logr" - certificatesv1 "k8s.io/api/certificates/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -48,6 +46,7 @@ type CertificateSigningRequestReconciler struct { Scheme *runtime.Scheme ProviderRegexp func(string) bool MaxExpirationSeconds int32 + BypassDNSResolution bool Resolver HostResolver } @@ -56,6 +55,7 @@ type CertificateSigningRequestReconciler struct { //+kubebuilder:rbac:groups=certificates.k8s.io,resources=signers,resourceNames="kubernetes.io/kubelet-serving",verbs=approve // Reconcile will perform a series of checks before deciding whether the CSR should be approved or denied +//nolint: gocyclo // cyclomatic complexity is high (over 15), but this improves readibility for the programmer, therefore we ignore the linting error func (r *CertificateSigningRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, returnErr error) { l := log.FromContext(ctx) @@ -71,18 +71,31 @@ func (r *CertificateSigningRequestReconciler) Reconcile(ctx context.Context, req return } - if passed := baselineCsrChecks(l, &csr); !passed { + // baseline CSR checks - triage to ignore CSR we should process + if csr.Spec.SignerName != certificatesv1.KubeletServingSignerName { + l.V(4).Info("Ignoring non-kubelet-serving CSR.") + return + } + + if approved, denied := GetCertApprovalCondition(&csr.Status); approved || denied { + l.V(3).Info("The CSR is already approved|denied. Ignoring", "approved", approved, "denied", denied) + return + } + + if len(csr.Status.Certificate) > 0 { + l.V(3).Info("The CSR is already signed. No need to do anything else.") return } + // actual CSR and x509 CR checks x509cr, err := ParseCSR(csr.Spec.Request) if err != nil { l.Error(err, fmt.Sprintf("unable to parse csr %q", csr.Name)) return } - if csr.Spec.ExpirationSeconds != nil && *csr.Spec.ExpirationSeconds > r.MaxExpirationSeconds { - reason := "CSR spec.expirationSeconds is longer than the maximum allowed expiration second" + if len(x509cr.DNSNames)+len(x509cr.IPAddresses) == 0 { + reason := "The x509 Cert Request SAN contains neither an IP address nor a DNS name" l.V(0).Info("Denying kubelet-serving CSR. Reason:" + reason) appendCondition(&csr, false, reason) @@ -97,12 +110,17 @@ func (r *CertificateSigningRequestReconciler) Reconcile(ctx context.Context, req "commonName", x509cr.Subject.CommonName, "specUsername", csr.Spec.Username) appendCondition(&csr, false, reason) - } else if valid, reason, err := r.RegexIPChecks(ctx, &csr, x509cr); !valid { + } else if valid, reason, err := r.DNSCheck(ctx, &csr, x509cr); !valid { if err != nil { l.V(0).Error(err, reason) return res, err // returning a non-nil error to make this request be processed again in the reconcile function } - l.V(0).Info("Denying kubelet-serving CSR. Regex/IP checks failed. Reason:" + reason) + l.V(0).Info("Denying kubelet-serving CSR. DNS checks failed. Reason:" + reason) + + appendCondition(&csr, false, reason) + } else if csr.Spec.ExpirationSeconds != nil && *csr.Spec.ExpirationSeconds > r.MaxExpirationSeconds { + reason := "CSR spec.expirationSeconds is longer than the maximum allowed expiration second" + l.V(0).Info("Denying kubelet-serving CSR. Reason:" + reason) appendCondition(&csr, false, reason) } else if valid, reason := ProviderChecks(&csr, x509cr); !valid { @@ -127,27 +145,6 @@ func (r *CertificateSigningRequestReconciler) Reconcile(ctx context.Context, req return res, nil } -func baselineCsrChecks(l logr.Logger, csr *certificatesv1.CertificateSigningRequest) (passed bool) { - passed = false - - if csr.Spec.SignerName != certificatesv1.KubeletServingSignerName { - l.V(4).Info("Ignoring non-kubelet-serving CSR.") - return - } - - if approved, denied := GetCertApprovalCondition(&csr.Status); approved || denied { - l.V(3).Info("The CSR is already approved|denied. Ignoring", "approved", approved, "denied", denied) - return - } - - if len(csr.Status.Certificate) > 0 { - l.V(3).Info("The CSR is already signed. No need to do anything else.") - return - } - - return true -} - func appendCondition(csr *certificatesv1.CertificateSigningRequest, approved bool, reason string) { if approved { csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1.CertificateSigningRequestCondition{ diff --git a/internal/controller/regex_ip_checks.go b/internal/controller/regex_ip_checks.go index 250dae2..cd6cb4b 100644 --- a/internal/controller/regex_ip_checks.go +++ b/internal/controller/regex_ip_checks.go @@ -12,69 +12,74 @@ import ( certificatesv1 "k8s.io/api/certificates/v1" ) -// RegexIPChecks is a function checking that the DNS Name and the IP address comply with the provider-specific regex/ranges -func (r *CertificateSigningRequestReconciler) RegexIPChecks(ctx context.Context, csr *certificatesv1.CertificateSigningRequest, x509cr *x509.CertificateRequest) (valid bool, reason string, err error) { +// DNSCheck is a function checking that the DNS name: +// complies with the provider-specific regex +// is resolvable (this check can be opted out with a parameter) +func (r *CertificateSigningRequestReconciler) DNSCheck(ctx context.Context, csr *certificatesv1.CertificateSigningRequest, x509cr *x509.CertificateRequest) (valid bool, reason string, err error) { if valid = (len(x509cr.DNSNames) <= 1); !valid { reason = "The x509 Cert Request contains more than 1 DNS name" return } - if valid = (len(x509cr.DNSNames)+len(x509cr.IPAddresses) != 0); !valid { - reason = "The x509 Cert Request SAN doesn't contain IP address nor DNS name" - return + // no DNS name to check, the DNS check is approved + if len(x509cr.DNSNames) == 0 { + valid = true + return valid, reason, nil } - if len(x509cr.DNSNames) == 1 { - sanDNSName := x509cr.DNSNames[0] + sanDNSName := x509cr.DNSNames[0] + hostname := strings.TrimPrefix(csr.Spec.Username, "system:node:") - if valid = r.ProviderRegexp(sanDNSName); !valid { - reason = "The DNS name in the x509 CSR is not allowed by the Cloud provider regex" - return - } + if valid = strings.HasPrefix(sanDNSName, hostname); !valid { + reason = "The SAN DNS Name in the x509 CSR is not prefixed by the node name (hostname)" + return + } - hostname := strings.TrimPrefix(csr.Spec.Username, "system:node:") - if valid = strings.HasPrefix(sanDNSName, hostname); !valid { - reason = "The SAN DNS Name in the x509 CSR is not prefixed by the node name (hostname)" - return - } + if valid = r.ProviderRegexp(sanDNSName); !valid { + reason = "The SAN DNS name in the x509 CR is not allowed by the Cloud provider regex" + return + } - dnsCtx, dnsCtxCancel := context.WithDeadline(ctx, time.Now().Add(time.Second)) // 1 second timeout for the dns request - defer dnsCtxCancel() + // bypassing DNS reslution - DNS check is approved + if r.BypassDNSResolution { + valid = true + return valid, reason, nil + } - var resolvedAddrs []string - resolvedAddrs, err = r.Resolver.LookupHost(dnsCtx, sanDNSName) + dnsCtx, dnsCtxCancel := context.WithDeadline(ctx, time.Now().Add(time.Second)) // 1 second timeout for the dns request + defer dnsCtxCancel() - if err != nil || len(resolvedAddrs) == 0 { - return false, "The SAN DNS Name could not be resolved, denying the CSR", nil - } + var resolvedAddrs []string + resolvedAddrs, err = r.Resolver.LookupHost(dnsCtx, sanDNSName) - var setBuilder netaddr.IPSetBuilder + if err != nil || len(resolvedAddrs) == 0 { + return false, "The SAN DNS Name could not be resolved, denying the CSR", nil + } - for _, a := range resolvedAddrs { - ipa, err := netaddr.ParseIP(a) - if err != nil { - return false, fmt.Sprintf("Error while parsing resolved IP address %s, denying the CSR", ipa), nil - } + var setBuilder netaddr.IPSetBuilder - setBuilder.Add(ipa) + for _, a := range resolvedAddrs { + ipa, err := netaddr.ParseIP(a) + if err != nil { + return false, fmt.Sprintf("Error while parsing resolved IP address %s, denying the CSR", ipa), nil } - ipSet, _ := setBuilder.IPSet() + setBuilder.Add(ipa) + } - sanIPAddrs := x509cr.IPAddresses - for _, ip := range sanIPAddrs { - ipa, ok := netaddr.FromStdIP(ip) - if !ok { - return false, fmt.Sprintf("Error while x509 CR IP address %s, denying the CSR", ip), nil - } + resolvedIPSet, _ := setBuilder.IPSet() - if !ipSet.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 - } + sanIPAddrs := x509cr.IPAddresses + for _, ip := range sanIPAddrs { + ipa, ok := netaddr.FromStdIP(ip) + if !ok { + return false, fmt.Sprintf("Error while parsing x509 CR IP address %s, denying the CSR", ip), nil } - } - valid = true + 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 valid, reason, nil }