NO-JIRA: csr: fix RequestCommonNameFilter and the self-signing denial path

pkg/operator/csr/csr_approver.go

@@ -113,7 +113,7 @@ func (c *csrApproverController) sync(ctx context.Context, syncCtx factory.SyncCo
 	}
 
 	if x509CSR.Subject.CommonName == csr.Spec.Username {
-		c.denyCSR(ctx, csrCopy, "IllegitimateRequester", "requester cannot request certificates for themselves", syncCtx.Recorder())
+		return c.denyCSR(ctx, csrCopy, "IllegitimateRequester", "requester cannot request certificates for themselves", syncCtx.Recorder())
 	}
 
 	csrDecision, denyReason, err := c.csrApprover.Approve(csr, x509CSR)
@@ -283,8 +283,14 @@ func NewRequestCommonNameFilter(commonNames ...string) *RequestCommonNameFilter
 	return &RequestCommonNameFilter{sets.New(commonNames...)}
 }
 
-func (f *RequestCommonNameFilter) Match(csr *certapiv1.CertificateSigningRequest) bool {
-	x509CSR, err := x509.ParseCertificateRequest(csr.Spec.Request)
+func (f *RequestCommonNameFilter) Matches(csr *certapiv1.CertificateSigningRequest) bool {
+	csrPEM, _ := pem.Decode(csr.Spec.Request)
+	if csrPEM == nil {
+		klog.V(4).Infof("failed to PEM-parse the CSR block in .spec.request of %q: no CSRs were found", csr.Name)
+		return false
+	}
+
+	x509CSR, err := x509.ParseCertificateRequest(csrPEM.Bytes)
 	if err != nil {
 		klog.V(4).Infof("failed to parse the CSR .spec.request of %q: %v", csr.Name, err)
 		return false

pkg/operator/csr/csr_approver_test.go

@@ -399,3 +399,53 @@ func (c fakeSyncContext) QueueKey() string {
 func (c fakeSyncContext) Recorder() events.Recorder {
 	return c.eventRecorder
 }
+
+func TestRequestCommonNameFilter(t *testing.T) {
+	// the filter has to satisfy CSRFilter to be usable in a filter chain
+	var _ CSRFilter = NewRequestCommonNameFilter("whatever")
+
+	tests := []struct {
+		name        string
+		commonNames []string
+		request     []byte
+		want        bool
+	}{
+		{
+			name:        "PEM-armored CSR with a listed CN",
+			commonNames: []string{"someone-else", "therealyou"},
+			request:     genCSR(t, "therealyou"),
+			want:        true,
+		},
+		{
+			name:        "PEM-armored CSR with an unlisted CN",
+			commonNames: []string{"therealyou"},
+			request:     genCSR(t, "someone-else"),
+			want:        false,
+		},
+		{
+			name:        "no PEM block at all",
+			commonNames: []string{"therealyou"},
+			request:     []byte("this is not a PEM block"),
+			want:        false,
+		},
+		{
+			name:        "empty request",
+			commonNames: []string{"therealyou"},
+			request:     nil,
+			want:        false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := NewRequestCommonNameFilter(tt.commonNames...)
+			got := f.Matches(&certapiv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-csr"},
+				Spec:       certapiv1.CertificateSigningRequestSpec{Request: tt.request},
+			})
+			if got != tt.want {
+				t.Errorf("Matches() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}