OCPBUGS-49769: Validate the key and cert content before accepting #669

rikatz · 2025-09-01T17:37:45Z

This change enforces the validation that a private key does not contain any certificate/public key, and that a public key does not contain any private key.

This is required because during the .pem file assembling for HAProxy, in case a certificate is present on the private key field, it will be ignored by the PEM sanitization function.

This lack of validation can cause a situation where the .pem file of HAProxy will be created as private key -> CA chain -> leaf certificate, which is invalid and rejected by HAProxy.

As the right order is private key -> leaf certificate -> CA Chain, and the proper ordering of the public key/CA certificate is already guaranteed by X509KeyPair(), we can reject the misusage of private keys or certificates on the wrong fields, that can cause HAProxy to stop

Additionally, some tests were added to validate that PEM files exported from a P12 and that contain bag attributes like friendlyNames are valid and that the process of sanitization of these files will clean them correctly and process they rest of the certificates and private keys.

As an example, the lack of validation would cause the following invalid PEM file to be added to HAProxy and make it get stuck on reconfiguration:

-----BEGIN RSA PRIVATE KEY-----
.....
-----END RSA PRIVATE KEY-----
-----BEGIN CERTIFICATE-----
.... this is the CA chain and not the leaf cert
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
... this is the leaf cert but it should happen right after the private key
-----END CERTIFICATE-----

EDIT: test executed:

created an invalid ingress with invalid key (the key contains the CA chain)

router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.585955       1 extended_validator.go:52] "msg"="skipping route due to invalid configuration" "error"="spec.tls.key: Invalid value: \"redacted key data\": field contains invalid types CERTIFICATE, expecting only PRIVATE KEY" "logger"="controller" "route"="ricardo2/test2-jnszn"
router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.586000       1 router_controller.go:273] invalid route configuration
router-default-6f5d7fcdfb-bghqr router E0901 19:25:43.585863       1 router_controller.go:273] invalid route configuration
router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.599418       1 extended_validator.go:52] "msg"="skipping route due to invalid configuration" "error"="spec.tls.key: Invalid value: \"redacted key data\": field contains invalid types CERTIFICATE, expecting only PRIVATE KEY" "logger"="controller" "route"="ricardo2/test2-jnszn"
router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.599449       1 router_controller.go:273] invalid route configuration
router-default-6f5d7fcdfb-bghqr router E0901 19:25:43.598906       1 extended_validator.go:52] "msg"="skipping route due to invalid configuration" "error"="spec.tls.key: Invalid value: \"redacted key data\": field contains invalid types CERTIFICATE, expecting only PRIVATE KEY" "logger"="controller" "route"="ricardo2/test2-jnszn"
router-default-6f5d7fcdfb-bghqr router E0901 19:25:43.598939       1 router_controller.go:273] invalid route configuration

check the route:

kubectl get route
NAME          HOST/PORT                  PATH   SERVICES     PORT    TERMINATION     WILDCARD
test2-jnszn   ExtendedValidationFailed   /      hello-node   <all>   edge/Redirect   None
....
status:
  ingress:
  - conditions:
    - lastTransitionTime: "2025-09-01T19:25:43Z"
      message: 'spec.tls.key: Invalid value: "redacted key data": field contains invalid
        types CERTIFICATE, expecting only PRIVATE KEY'
      reason: ExtendedValidationFailed
      status: "False"
      type: Admitted
    host: something2-ricardo2.apps.lab.tld
    routerCanonicalHostname: router-default.apps.lab.tld
    routerName: default
    wildcardPolicy: None

This change enforces the validation that a private key does not contain any certificate/public key, and that a public key does not contain any private key. This is required because during the .pem file assembling for HAProxy, in case a certificate is present on the private key field, it will be ignored by the PEM sanitization function. This lack of validation can cause a situation where the .pem file of HAProxy will be created as private key -> CA chain -> leaf certificate, which is invalid and rejected by HAProxy. As the right order is private key -> leaf certificate -> CA Chain, and the proper ordering of the public key/CA certificate is already guaranteed by X509KeyPair(), we can reject the misusage of private keys or certificates on the wrong fields, that can cause HAProxy to stop Additionally, some tests were added to validate that PEM files exported from a P12 and that contain bag attributes like friendlyNames are valid and that the process of sanitization of these files will clean them correctly and process they rest of the certificates and private keys

openshift-ci-robot · 2025-09-01T17:37:51Z

@rikatz: This pull request references Jira Issue OCPBUGS-49769, which is invalid:

expected the bug to target the "4.20.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

This change enforces the validation that a private key does not contain any certificate/public key, and that a public key does not contain any private key.

This is required because during the .pem file assembling for HAProxy, in case a certificate is present on the private key field, it will be ignored by the PEM sanitization function.

This lack of validation can cause a situation where the .pem file of HAProxy will be created as private key -> CA chain -> leaf certificate, which is invalid and rejected by HAProxy.

As the right order is private key -> leaf certificate -> CA Chain, and the proper ordering of the public key/CA certificate is already guaranteed by X509KeyPair(), we can reject the misusage of private keys or certificates on the wrong fields, that can cause HAProxy to stop

Additionally, some tests were added to validate that PEM files exported from a P12 and that contain bag attributes like friendlyNames are valid and that the process of sanitization of these files will clean them correctly and process they rest of the certificates and private keys.

As an example, the lack of validation would cause the following invalid PEM file to be added to HAProxy and make it get stuck on reconfiguration:
-----BEGIN RSA PRIVATE KEY-----
.....
-----END RSA PRIVATE KEY-----
-----BEGIN CERTIFICATE-----
.... this is the CA chain and not the leaf cert
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
... this is the leaf cert but it should happen right after the private key
-----END CERTIFICATE-----

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 openshift-eng/jira-lifecycle-plugin repository.

rikatz · 2025-09-01T17:39:17Z

/jira refresh

openshift-ci-robot · 2025-09-01T17:39:21Z

@rikatz: This pull request references Jira Issue OCPBUGS-49769, which is invalid:

expected the bug to target either version "4.20." or "openshift-4.20.", but it targets "4.21" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

rikatz · 2025-09-01T17:41:12Z

/jira refresh

openshift-ci-robot · 2025-09-01T17:41:21Z

@rikatz: This pull request references Jira Issue OCPBUGS-49769, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.20.0) matches configured target version for branch (4.20.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @melvinjoseph86

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-09-01T19:27:48Z

@rikatz: This pull request references Jira Issue OCPBUGS-49769, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.20.0) matches configured target version for branch (4.20.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @melvinjoseph86

In response to this:

This change enforces the validation that a private key does not contain any certificate/public key, and that a public key does not contain any private key.

This is required because during the .pem file assembling for HAProxy, in case a certificate is present on the private key field, it will be ignored by the PEM sanitization function.

This lack of validation can cause a situation where the .pem file of HAProxy will be created as private key -> CA chain -> leaf certificate, which is invalid and rejected by HAProxy.

As the right order is private key -> leaf certificate -> CA Chain, and the proper ordering of the public key/CA certificate is already guaranteed by X509KeyPair(), we can reject the misusage of private keys or certificates on the wrong fields, that can cause HAProxy to stop

Additionally, some tests were added to validate that PEM files exported from a P12 and that contain bag attributes like friendlyNames are valid and that the process of sanitization of these files will clean them correctly and process they rest of the certificates and private keys.

As an example, the lack of validation would cause the following invalid PEM file to be added to HAProxy and make it get stuck on reconfiguration:
-----BEGIN RSA PRIVATE KEY-----
.....
-----END RSA PRIVATE KEY-----
-----BEGIN CERTIFICATE-----
.... this is the CA chain and not the leaf cert
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
... this is the leaf cert but it should happen right after the private key
-----END CERTIFICATE-----
EDIT: test executed:

created an invalid ingress with invalid key (the key contains the CA chain)
router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.585955       1 extended_validator.go:52] "msg"="skipping route due to invalid configuration" "error"="spec.tls.key: Invalid value: \"redacted key data\": field contains invalid types CERTIFICATE, expecting only PRIVATE KEY" "logger"="controller" "route"="ricardo2/test2-jnszn"
router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.586000       1 router_controller.go:273] invalid route configuration
router-default-6f5d7fcdfb-bghqr router E0901 19:25:43.585863       1 router_controller.go:273] invalid route configuration
router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.599418       1 extended_validator.go:52] "msg"="skipping route due to invalid configuration" "error"="spec.tls.key: Invalid value: \"redacted key data\": field contains invalid types CERTIFICATE, expecting only PRIVATE KEY" "logger"="controller" "route"="ricardo2/test2-jnszn"
router-default-6f5d7fcdfb-rs5bg router E0901 19:25:43.599449       1 router_controller.go:273] invalid route configuration
router-default-6f5d7fcdfb-bghqr router E0901 19:25:43.598906       1 extended_validator.go:52] "msg"="skipping route due to invalid configuration" "error"="spec.tls.key: Invalid value: \"redacted key data\": field contains invalid types CERTIFICATE, expecting only PRIVATE KEY" "logger"="controller" "route"="ricardo2/test2-jnszn"
router-default-6f5d7fcdfb-bghqr router E0901 19:25:43.598939       1 router_controller.go:273] invalid route configuration
check the route:
kubectl get route
NAME          HOST/PORT                  PATH   SERVICES     PORT    TERMINATION     WILDCARD
test2-jnszn   ExtendedValidationFailed   /      hello-node   <all>   edge/Redirect   None
....
status:
 ingress:
 - conditions:
   - lastTransitionTime: "2025-09-01T19:25:43Z"
     message: 'spec.tls.key: Invalid value: "redacted key data": field contains invalid
       types CERTIFICATE, expecting only PRIVATE KEY'
     reason: ExtendedValidationFailed
     status: "False"
     type: Admitted
   host: something2-ricardo2.apps.lab.tld
   routerCanonicalHostname: router-default.apps.lab.tld
   routerName: default
   wildcardPolicy: None

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 openshift-eng/jira-lifecycle-plugin repository.

rikatz · 2025-09-02T14:37:50Z

/retest

pkg/router/routeapihelpers/validation_test.go

rikatz · 2025-09-02T19:11:27Z

/hold

I will try to write some e2e test for these cases as well, + some unit tests on different sets as per discussed with @Miciah

Miciah · 2025-09-03T15:01:28Z

/assign

Thealisyed · 2025-09-03T15:07:55Z

/assign @alebedev87 @candita @Thealisyed

openshift-ci · 2025-09-04T14:23:41Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from alebedev87. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

rikatz · 2025-09-05T14:01:33Z

/retest

openshift-ci · 2025-09-05T18:25:44Z

@rikatz: The following test 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-aws-serial	`b13ee67`	link	true	`/test e2e-aws-serial`

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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Sep 1, 2025

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Sep 1, 2025

openshift-ci bot requested review from Miciah and Thealisyed September 1, 2025 17:38

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 1, 2025

openshift-ci bot requested a review from melvinjoseph86 September 1, 2025 17:41

alebedev87 reviewed Sep 2, 2025

View reviewed changes

pkg/router/routeapihelpers/validation_test.go Show resolved Hide resolved

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 2, 2025

openshift-ci bot assigned Miciah Sep 3, 2025

openshift-ci bot assigned alebedev87, candita and Thealisyed Sep 3, 2025

Add more tests on bad keys

b13ee67

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

OCPBUGS-49769: Validate the key and cert content before accepting #669

OCPBUGS-49769: Validate the key and cert content before accepting #669

rikatz commented Sep 1, 2025 •

edited

Loading

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

rikatz commented Sep 1, 2025

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

rikatz commented Sep 1, 2025

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

rikatz commented Sep 2, 2025

Uh oh!

Uh oh!

rikatz commented Sep 2, 2025

Uh oh!

Miciah commented Sep 3, 2025

Uh oh!

Thealisyed commented Sep 3, 2025

Uh oh!

openshift-ci bot commented Sep 4, 2025

Uh oh!

rikatz commented Sep 5, 2025

Uh oh!

openshift-ci bot commented Sep 5, 2025

Uh oh!

Uh oh!

OCPBUGS-49769: Validate the key and cert content before accepting #669

Are you sure you want to change the base?

OCPBUGS-49769: Validate the key and cert content before accepting #669

Conversation

rikatz commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

rikatz commented Sep 1, 2025

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

rikatz commented Sep 1, 2025

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

openshift-ci-robot commented Sep 1, 2025

Uh oh!

rikatz commented Sep 2, 2025

Uh oh!

Uh oh!

rikatz commented Sep 2, 2025

Uh oh!

Miciah commented Sep 3, 2025

Uh oh!

Thealisyed commented Sep 3, 2025

Uh oh!

openshift-ci bot commented Sep 4, 2025

Uh oh!

rikatz commented Sep 5, 2025

Uh oh!

openshift-ci bot commented Sep 5, 2025

Uh oh!

Uh oh!

rikatz commented Sep 1, 2025 •

edited

Loading