HOSTEDCP-1257: control-plane-pki-operator: add a CSR flow for break-glass creds #3267

stevekuznetsov · 2023-12-01T01:56:21Z

In order to acquire break-glass credentials, end users will not have access to the management plane from which they can read certificates. Instead, we expect them to create (by proxy) a CertificateSigningRequest that we will sign, if it is approved.

/cc @sjenning @csrwng @enxebre

openshift-ci-robot · 2023-12-01T01:56:24Z

@stevekuznetsov: This pull request references HOSTEDCP-1256 which is a valid jira issue.

In response to this:

In order to acquire break-glass credentials, end users will not have access to the management plane from which they can read certificates. Instead, we expect them to create (by proxy) a CertificateSigningRequest that we will sign, if it is approved.

/cc @sjenning @csrwng @enxebre

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

netlify · 2023-12-01T01:57:20Z

✅ Deploy Preview for hypershift-docs ready!

Name	Link
🔨 Latest commit	`e104187`
🔍 Latest deploy log	https://app.netlify.com/sites/hypershift-docs/deploys/656e3d7589347b000870bedd
😎 Deploy Preview	https://deploy-preview-3267--hypershift-docs.netlify.app/reference/api
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

stevekuznetsov · 2023-12-01T02:00:55Z

control-plane-pki-operator/certificates/authority/policies.go

@@ -0,0 +1,185 @@
+/*


Do we have a policy for wholesale copy-paste of modules? Upstream made it clear they would not be OK with exporting this.

I would at least add a comment saying where this came from

stevekuznetsov · 2023-12-01T02:02:28Z

control-plane-pki-operator/certificates/validation.go

+type ValidatorFunc func(csr *certificatesv1.CertificateSigningRequest, x509cr *x509.CertificateRequest) error
+
+// Validator returns a function that validates CertificateSigningRequests
+func Validator(hcp *hypershiftv1beta1.HostedControlPlane, signer SignerClass) ValidatorFunc {


Please review this - we're declaring what kinds of CSRs we accept here.

stevekuznetsov · 2023-12-01T02:02:54Z

control-plane-pki-operator/certificatesigningcontroller/certificatesigningcontroller.go

+	der, err := ca.Sign(x509cr.Raw, authority.PermissiveSigningPolicy{
+		TTL:      duration(certTTL, expirationSeconds),
+		Usages:   usages,
+		Backdate: backdate,       // this must always be less than the minimum TTL requested by a user (see sanity check requestedDuration below)


Please review backdating & validity - as written, it's the same as upstream k8s signers.

stevekuznetsov · 2023-12-01T02:03:31Z

control-plane-pki-operator/certificatesigningcontroller/certificatesigningcontroller.go

+	return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), nil
+}
+
+func duration(certTTL time.Duration, expirationSeconds *int32) time.Duration {


Please review duration policy - as written, it's the same as upstream k8s signers.

stevekuznetsov · 2023-12-01T02:04:24Z

control-plane-pki-operator/operator.go

+		kubeInformersForNamespaces,
+		kubeClient,
+		controllerContext.EventRecorder,
+		36*certRotationScale/24,


Please review TTL.

stevekuznetsov · 2023-12-01T02:05:07Z

hypershift-operator/controllers/manifests/controlplanepkioperator/reconcile.go

@@ -0,0 +1,80 @@
+package controlplanepkioperator


This file (and the test) don't follow the convention in this controller, but the convention is to put everything in disjoint places and many thousand-line-files so I hope that's ok

...nifests/controlplanepkioperator/testdata/zz_fixture_TestReconcileCSRApproverClusterRole.yaml

stevekuznetsov · 2023-12-01T15:15:06Z

control-plane-pki-operator/certificatesigningcontroller/certificatesigningcontroller_test.go

+
+			// signing the certificate necessarily uses cryptographic randomness, so we can't know
+			// what the output will be a priori
+			if testCase.expectedCfg != nil && testCase.expectedCfg.Status != nil && testCase.expectedCfg.Status.Certificate != nil &&


This is unfortunate but I don't know of any way for us to avoid it. Essentially we end up asserting that if the test case expected something in the signed certificate field, there is some value - any value - there.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

In order to acquire break-glass credentials, end users will not have access to the management plane from which they can read certificates. Instead, we expect them to create (by proxy) a CertificateSigningRequest that we will sign, if it is approved. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

csrwng

Some initial comments

api/hypershift/v1alpha1/certificatesigningrequestapproval_types.go

hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go

csrwng · 2023-12-04T18:40:49Z

control-plane-pki-operator/certificates/authority/policies.go

@@ -0,0 +1,185 @@
+/*


I would at least add a comment saying where this came from

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

stevekuznetsov · 2023-12-04T20:59:02Z

@csrwng thanks for the review - I'd like to hear from the client-go folks if there's a CA library I should use instead of pulling in that bit from k8s, but this is ready otherwise. Worst case, we can un-copy that in a follow-up.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

stevekuznetsov · 2023-12-06T15:48:46Z

/rtest

stevekuznetsov · 2023-12-06T15:48:49Z

/retest

stevekuznetsov · 2023-12-06T20:10:23Z

/retest

sjenning · 2023-12-08T14:28:06Z

/approve
/lgtm
/retest-required

~~Lets try to get this in before the branch~~

Branching already happened 👎

openshift-ci · 2023-12-08T14:28:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sjenning, stevekuznetsov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sjenning]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-12-08T15:17:51Z

@stevekuznetsov: This pull request references HOSTEDCP-1257 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

In order to acquire break-glass credentials, end users will not have access to the management plane from which they can read certificates. Instead, we expect them to create (by proxy) a CertificateSigningRequest that we will sign, if it is approved.

/cc @sjenning @csrwng @enxebre

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-12-08T16:06:03Z

/retest-required

Remaining retests: 0 against base HEAD 6315b72 and 2 for PR HEAD a1513fa in total

openshift-ci-robot · 2023-12-11T09:47:40Z

/retest-required

Remaining retests: 0 against base HEAD bcb3e90 and 1 for PR HEAD a1513fa in total

openshift-ci · 2023-12-11T12:18:26Z

@stevekuznetsov: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ibmcloud-iks	`a1513fa`	link	false	`/test e2e-ibmcloud-iks`
ci/prow/e2e-ibmcloud-roks	`a1513fa`	link	false	`/test e2e-ibmcloud-roks`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2023-12-11T13:30:04Z

/retest-required

Remaining retests: 0 against base HEAD 0952c61 and 0 for PR HEAD a1513fa in total

openshift-bot · 2023-12-12T10:42:39Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-hypershift-container-v4.16.0-202312121005.p0.g0f7c8c4.assembly.stream for distgit hypershift.
All builds following this will include this PR.

stevekuznetsov · 2023-12-14T13:20:58Z

/cherry-pick release-4.15

openshift-cherrypick-robot · 2023-12-14T13:21:45Z

@stevekuznetsov: new pull request created: #3324

In response to this:

/cherry-pick release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 1, 2023

openshift-ci bot requested review from csrwng, enxebre and sjenning December 1, 2023 01:56

stevekuznetsov added 2 commits November 30, 2023 18:59

vendor: pull in dependencies

82898a3

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

api,client: add the CertificateSigningRequestApproval type

37f15d5

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

stevekuznetsov force-pushed the skuznets/pki-csr branch 2 times, most recently from 3be2099 to 22eb99a Compare December 1, 2023 02:01

stevekuznetsov commented Dec 1, 2023

View reviewed changes

stevekuznetsov force-pushed the skuznets/pki-csr branch from 22eb99a to b98d5cf Compare December 1, 2023 14:37

stevekuznetsov commented Dec 1, 2023

View reviewed changes

stevekuznetsov force-pushed the skuznets/pki-csr branch 2 times, most recently from e7ce332 to 3785020 Compare December 1, 2023 19:34

control-plane-pki-operator: add a label

b7edccd

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

stevekuznetsov force-pushed the skuznets/pki-csr branch 5 times, most recently from a15269b to a845102 Compare December 2, 2023 18:08

openshift-ci bot added the area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release label Dec 2, 2023

stevekuznetsov force-pushed the skuznets/pki-csr branch 2 times, most recently from 8a64c99 to d70aba9 Compare December 2, 2023 20:24

stevekuznetsov force-pushed the skuznets/pki-csr branch 2 times, most recently from ed82750 to 9d76262 Compare December 4, 2023 01:50

stevekuznetsov force-pushed the skuznets/pki-csr branch from 9d76262 to 95c604a Compare December 4, 2023 03:32

csrwng reviewed Dec 4, 2023

View reviewed changes

api: move csra to v1beta1

a687817

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

openshift-ci bot added the area/documentation Indicates the PR includes changes for documentation label Dec 4, 2023

stevekuznetsov added 2 commits December 5, 2023 11:06

hypershift-operator: conditionally apply cppo RBAC

58da05b

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

hypershift-operator: use struct literals for clarity

a1513fa

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

stevekuznetsov force-pushed the skuznets/pki-csr branch from 67fd6e8 to a1513fa Compare December 5, 2023 18:06

openshift-ci bot assigned sjenning Dec 8, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 8, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 8, 2023

stevekuznetsov changed the title ~~HOSTEDCP-1256: control-plane-pki-operator: add a CSR flow for break-glass creds~~ HOSTEDCP-1257: control-plane-pki-operator: add a CSR flow for break-glass creds Dec 8, 2023

openshift-merge-bot bot merged commit 0f7c8c4 into openshift:main Dec 12, 2023
12 of 14 checks passed

openshift-cherrypick-robot mentioned this pull request Dec 14, 2023

[release-4.15] HOSTEDCP-1257: control-plane-pki-operator: add a CSR flow for break-glass creds #3324

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HOSTEDCP-1257: control-plane-pki-operator: add a CSR flow for break-glass creds #3267

HOSTEDCP-1257: control-plane-pki-operator: add a CSR flow for break-glass creds #3267

stevekuznetsov commented Dec 1, 2023

openshift-ci-robot commented Dec 1, 2023 •

edited by openshift-ci bot

netlify bot commented Dec 1, 2023 •

edited

stevekuznetsov Dec 1, 2023

csrwng Dec 4, 2023

stevekuznetsov Dec 1, 2023

stevekuznetsov Dec 1, 2023

stevekuznetsov Dec 1, 2023

stevekuznetsov Dec 1, 2023

stevekuznetsov Dec 1, 2023

stevekuznetsov Dec 1, 2023

csrwng left a comment

csrwng Dec 4, 2023

stevekuznetsov commented Dec 4, 2023

stevekuznetsov commented Dec 6, 2023

stevekuznetsov commented Dec 6, 2023

stevekuznetsov commented Dec 6, 2023

sjenning commented Dec 8, 2023 •

edited

openshift-ci bot commented Dec 8, 2023

openshift-ci-robot commented Dec 8, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Dec 8, 2023

openshift-ci-robot commented Dec 11, 2023

openshift-ci bot commented Dec 11, 2023 •

edited

openshift-ci-robot commented Dec 11, 2023

openshift-bot commented Dec 12, 2023

stevekuznetsov commented Dec 14, 2023

openshift-cherrypick-robot commented Dec 14, 2023

HOSTEDCP-1257: control-plane-pki-operator: add a CSR flow for break-glass creds #3267

HOSTEDCP-1257: control-plane-pki-operator: add a CSR flow for break-glass creds #3267

Conversation

stevekuznetsov commented Dec 1, 2023

openshift-ci-robot commented Dec 1, 2023 • edited by openshift-ci bot

netlify bot commented Dec 1, 2023 • edited

✅ Deploy Preview for hypershift-docs ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csrwng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevekuznetsov commented Dec 4, 2023

stevekuznetsov commented Dec 6, 2023

stevekuznetsov commented Dec 6, 2023

stevekuznetsov commented Dec 6, 2023

sjenning commented Dec 8, 2023 • edited

openshift-ci bot commented Dec 8, 2023

openshift-ci-robot commented Dec 8, 2023 • edited by openshift-ci bot

openshift-ci-robot commented Dec 8, 2023

openshift-ci-robot commented Dec 11, 2023

openshift-ci bot commented Dec 11, 2023 • edited

openshift-ci-robot commented Dec 11, 2023

openshift-bot commented Dec 12, 2023

stevekuznetsov commented Dec 14, 2023

openshift-cherrypick-robot commented Dec 14, 2023

openshift-ci-robot commented Dec 1, 2023 •

edited by openshift-ci bot

netlify bot commented Dec 1, 2023 •

edited

sjenning commented Dec 8, 2023 •

edited

openshift-ci-robot commented Dec 8, 2023 •

edited by openshift-ci bot

openshift-ci bot commented Dec 11, 2023 •

edited