Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import all certificates from propagated bundles into Keycloak's truststore #560

Merged
merged 1 commit into from
Dec 9, 2020

Conversation

mmorhun
Copy link
Contributor

@mmorhun mmorhun commented Dec 2, 2020

Signed-off-by: Mykola Morhun mmorhun@redhat.com

What does this PR do?

Makes it possible to import several CA certificates from a single file into Keycloak's java trust store.

What issues does this PR fix or reference?

eclipse-che/che#18339

How to test this PR?

  1. Create a *.pem file that contains several root CA certificates
  2. Create a configmap with single data field which contains the *.pem file
  3. Add the configmap as trusted for Che
  4. Check that all CA certs are added into Keycloak's trust store

@mmorhun
Copy link
Contributor Author

mmorhun commented Dec 2, 2020

/test v4-che-operator-update

Copy link
Contributor

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the Che server ? Don't we add certificates in the java truststore in the Che Java deployment as well ? Does it need the same fix ?

// - absolute path to ca-bundle file
// - absolute path to java keystore
// - java keystore password
func getImportCABundleScript() string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you manage the case where there are duplicate certificates (I assume we should be robust and ignore this error) ?
What about other errors when importing a certificate, Do we expect them to fail the start of the server ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you manage the case where there are duplicate certificates (I assume we should be robust and ignore this error) ?

When adding a certificate into trust store we pass unique alias for it. So if user provides identical certificates - both will be added to trust store without any error.

What about other errors when importing a certificate, Do we expect them to fail the start of the server ?

Invalid certificates will not be imported, Keycloak and Che server will be started without invalid certificates

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation.

@mmorhun
Copy link
Contributor Author

mmorhun commented Dec 2, 2020

What about the Che server ? Don't we add certificates in the java truststore in the Che Java deployment as well ? Does it need the same fix ?

@davidfestal already there: eclipse-che/che#18504

@davidfestal
Copy link
Contributor

What about the Che server ? Don't we add certificates in the java truststore in the Che Java deployment as well ? Does it need the same fix ?

@davidfestal already there: eclipse/che#18504

Great ! Thanks !

@mmorhun
Copy link
Contributor Author

mmorhun commented Dec 8, 2020

@davidfestal do you have any other remarks?

… store

Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@eclipse-che eclipse-che deleted a comment from openshift-ci-robot Dec 8, 2020
@eclipse-che eclipse-che deleted a comment from openshift-ci-robot Dec 8, 2020
@mmorhun mmorhun merged commit ed3df35 into master Dec 9, 2020
@mmorhun mmorhun deleted the che-18339 branch December 9, 2020 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants