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

asadmin to import plain PKCS#8 RSA keypairs into the java keystore #2599

Merged
merged 3 commits into from
Apr 15, 2018
Merged

asadmin to import plain PKCS#8 RSA keypairs into the java keystore #2599

merged 3 commits into from
Apr 15, 2018

Conversation

ratcashdev
Copy link
Contributor

@ratcashdev ratcashdev commented Apr 4, 2018

ASADMIN command to import unencrypted PKCS8 RSA keypairs into JAVA keystores using the api directly (not via keytool). Such keys and certificates are produced by LETSEncrypt.

Don't allow empty or short key and/or keystore passwords.
Added convenience methods to enforce glassfish convention of keys using the same passwords as the keystore.

A couple of considerations:

  1. Please have a look on the API in KeystoreManager. In many places it expects the keyStoreType, but as of https://bugs.openjdk.java.net/browse/JDK-8062552 it could be omitted and presume "JKS" as it works both for PKCS12 and JKS keystores - there's a relevant test for it.
  2. Currently oly RSA keys are supported/tested
  3. Only non-encrypted, plain key files are supported - but this is what LetsEncrypt delivers.
  4. There are methods that may violate the glassfish convention of same passwords for the key and keystore, but those are clearly documented.

…ing the JAVA api directly (not via keytool).

Don't allow empty or short key and/or keystore passwords.
Methods to enfore glassfish convention of keys using the same passwords as the keystore.
@smillidge smillidge added the PR: CLA CLA submitted on PR by the contributor label Apr 6, 2018
@smillidge
Copy link
Contributor

thanks I will try and review soon

Copy link
Contributor

@smillidge smillidge left a comment

Choose a reason for hiding this comment

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

Looks pretty good just a javadoc needed

import java.security.cert.CertificateFactory;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.PKCS8EncodedKeySpec;
import java.util.Base64;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a JDK8 only class. This is OK for Payara 5 branch but not for Payara 4

Copy link
Contributor Author

@ratcashdev ratcashdev Apr 7, 2018

Choose a reason for hiding this comment

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

Good point. So should we make this backportable to Payara 4 too? Which class would you suggest to use for decoding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW: com.sun.enterprise.v3.admin.AdminAdapter is using this class too, so I thought it's ok.

return Base64.getDecoder().decode(base64KeyData);
}

public Collection<? extends Certificate> readPemCertificateChain ( File pemFile ) throws KeyStoreException {
Copy link
Contributor

Choose a reason for hiding this comment

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

javadoc needed

@smillidge
Copy link
Contributor

jenkins test please

@payara-ci
Copy link
Contributor

Quick build and test passed!

@ratcashdev
Copy link
Contributor Author

jenkins test please

@lprimak lprimak changed the title ASADMIN to import plain PKCS#8 RSA keypairs into the java keystore asadmin to import plain PKCS#8 RSA keypairs into the java keystore Apr 11, 2018
@smillidge
Copy link
Contributor

jenkins test please

@payara-ci
Copy link
Contributor

Quick build and test passed!

@lprimak lprimak merged commit b3bd6b0 into payara:master Apr 15, 2018
@ratcashdev ratcashdev deleted the feature/asadmin-add-pkcs8 branch April 16, 2018 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: CLA CLA submitted on PR by the contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants