Repository: kubermatic/machine-controller
Pull request #707 (2020-03-11 12:18:35)

Add additional validations in NodeCSRApprover controller
**What this PR does / why we need it**:

This PR adds additional validations in the NodeCSRApprover controller.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>` format, will close the issue(s) when PR gets merged)*:
Fixes https://github.com/kubermatic/kubermatic/issues/5177

**Optional Release Note**:
```release-note
NONE
```


kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/707#" title="Author self-approved">xmudrii</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/master/OWNERS)~~ [xmudrii]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
kubermatic-bot: @xmudrii: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-machine-controller-test | 97d22de1578305b7644fe8cb46b5eef47663177c | [link](https://public-prow.loodse.com/view/gcs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/707/pull-machine-controller-test/1237714702868418562) | `/test pull-machine-controller-test`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=707)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: @xmudrii: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-machine-controller-test | 94f9197a2c3afa3ee74b0a65078b35af115ff80f | [link](https://public-prow.loodse.com/view/gcs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/707/pull-machine-controller-test/1237735965808660480) | `/test pull-machine-controller-test`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=707)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: @xmudrii: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-machine-controller-test | 777316cc013094d6de49809d95766f498db7f4d3 | [link](https://public-prow.loodse.com/view/gcs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/707/pull-machine-controller-test/1238066271686234114) | `/test pull-machine-controller-test`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=707)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: 3dfda507ba1069da89cc7cad66781996d73b29f8</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/707#pullrequestreview-375558490" title="Approved">kron4eg</a>*, *<a href="https://github.com/kubermatic/machine-controller/pull/707#" title="Author self-approved">xmudrii</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/master/OWNERS)~~ [kron4eg,xmudrii]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
kubermatic-bot: New changes are detected. LGTM label has been removed.
xmudrii: @irozzo-1A @kron4eg The comment is addressed, PTAL.
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: d61fa2bfb66d3aabd9630e3ab4fed082b24a8afa</details>
kron4eg: /hold cancel

Patch:
diff --git a/pkg/controller/nodecsrapprover/node_csr_approver.go b/pkg/controller/nodecsrapprover/node_csr_approver.go
index 5e253953..84d71355 100644
--- a/pkg/controller/nodecsrapprover/node_csr_approver.go
+++ b/pkg/controller/nodecsrapprover/node_csr_approver.go
@@ -21,6 +21,9 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
+	"strings"
+
+	"github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
 
 	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -35,7 +38,16 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
-const ControllerName = "node_csr_autoapprover"
+const (
+	// ControllerName is name of the NodeCSRApprover controller
+	ControllerName = "node_csr_autoapprover"
+
+	nodeUser       = "system:node"
+	nodeUserPrefix = nodeUser + ":"
+
+	nodeGroup          = "system:nodes"
+	authenticatedGroup = "system:authenticated"
+)
 
 type reconciler struct {
 	client.Client
@@ -91,30 +103,41 @@ func (r *reconciler) reconcile(ctx context.Context, request reconcile.Request) e
 		}
 	}
 
-	// Ensure system:nodes is in groups
-	if !sets.NewString(csr.Spec.Groups...).Has("system:nodes") {
-		klog.V(4).Infof("Skipping reconciling CSR '%s' because 'system:nodes' is not in its groups", csr.ObjectMeta.Name)
+	// Validate the CSR object and get the node name
+	nodeName, err := r.validateCSRObject(csr)
+	if err != nil {
+		klog.V(4).Infof("Skipping reconciling CSR '%s' because CSR object is not valid: %v", csr.ObjectMeta.Name, err)
 		return nil
 	}
 
-	// Check are present usages matching allowed usages
-	if len(csr.Spec.Usages) != 3 {
-		klog.V(4).Infof("Skipping reconciling CSR '%s' because it has not exactly three usages defined", csr.ObjectMeta.Name)
-		return nil
+	// Get machine name for the appropriate node
+	machine, found, err := r.getMachineForNode(nodeName)
+	if err != nil {
+		return fmt.Errorf("failed to get machine for node '%s': %v", nodeName, err)
 	}
-	for _, usage := range csr.Spec.Usages {
-		if !isUsageInUsageList(usage, allowedUsages) {
-			klog.V(4).Infof("Skipping reconciling CSR '%s' because its usage (%v) is not in the list of allowed usages (%v)",
-				csr.ObjectMeta.Name, usage, allowedUsages)
-			return nil
-		}
+	if !found {
+		return fmt.Errorf("no machine found for given node '%s'", nodeName)
 	}
-	// Validate the CSR request
-	if err := validateCSRRequest(csr); err != nil {
-		klog.V(4).Infof("Skipping reconciling CSR '%s' because CSR request is not valid: %v", csr.ObjectMeta.Name, err)
-		return nil
+
+	// Parse the certificate request
+	csrBlock, rest := pem.Decode(csr.Spec.Request)
+	if csrBlock == nil {
+		return fmt.Errorf("no certificate request found for the given CSR")
+	}
+	if len(rest) != 0 {
+		return fmt.Errorf("found more than one PEM encoded block in the result")
+	}
+	certRequest, err := x509.ParseCertificateRequest(csrBlock.Bytes)
+	if err != nil {
+		return err
+	}
+
+	// Validate the certificate request
+	if err := r.validateX509CSR(csr, certRequest, machine); err != nil {
+		return fmt.Errorf("error validating the x509 certificate request: %v", err)
 	}
 
+	// Approve CSR
 	klog.V(4).Infof("Approving CSR %s", csr.ObjectMeta.Name)
 	approvalCondition := certificatesv1beta1.CertificateSigningRequestCondition{
 		Type:   certificatesv1beta1.CertificateApproved,
@@ -130,40 +153,104 @@ func (r *reconciler) reconcile(ctx context.Context, request reconcile.Request) e
 	return nil
 }
 
-func isUsageInUsageList(usage certificatesv1beta1.KeyUsage, usageList []certificatesv1beta1.KeyUsage) bool {
-	for _, usageListItem := range usageList {
-		if usage == usageListItem {
-			return true
-		}
+// validateCSRObject valides the CSR object and returns name of the node that requested the certificate
+func (r *reconciler) validateCSRObject(csr *certificatesv1beta1.CertificateSigningRequest) (string, error) {
+	// Get and validate the node name
+	if !strings.HasPrefix(csr.Spec.Username, nodeUserPrefix) {
+		return "", fmt.Errorf("username must have the '%s' prefix", nodeUserPrefix)
+	}
+	nodeName := strings.TrimPrefix(csr.Spec.Username, nodeUserPrefix)
+	if len(nodeName) == 0 {
+		return "", fmt.Errorf("node name is empty")
 	}
-	return false
-}
 
-// validateCSRRequest parses the CSR request and compares certificate request CommonName with CSR username, and
-// certificateRequest Organization with CSR groups.
-func validateCSRRequest(csr *certificatesv1beta1.CertificateSigningRequest) error {
-	csrBlock, rest := pem.Decode(csr.Spec.Request)
-	if csrBlock == nil {
-		return fmt.Errorf("no certificate request found for the given CSR")
+	// Ensure system:nodes and system:authenticated are in groups
+	if len(csr.Spec.Groups) < 2 {
+		return "", fmt.Errorf("there are less than 2 groups")
 	}
-	if len(rest) != 0 {
-		return fmt.Errorf("found more than one PEM encoded block in the result")
+	if !sets.NewString(csr.Spec.Groups...).HasAll(nodeGroup, authenticatedGroup) {
+		return "", fmt.Errorf("'%s' and/or '%s' are not in its groups", nodeGroup, authenticatedGroup)
 	}
-	csrRequest, err := x509.ParseCertificateRequest(csrBlock.Bytes)
-	if err != nil {
-		return err
+
+	// Check are present usages matching allowed usages
+	if len(csr.Spec.Usages) != 3 {
+		return "", fmt.Errorf("there are no exactly three usages defined")
 	}
+	for _, usage := range csr.Spec.Usages {
+		if !isUsageInUsageList(usage, allowedUsages) {
+			return "", fmt.Errorf("usage %v is not in the list of allowed usages (%v)", usage, allowedUsages)
+		}
+	}
+
+	return nodeName, nil
+}
+
+// validateX509CSR validates the certificate request by comparing CN with username,
+// and organization with groups.
+func (r *reconciler) validateX509CSR(csr *certificatesv1beta1.CertificateSigningRequest, certReq *x509.CertificateRequest, machine v1alpha1.Machine) error {
 	// Validate Subject CommonName
-	if csrRequest.Subject.CommonName != csr.Spec.Username {
-		return fmt.Errorf("commonName '%s' is different then CSR username '%s'", csrRequest.Subject.CommonName, csr.Spec.Username)
+	if certReq.Subject.CommonName != csr.Spec.Username {
+		return fmt.Errorf("commonName '%s' is different then CSR username '%s'", certReq.Subject.CommonName, csr.Spec.Username)
 	}
+
 	// Validate Subject Organization
-	if len(csrRequest.Subject.Organization) != 1 {
-		return fmt.Errorf("error validating subject organizations")
+	if len(certReq.Subject.Organization) != 1 {
+		return fmt.Errorf("expected only one organization but got %d instead", len(certReq.Subject.Organization))
+	}
+	if certReq.Subject.Organization[0] != nodeGroup {
+		return fmt.Errorf("organization '%s' doesn't match node group '%s'", certReq.Subject.Organization[0], nodeGroup)
+	}
+
+	machineAddressSet := sets.NewString(machine.Status.NodeRef.Name)
+	for _, addr := range machine.Status.Addresses {
+		machineAddressSet.Insert(addr.Address)
 	}
-	if csrRequest.Subject.Organization[0] != "system:nodes" {
-		return fmt.Errorf("expected 'system:nodes' in organization, but got '%s'", csrRequest.Subject.Organization[0])
+
+	// Validate SAN DNS names
+	for _, dns := range certReq.DNSNames {
+		if len(dns) == 0 {
+			continue
+		}
+		if !machineAddressSet.Has(dns) {
+			return fmt.Errorf("dns name '%s' cannot be associated with node '%s'", dns, machine.Status.NodeRef.Name)
+		}
+	}
+
+	// Validate SAN IP addresses
+	for _, ip := range certReq.IPAddresses {
+		if len(ip) == 0 {
+			continue
+		}
+		if !machineAddressSet.Has(ip.String()) {
+			return fmt.Errorf("ip address '%v' cannot be associated with node '%s'", ip, machine.Status.NodeRef.Name)
+		}
 	}
 
 	return nil
 }
+
+func (r *reconciler) getMachineForNode(nodeName string) (v1alpha1.Machine, bool, error) {
+	// List all Machines in all namespaces
+	machines := &v1alpha1.MachineList{}
+	listOptions := &client.ListOptions{Namespace: ""}
+	if err := r.Client.List(context.Background(), machines, listOptions); err != nil {
+		return v1alpha1.Machine{}, false, fmt.Errorf("failed to list all machine objects: %v", err)
+	}
+
+	for _, machine := range machines.Items {
+		if machine.Status.NodeRef != nil && machine.Status.NodeRef.Name == nodeName {
+			return machine, true, nil
+		}
+	}
+
+	return v1alpha1.Machine{}, false, fmt.Errorf("failed to get machine for given node name '%s'", nodeName)
+}
+
+func isUsageInUsageList(usage certificatesv1beta1.KeyUsage, usageList []certificatesv1beta1.KeyUsage) bool {
+	for _, usageListItem := range usageList {
+		if usage == usageListItem {
+			return true
+		}
+	}
+	return false
+}


Test patch:
diff --git a/pkg/controller/nodecsrapprover/node_csr_approver_test.go b/pkg/controller/nodecsrapprover/node_csr_approver_test.go
index a7e87373..df8284cd 100644
--- a/pkg/controller/nodecsrapprover/node_csr_approver_test.go
+++ b/pkg/controller/nodecsrapprover/node_csr_approver_test.go
@@ -17,102 +17,706 @@ limitations under the License.
 package nodecsrapprover
 
 import (
-	"context"
-	"sync"
+	"crypto/x509"
+	"encoding/pem"
+	"fmt"
 	"testing"
 
+	"github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
+
 	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/kubernetes/typed/certificates/v1beta1/fake"
-	fakeclienttest "k8s.io/client-go/testing"
-
-	k8sclientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
+/*
+	Certificate generation (cfssl required):
+cat <<EOF | cfssl genkey - | cfssljson -bare server
+{
+  "hosts": [
+    "ip-172-31-114-48.eu-west-3.compute.internal",
+    "192.0.2.24"
+  ],
+  "CN": "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+  "key": {
+    "algo": "ecdsa",
+    "size": 256
+  },
+  "names": [{
+	"O": "system:nodes"
+  }]
+}
+EOF
+*/
+
 const (
-	testCertificateRequest = `-----BEGIN CERTIFICATE REQUEST-----
-MIIBnTCCAUQCAQAwWTEVMBMGA1UEChMMc3lzdGVtOm5vZGVzMUAwPgYDVQQDEzdz
+	testValidCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBYzCCAQoCAQAwWTEVMBMGA1UEChMMc3lzdGVtOm5vZGVzMUAwPgYDVQQDEzdz
+eXN0ZW06bm9kZTppcC0xNzItMzEtMTE0LTQ4LmV1LXdlc3QtMy5jb21wdXRlLmlu
+dGVybmFsMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEvt0lQhUzm7HRM+nK64Uj
+B8Nm4N59ksjOUiEnH1nDamPTZXjVzhfUShKiaYTpGU11xZz2rZu97AGhZeiSfPI4
+eqBPME0GCSqGSIb3DQEJDjFAMD4wPAYDVR0RBDUwM4IraXAtMTcyLTMxLTExNC00
+OC5ldS13ZXN0LTMuY29tcHV0ZS5pbnRlcm5hbIcEwAACGDAKBggqhkjOPQQDAgNH
+ADBEAiAenX66LOL8U0C6Oo5WAQLxErmImuKpN1lRj/qeYE3sYQIgGu1UpkyNmMqM
+ZNeG8Y4JZvQejnbdkbToeB0yodC1tEE=
+-----END CERTIFICATE REQUEST-----
+`
+
+	testInvalidCommonNameCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBMDCB1wIBADAmMRUwEwYDVQQKEwxzeXN0ZW06bm9kZXMxDTALBgNVBAMTBHRl
+c3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAT4FVNaMAPVcv05WF+EQ3FbTmax
+A3dIxE938g04uFvKFV00TggRaUW/obOo8BET0ADHmxA3QWM9XfX2UNI88VRLoE8w
+TQYJKoZIhvcNAQkOMUAwPjA8BgNVHREENTAzgitpcC0xNzItMzEtMTE0LTQ4LmV1
+LXdlc3QtMy5jb21wdXRlLmludGVybmFshwTAAAIYMAoGCCqGSM49BAMCA0gAMEUC
+IQDqnqQLBH2nBVoz4P35lrSPVqAZYmg5HN145jMy3/GUFQIgIHJJ0U6sd8xbksMV
+xW5irA6nWST0ILKeIJz7BBds2K0=
+-----END CERTIFICATE REQUEST-----
+`
+
+	testMultipleOrganizationsCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBcDCCARcCAQAwZjEiMBMGA1UEChMMc3lzdGVtOm5vZGVzMAsGA1UEChMEdGVz
+dDFAMD4GA1UEAxM3c3lzdGVtOm5vZGU6aXAtMTcyLTMxLTExNC00OC5ldS13ZXN0
+LTMuY29tcHV0ZS5pbnRlcm5hbDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABNF9
+okm72Aez+e9qeylkTGYQOTfEgSRRgmgyOtkqug/QwxkR0adlyIJcKiX/DZ4Eiqhe
+tFgq4c4vBNKKg6SKxWKgTzBNBgkqhkiG9w0BCQ4xQDA+MDwGA1UdEQQ1MDOCK2lw
+LTE3Mi0zMS0xMTQtNDguZXUtd2VzdC0zLmNvbXB1dGUuaW50ZXJuYWyHBMAAAhgw
+CgYIKoZIzj0EAwIDRwAwRAIgTKGXyZ8WEoPWba8varaH1RTKjLVq8Xa0iDMQGCRx
+wDQCIBkAk4Z5KiOQMWEH8gH+cl5EVprL4bCEyPnnNF8gofpz
+-----END CERTIFICATE REQUEST-----
+`
+
+	testNoOrganizationCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBTTCB8wIBADBCMUAwPgYDVQQDEzdzeXN0ZW06bm9kZTppcC0xNzItMzEtMTE0
+LTQ4LmV1LXdlc3QtMy5jb21wdXRlLmludGVybmFsMFkwEwYHKoZIzj0CAQYIKoZI
+zj0DAQcDQgAEMA3EgzRCWIK1NbZ3WE4w3SRX4AXub49aGs1f7emlmGjs7smhN/NB
+fzbPH/eMGkVB8A10+DyuHw51XCLufUrbJKBPME0GCSqGSIb3DQEJDjFAMD4wPAYD
+VR0RBDUwM4IraXAtMTcyLTMxLTExNC00OC5ldS13ZXN0LTMuY29tcHV0ZS5pbnRl
+cm5hbIcEwAACGDAKBggqhkjOPQQDAgNJADBGAiEAtuVHYeKM8uNtJy0kFCFJmsIy
+g/X2uvkjAnSLjizhF3oCIQCfwZ5jQy5JUufOYORk0oEl7DAln10PkRFICBoDcVtt
+wQ==
+-----END CERTIFICATE REQUEST-----
+`
+
+	testInvalidOrganizationCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBWzCCAQICAQAwUTENMAsGA1UEChMEdGVzdDFAMD4GA1UEAxM3c3lzdGVtOm5v
+ZGU6aXAtMTcyLTMxLTExNC00OC5ldS13ZXN0LTMuY29tcHV0ZS5pbnRlcm5hbDBZ
+MBMGByqGSM49AgEGCCqGSM49AwEHA0IABP9SWPd4Tf8zWuzTTBratQRFwC6dXyyq
+IltYaTny5+pZxwiSzZCuLSYe1mSPQvBLjfMcMJiDIeH82oHXPehxPR+gTzBNBgkq
+hkiG9w0BCQ4xQDA+MDwGA1UdEQQ1MDOCK2lwLTE3Mi0zMS0xMTQtNDguZXUtd2Vz
+dC0zLmNvbXB1dGUuaW50ZXJuYWyHBMAAAhgwCgYIKoZIzj0EAwIDRwAwRAIgNui6
+Ch0hJUx9r3a1VTOMRa5BDJ6z02hizuSC4rMg3N4CIEmKExaOFYT5lwOs5ZYaQ4xc
+6usii8YdNQ7Y0WlreYTQ
+-----END CERTIFICATE REQUEST-----
+`
+
+	testInvalidDNSNameCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBYzCCAQoCAQAwWTEVMBMGA1UEChMMc3lzdGVtOm5vZGVzMUAwPgYDVQQDEzdz
 eXN0ZW06bm9kZTppcC0xNzItMzEtMTE0LTQ4LmV1LXdlc3QtMy5jb21wdXRlLmlu
-dGVybmFsMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEAvwSO5A7Thh4Dw1KejVf
-JwvVeTXCbe5i/AzQr1DRO2g9Hsd/3iwFc27OX29PJ6tXt9wNVG9dn+D7fLuvRIdY
-4KCBiDCBhQYJKoZIhvcNAQkOMXgwdjB0BgNVHREEbTBrgjBlYzItNTItNDctOTct
-MTExLmV1LXdlc3QtMy5jb21wdXRlLmFtYXpvbmF3cy5jb22CK2lwLTE3Mi0zMS0x
-MTQtNDguZXUtd2VzdC0zLmNvbXB1dGUuaW50ZXJuYWyHBKwfcjCHBDQvYW8wCgYI
-KoZIzj0EAwIDRwAwRAIgLdF0Ud9UHmE3Ezxovw5oafCAKYiCE/EirpNXkUHee80C
-IDjKG4ahwgDJQRtpmGqufjFBqrVVI3DxEFvt2RATJ3HA
------END CERTIFICATE REQUEST-----`
+dGVybmFsMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEI0vty8uehZOaXxjluaEi
+DaTBhB0rplD92+Mi8Xt36nCq+BmrtXBIst1Cp8jzx/AESEuC/06T1FoZnXir5H5k
+KKBPME0GCSqGSIb3DQEJDjFAMD4wPAYDVR0RBDUwM4IraXAtMTcyLTMxLTExNC00
+OC5ldS1lYXN0LTMuY29tcHV0ZS5pbnRlcm5hbIcEwAACGDAKBggqhkjOPQQDAgNH
+ADBEAiARKzqSf5A4K9mSpfN5BZkvagN2hpSuCrNh969aMHKWGgIgLMlqn6GED5cS
+rHy6Ox5XuLIEfuyO2lw4Tpse1T0MN5c=
+-----END CERTIFICATE REQUEST-----
+`
+
+	testInvalidIPAddressCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBYzCCAQoCAQAwWTEVMBMGA1UEChMMc3lzdGVtOm5vZGVzMUAwPgYDVQQDEzdz
+eXN0ZW06bm9kZTppcC0xNzItMzEtMTE0LTQ4LmV1LXdlc3QtMy5jb21wdXRlLmlu
+dGVybmFsMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5bGjubaUuMpo5IHK/nfm
+fPoG/uPDkDUtx9wugPaNGd8vn5Mu/3QMf3NuD5kXRY4RCWl9w4vddkLdT+hSekP1
+i6BPME0GCSqGSIb3DQEJDjFAMD4wPAYDVR0RBDUwM4IraXAtMTcyLTMxLTExNC00
+OC5ldS13ZXN0LTMuY29tcHV0ZS5pbnRlcm5hbIcEwAACAjAKBggqhkjOPQQDAgNH
+ADBEAiB9myW6lAcQKX2WCsfriwK3aIMzgY4jCkr8tPV1oZeLLQIgbwlqgQcAOQqC
+/6Fr0hK/AIDlykBtDKkNYDgm3n3Oqqc=
+-----END CERTIFICATE REQUEST-----
+`
+
+	testAdditionalDNSNameCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBkTCCATcCAQAwWTEVMBMGA1UEChMMc3lzdGVtOm5vZGVzMUAwPgYDVQQDEzdz
+eXN0ZW06bm9kZTppcC0xNzItMzEtMTE0LTQ4LmV1LXdlc3QtMy5jb21wdXRlLmlu
+dGVybmFsMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEA9/qlNDkHRRv/MMvqGDv
+glwVIepZ1nXpA72XmwT9eKGKIGhvYkU5y6HkpkLv45niTxhhYN4EbSwAC7651ZsK
+6qB8MHoGCSqGSIb3DQEJDjFtMGswaQYDVR0RBGIwYIIraXAtMTcyLTMxLTExNC00
+OC5ldS13ZXN0LTMuY29tcHV0ZS5pbnRlcm5hbIIraXAtMTcyLTMxLTExNC00OC5l
+dS1lYXN0LTMuY29tcHV0ZS5pbnRlcm5hbIcEwAACGDAKBggqhkjOPQQDAgNIADBF
+AiAr5gSITnVYXOFmvEU+mcHk8w/8/cbKwMHWtLZ0ZLN+igIhAP0BE//qz0lAFINS
+Su19WQIpe9VR/dGJvXfsgaLjyPyJ
+-----END CERTIFICATE REQUEST-----
+`
+
+	testAdditionalIPAddressCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBazCCARACAQAwWTEVMBMGA1UEChMMc3lzdGVtOm5vZGVzMUAwPgYDVQQDEzdz
+eXN0ZW06bm9kZTppcC0xNzItMzEtMTE0LTQ4LmV1LXdlc3QtMy5jb21wdXRlLmlu
+dGVybmFsMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEPg/O69m6AjfXB8XoKjAx
+2RqAdKzt1aJ9u0rbOcoa2QkTcFeMY2GbAZ/6IVWnTDsqORJeJrHP5WYNUFgBjQ2c
+gaBVMFMGCSqGSIb3DQEJDjFGMEQwQgYDVR0RBDswOYIraXAtMTcyLTMxLTExNC00
+OC5ldS13ZXN0LTMuY29tcHV0ZS5pbnRlcm5hbIcEwAACGIcEwAACGTAKBggqhkjO
+PQQDAgNJADBGAiEA3HDpVRYYgcmdCzq5o6mwkMzegZ0P0aZNPdCQLyJt3GoCIQDo
+/iL1+piFJXAOI2GjsZNpeQJ4rPJ7l/t95tzgcVAUtw==
+-----END CERTIFICATE REQUEST-----
+`
+
+	testNoDNSNameCSR = `-----BEGIN CERTIFICATE REQUEST-----
+MIIBNjCB3QIBADBZMRUwEwYDVQQKEwxzeXN0ZW06bm9kZXMxQDA+BgNVBAMTN3N5
+c3RlbTpub2RlOmlwLTE3Mi0zMS0xMTQtNDguZXUtd2VzdC0zLmNvbXB1dGUuaW50
+ZXJuYWwwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATjbIZVQA/A+lki63bMCgVR
+UrkCqtVufWtKU5KPTD9BGZ6/dNW/uUdVftLe7Vp4+8cp2YoZIQA5NfMGYMtcE9PV
+oCIwIAYJKoZIhvcNAQkOMRMwETAPBgNVHREECDAGhwTAAAIYMAoGCCqGSM49BAMC
+A0gAMEUCIQDxZpheeDW9azYm0T0OoOyFCFslJsRf9uvJM66byWr0KwIgduhJVRog
+ELloyxg3KHxPJi8TYkTPkB+Pc9XiUGmhkzE=
+-----END CERTIFICATE REQUEST-----
+`
 )
 
-func TestReconciler_Reconcile(t *testing.T) {
-	reaction := &fakeApproveReaction{}
-	simpleReactor := &fakeclienttest.SimpleReactor{
-		Verb:     "*",
-		Resource: "*",
-		Reaction: reaction.approveReaction,
-	}
+func TestValidateCSRObject(t *testing.T) {
 	testCases := []struct {
-		name       string
-		reconciler reconciler
+		name     string
+		csr      *certificatesv1beta1.CertificateSigningRequest
+		nodeName string
+		err      error
 	}{
 		{
-			name: "test approving a created certificate",
-			reconciler: reconciler{
-				Client: k8sclientfake.NewFakeClient(&certificatesv1beta1.CertificateSigningRequest{
-					ObjectMeta: metav1.ObjectMeta{
-						ResourceVersion: "123456",
-						Name:            "csr",
-						Namespace:       metav1.NamespaceSystem,
-					},
-					Spec: certificatesv1beta1.CertificateSigningRequestSpec{
-						Request: []byte(testCertificateRequest),
-						Usages: []certificatesv1beta1.KeyUsage{
-							certificatesv1beta1.UsageDigitalSignature,
-							certificatesv1beta1.UsageKeyEncipherment,
-							certificatesv1beta1.UsageServerAuth,
-						},
-						Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
-						Groups: []string{
-							"system:nodes",
-						},
+			name: "test validating a valid csr",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testValidCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
 					},
-				}),
-				certClient: &fake.FakeCertificateSigningRequests{
-					Fake: &fake.FakeCertificatesV1beta1{
-						Fake: &fakeclienttest.Fake{
-							RWMutex:       sync.RWMutex{},
-							ReactionChain: []fakeclienttest.Reactor{simpleReactor},
-						},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			nodeName: "ip-172-31-114-48.eu-west-3.compute.internal",
+			err:      nil,
+		},
+		{
+			name: "test validating a valid csr with more than 2 groups",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testValidCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+						"test-group",
+					},
+				},
+			},
+			nodeName: "ip-172-31-114-48.eu-west-3.compute.internal",
+			err:      nil,
+		},
+		{
+			name: "test validating an invalid csr (missing prefix)",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Username: "node:ip-172-31-114-48.eu-west-3.compute.internal",
+				},
+			},
+			nodeName: "",
+			err:      fmt.Errorf("username must have the '%s' prefix", nodeUserPrefix),
+		},
+		{
+			name: "validate with missing node name in username",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Username: "system:node:",
+				},
+			},
+			nodeName: "",
+			err:      fmt.Errorf("node name is empty"),
+		},
+		{
+			name: "validate csr with with missing system:nodes group",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:authenticated",
+					},
+				},
+			},
+			nodeName: "",
+			err:      fmt.Errorf("there are less than 2 groups"),
+		},
+		{
+			name: "validate csr with two invalid groups",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"group-1",
+						"group-2",
 					},
 				},
 			},
+			nodeName: "",
+			err:      fmt.Errorf("'%s' and/or '%s' are not in its groups", nodeGroup, authenticatedGroup),
+		},
+		{
+			name: "validate csr with less than 3 usages",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testValidCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			nodeName: "",
+			err:      fmt.Errorf("there are no exactly three usages defined"),
+		},
+		{
+			name: "validate csr with more than 3 usages",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testValidCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+						certificatesv1beta1.UsageClientAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			nodeName: "",
+			err:      fmt.Errorf("there are no exactly three usages defined"),
+		},
+		{
+			name: "validate csr with usages not matching expected usages",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testValidCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageClientAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			nodeName: "",
+			err:      fmt.Errorf("usage %v is not in the list of allowed usages (%v)", "client auth", allowedUsages),
 		},
 	}
-
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			ctx := context.Background()
-			if err := tc.reconciler.reconcile(ctx, reconcile.Request{
-				NamespacedName: types.NamespacedName{Name: "csr", Namespace: metav1.NamespaceSystem}}); err != nil {
-				t.Fatalf("failed executing test: %v", err)
+			r := &reconciler{}
+			nodeName, err := r.validateCSRObject(tc.csr)
+			if err != nil && err.Error() != tc.err.Error() {
+				t.Errorf("expected error '%v', but got '%v'", tc.err, err)
+			} else if err == nil && tc.err != nil {
+				t.Errorf("expected error '%v'", tc.err)
 			}
-
-			for _, cond := range reaction.expectedCSR.Status.Conditions {
-				if cond.Type != certificatesv1beta1.CertificateApproved {
-					t.Fatalf("failed updating csr condition")
-				}
+			if nodeName != tc.nodeName {
+				t.Errorf("expected node name '%s', but got '%s'", tc.nodeName, nodeName)
 			}
 		})
 	}
 }
 
-type fakeApproveReaction struct {
-	expectedCSR *certificatesv1beta1.CertificateSigningRequest
-}
+func TestValidateX509CSR(t *testing.T) {
+	machine := v1alpha1.Machine{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-machine",
+			Namespace: metav1.NamespaceSystem,
+		},
+		Spec: v1alpha1.MachineSpec{},
+		Status: v1alpha1.MachineStatus{
+			NodeRef: &corev1.ObjectReference{
+				APIVersion: "v1",
+				Kind:       "Node",
+				Name:       "ip-172-31-114-48.eu-west-3.compute.internal",
+			},
+			Addresses: []corev1.NodeAddress{
+				{
+					Address: "ip-172-31-114-48.eu-west-3.compute.internal",
+					Type:    corev1.NodeExternalDNS,
+				},
+				{
+					Address: "192.0.2.24",
+					Type:    corev1.NodeExternalIP,
+				},
+			},
+		},
+	}
+
+	testCases := []struct {
+		name    string
+		csr     *certificatesv1beta1.CertificateSigningRequest
+		machine v1alpha1.Machine
+		err     error
+	}{
+		{
+			name: "validate valid csr",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testValidCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     nil,
+		},
+		{
+			name: "validate csr with no dns name in machine object",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testNoDNSNameCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: v1alpha1.Machine{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-machine",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: v1alpha1.MachineSpec{},
+				Status: v1alpha1.MachineStatus{
+					NodeRef: &corev1.ObjectReference{
+						APIVersion: "v1",
+						Kind:       "Node",
+						Name:       "ip-172-31-114-48.eu-west-3.compute.internal",
+					},
+					Addresses: []corev1.NodeAddress{
+						{
+							Address: "192.0.2.24",
+							Type:    corev1.NodeExternalIP,
+						},
+					},
+				},
+			},
+			err: nil,
+		},
+		{
+			name: "validate csr with different common name than username",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testInvalidCommonNameCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("commonName '%s' is different then CSR username '%s'", "test", "system:node:ip-172-31-114-48.eu-west-3.compute.internal"),
+		},
+		{
+			name: "validate csr with different username than common name",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testValidCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "test-username",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("commonName '%s' is different then CSR username '%s'", "system:node:ip-172-31-114-48.eu-west-3.compute.internal", "test-username"),
+		},
+		{
+			name: "validate csr with multiple organizations",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testMultipleOrganizationsCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("expected only one organization but got %d instead", 2),
+		},
+		{
+			name: "validate csr with no organization",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testNoOrganizationCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("expected only one organization but got %d instead", 0),
+		},
+		{
+			name: "validate csr with organization not matching system:nodes",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testInvalidOrganizationCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("organization '%s' doesn't match node group '%s'", "test", nodeGroup),
+		},
+		{
+			name: "validate csr with invalid dns name",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testInvalidDNSNameCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("dns name '%s' cannot be associated with node '%s'", "ip-172-31-114-48.eu-east-3.compute.internal", machine.Status.NodeRef.Name),
+		},
+		{
+			name: "validate csr with invalid ip address",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testInvalidIPAddressCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("ip address '%v' cannot be associated with node '%s'", "192.0.2.2", machine.Status.NodeRef.Name),
+		},
+		{
+			name: "validate csr with dns name not defined in the machine object",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testAdditionalDNSNameCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("dns name '%s' cannot be associated with node '%s'", "ip-172-31-114-48.eu-east-3.compute.internal", machine.Status.NodeRef.Name),
+		},
+		{
+			name: "validate csr with ip address not defined in the machine object",
+			csr: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "csr",
+					Namespace: metav1.NamespaceSystem,
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					Request: []byte(testAdditionalIPAddressCSR),
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageServerAuth,
+					},
+					Username: "system:node:ip-172-31-114-48.eu-west-3.compute.internal",
+					Groups: []string{
+						"system:nodes",
+						"system:authenticated",
+					},
+				},
+			},
+			machine: machine,
+			err:     fmt.Errorf("ip address '%v' cannot be associated with node '%s'", "192.0.2.25", machine.Status.NodeRef.Name),
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			r := &reconciler{}
 
-func (f *fakeApproveReaction) approveReaction(action fakeclienttest.Action) (bool, runtime.Object, error) {
-	f.expectedCSR = action.(fakeclienttest.UpdateActionImpl).Object.(*certificatesv1beta1.CertificateSigningRequest)
-	return true, f.expectedCSR, nil
+			// Parse the certificate request
+			csrBlock, rest := pem.Decode(tc.csr.Spec.Request)
+			if csrBlock == nil {
+				t.Fatal("no certificate request found for the given CSR")
+			}
+			if len(rest) != 0 {
+				t.Fatal("found more than one PEM encoded block in the result")
+			}
+			certReq, err := x509.ParseCertificateRequest(csrBlock.Bytes)
+			if err != nil {
+				t.Fatalf("failed to parse x509 certificate request: %v", err)
+			}
+
+			err = r.validateX509CSR(tc.csr, certReq, tc.machine)
+			if err != nil && err.Error() != tc.err.Error() {
+				t.Errorf("expected error '%v', but got '%v'", tc.err, err)
+			} else if err == nil && tc.err != nil {
+				t.Errorf("expected error '%v'", tc.err)
+			}
+		})
+	}
 }