Add bootstrap phase check

Author: Matt Rogers

csr_check.go

@@ -17,58 +17,33 @@ const (
 	nodeUserPrefix = nodeUser + ":"
 )
 
-// authorizeCSR authorizes the CertificateSigningRequest req for a node's server certificate.
-// csr should be the parsed CSR from req.Spec.Request. Names contained in the CSR are checked against addresses in the
-// corresponding node's machine status.
-func authorizeCSR(machineList *capiclient.MachineList, req *certificatesv1beta1.CertificateSigningRequest, csr *x509.CertificateRequest) bool {
-	if machineList == nil || len(machineList.Items) < 1 || req == nil || csr == nil {
-		return false
+func validateCSRContents(req *certificatesv1beta1.CertificateSigningRequest, csr *x509.CertificateRequest) (string, bool) {
+	if !strings.HasPrefix(req.Spec.Username, nodeUserPrefix) {
+		return "",false
 	}
 
-	csrSubject := csr.Subject
-	csrIPSan := csr.IPAddresses
-	csrDNSSan := csr.DNSNames
-
-	// Check username: system:node:ip-10-0-152-205.ec2.internal
-	userName := req.Spec.Username
-	if !strings.HasPrefix(userName, nodeUserPrefix) {
-		return false
-	}
-
-	nodeAsking := strings.TrimPrefix(userName, nodeUserPrefix)
+	nodeAsking := strings.TrimPrefix(req.Spec.Username, nodeUserPrefix)
 	if len(nodeAsking) < 1 {
-		return false
-	}
-
-	// Check that we have a registered node with the request name
-	var targetMachine *capiclient.MachineStatus
-	for _, machine := range machineList.Items {
-		if machine.Status.NodeRef != nil && machine.Status.NodeRef.Name == nodeAsking {
-			targetMachine = machine.Status.DeepCopy()
-			break
-		}
-	}
-	if targetMachine == nil {
-		return false
+		return "",false
 	}
 
 	// Check groups, we need at least:
 	// - system:nodes
 	// - system:authenticated
 	if len(req.Spec.Groups) < 2 {
-		return false
+		return "",false
 	}
 	groupSet := sets.NewString(req.Spec.Groups...)
 	if !groupSet.HasAll(nodeGroup, "system:authenticated") {
-		return false
+		return "",false
 	}
 
 	// Check usages, we need only:
 	// - digital signature
 	// - key encipherment
 	// - server auth
 	if len(req.Spec.Usages) != 3 {
-		return false
+		return "",false
 	}
 
 	usages := make([]string, 0)
@@ -78,7 +53,7 @@ func authorizeCSR(machineList *capiclient.MachineList, req *certificatesv1beta1.
 
 	// No extra usages!
 	if len(usages) != 3 {
-		return false
+		return "",false
 	}
 
 	usageSet := sets.NewString(usages...)
@@ -87,29 +62,56 @@ func authorizeCSR(machineList *capiclient.MachineList, req *certificatesv1beta1.
 		string(certificatesv1beta1.UsageKeyEncipherment),
 		string(certificatesv1beta1.UsageServerAuth),
 	) {
-		return false
+		return "",false
 	}
 
 	// Check subject: O = system:nodes, CN = system:node:ip-10-0-152-205.ec2.internal
-	if csrSubject.CommonName != userName {
-		return false
+	if csr.Subject.CommonName != req.Spec.Username {
+		return "",false
 	}
 
 	var hasOrg bool
-	for i := range csrSubject.Organization {
-		if csrSubject.Organization[i] == nodeGroup {
+	for i := range csr.Subject.Organization {
+		if csr.Subject.Organization[i] == nodeGroup {
 			hasOrg = true
 			break
 		}
 	}
 	if !hasOrg {
+		return "",false
+	}
+
+	return nodeAsking, true
+}
+
+// authorizeCSR authorizes the CertificateSigningRequest req for a node's server certificate.
+// csr should be the parsed CSR from req.Spec.Request. Names contained in the CSR are checked against addresses in the
+// corresponding node's machine status.
+func authorizeCSR(machineList *capiclient.MachineList, req *certificatesv1beta1.CertificateSigningRequest, csr *x509.CertificateRequest) bool {
+	if machineList == nil || len(machineList.Items) < 1 || req == nil || csr == nil {
+		return false
+	}
+
+	nodeAsking, ok := validateCSRContents(req, csr)
+	if !ok {
+		return false
+	}
+	// Check that we have a registered node with the request name
+	var targetMachine *capiclient.MachineStatus
+	for _, machine := range machineList.Items {
+		if machine.Status.NodeRef != nil && machine.Status.NodeRef.Name == nodeAsking {
+			targetMachine = machine.Status.DeepCopy()
+			break
+		}
+	}
+	if targetMachine == nil {
 		return false
 	}
 
 	// SAN checks for both DNS and IPs, e.g.,
 	// DNS:ip-10-0-152-205, DNS:ip-10-0-152-205.ec2.internal, IP Address:10.0.152.205, IP Address:10.0.152.205
 	// All names in the request must correspond to addresses assigned to a single machine.
-	for _, san := range csrDNSSan {
+	for _, san := range csr.DNSNames {
 		if len(san) < 1 {
 			continue
 		}
@@ -129,7 +131,7 @@ func authorizeCSR(machineList *capiclient.MachineList, req *certificatesv1beta1.
 		}
 	}
 
-	for _, san := range csrIPSan {
+	for _, san := range csr.IPAddresses {
 		if len(san) < 1 {
 			continue
 		}

main.go

@@ -103,34 +103,43 @@ func (c *Controller) handleNewCSR(key string) error {
 		return nil
 	}
 
-	var machineListObj runtime.Object
-	err = c.clientset.RESTClient().Get().Namespace("openshift-cluster-api").Name("machines").Do().Into(machineListObj)
-	if err != nil {
-		glog.Infof("error fetching machine list: %v", err)
-		return nil
-	}
-	machineList, ok := machineListObj.(*capiclient.MachineList)
-	if !ok {
-		glog.Infof("fetched machineList but object is not the expected type")
-		return nil
-	}
-
 	parsedCSR, err := csrclient.ParseCSR(csr)
 	if err != nil {
 		glog.Infof("error parsing request CSR: %v", err)
 		return nil
 	}
 
-	if !authorizeCSR(machineList, csr, parsedCSR) {
-		// Don't deny since it might be someone else's CSR
-		glog.Infof("CSR %s not authorized", csr.GetName())
-		return nil
+	approvalMsg := "This CSR was approved by the Node CSR Approver"
+	var machineListObj runtime.Object
+	err = c.clientset.RESTClient().Get().Namespace("openshift-cluster-api").Name("machines").Do().Into(machineListObj)
+	if err == nil {
+		machineList, ok := machineListObj.(*capiclient.MachineList)
+		if !ok {
+			glog.Infof("fetched machineList but object is not the expected type")
+			return nil
+		}
+
+		if !authorizeCSR(machineList, csr, parsedCSR) {
+			// Don't deny since it might be someone else's CSR
+			glog.Infof("CSR %s not authorized", csr.GetName())
+			return nil
+		}
+	}
+	if err != nil {
+		glog.Infof("machine api not available: %v", err)
+		// Validate the CSR for the bootstrapping phase without a SAN check.
+		_, ok := validateCSRContents(csr, parsedCSR)
+		if !ok {
+			glog.Infof("CSR %s not valid", csr.GetName())
+			return nil
+		}
+		approvalMsg += " (no SAN validation)"
 	}
 
 	csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1beta1.CertificateSigningRequestCondition{
 		Type:           certificatesv1beta1.CertificateApproved,
 		Reason:         "NodeCSRApprove",
-		Message:        "This CSR was approved by the Node CSR Approver.",
+		Message:        approvalMsg,
 		LastUpdateTime: metav1.Now(),
 	})