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

Fix algorithm check to not assume software key instances to work with PKCS#11 #76

Merged
merged 2 commits into from Sep 30, 2019

Conversation

@netmackan
Copy link
Contributor

commented Sep 24, 2019

DNSSEC signing does not currently work with PKCS#11 keys as the checkAlgorithm() method assumes the key instance is a software key and not an unextractable key in an HSM.

When providing a private key instance from SunPKCS11 such as a P11PrivateKey the following exception is thrown:

org.xbill.DNS.DNSSEC$IncompatibleKeyException: incompatible keys
at deployment.signserver.ear//org.xbill.DNS.DNSSEC.checkAlgorithm(DNSSEC.java:1071)
at deployment.signserver.ear//org.xbill.DNS.DNSSEC.sign(DNSSEC.java:1129)

This patch suggests changing from checking the key type using instanceof, as one can not know which instances will be provided and corresponds to which key type, to instead use the Key.getAlgorithm() method. https://docs.oracle.com/javase/9/docs/api/java/security/Key.html#getAlgorithm--

This way signing also works with PKCS#11.

@netmackan

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2019

The build failure is most likely due to that this DNSSEC.java file was not following the styleguide even before this change, right?
Let me know if anything needs to be changed.

@ibauersachs

This comment has been minimized.

Copy link
Member

commented Sep 25, 2019

The build fails because the now unused imports java.security.interfaces.EC/RSA/DSAPrivateKey weren't removed. Do you mind fixing that?

Markus Kilås
@netmackan

This comment has been minimized.

Copy link
Contributor Author

commented Sep 29, 2019

Thanks that was it! Fixed now.

@ibauersachs ibauersachs merged commit 2213c76 into dnsjava:master Sep 30, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 52.266%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.