[WFCORE-3305][WFCORE-3425] Add more runtime operations to the Elytron key-store resource for advanced KeyStore manipulation #2949
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Full integration - Windows Build 4601 outcome was FAILURE using a merge of 0f9dd63 Failed tests |
|
Added a few more tests for the import-certificate operation as discussed with Martin Choma. |
fjuma
force-pushed
the
WFCORE-3305
branch
2 times, most recently
from
a412a42
to
6b3b7a8
Compare
|
retest this please |
bstansberry
reviewed
Jan 24, 2018
| certChain[0] = certAndKey.getSelfSignedCertificate(); | ||
| keyStore.setKeyEntry(alias, privateKey, keyPassword, certChain); | ||
| } catch (KeyStoreException | IllegalArgumentException | DateTimeException e) { | ||
| throw new OperationFailedException(e); |
There was a problem hiding this comment.
Do all of these indicate user errors? If any don't, throw a runtime exception.
bstansberry
reviewed
Jan 24, 2018
| if (e instanceof OperationFailedException) { | ||
| throw (OperationFailedException) e; | ||
| } else { | ||
| throw new OperationFailedException(e); |
There was a problem hiding this comment.
Don't throw OFE unless it's a user mistake. If not use a runtime exception.
bstansberry
reviewed
Jan 24, 2018
| if (e instanceof OperationFailedException) { | ||
| throw (OperationFailedException) e; | ||
| } else { | ||
| throw new OperationFailedException(e); |
bstansberry
reviewed
Jan 24, 2018
| if (e instanceof OperationFailedException) { | ||
| throw (OperationFailedException) e; | ||
| } else { | ||
| throw new OperationFailedException(e); |
bstansberry
reviewed
Jan 24, 2018
| if (e instanceof OperationFailedException) { | ||
| throw (OperationFailedException) e; | ||
| } else { | ||
| throw new OperationFailedException(e); |
bstansberry
reviewed
Jan 24, 2018
| try { | ||
| return keyStoreService.resolveKeyPassword(credentialSourceSupplier); | ||
| } catch (Exception e) { | ||
| throw new OperationFailedException(e); |
|
This should get an approval by someone more familiar with elytron and with the requirements. It also needs to be out of the analysis phase so we know those requirements are set. :) Seems ok to me other than my notes about exception handling. If there's additional external testing of this going on, please comment noting that and comment when that's done. |
|
I've updated the exception handling. The two test failures currently occurring on Windows ( |
|
Just a note that Martin Choma has moved this issue out of the analysis phase now so the requirements are set. |
|
retest this please |
1 similar comment
|
retest this please |
|
Core - Full Integration Build 6593 outcome was FAILURE using a merge of 29f83bd Failed tests |
|
I have marked EAP7-650 as Test Development done. There are no blockers on this. I am OK with getting this into WF12. |
darranl
reviewed
Feb 12, 2018
| @@ -271,6 +266,21 @@ boolean isModified() { | |||
| return trackingKeyStore.isModified(); | |||
| } | |||
|
|
|||
| char[] resolveKeyPassword(final ExceptionSupplier<CredentialSource, Exception> keyPasswordCredentialSourceSupplier) throws Exception { | |||
There was a problem hiding this comment.
Not related to this PR but we also want at some point to look at KeyStore password re-use for the KeyManager resource as well.
|
Do we have any information about the new binary files so if anyone is debugging or needs to recreate them they know what they need? |
…source for advanced KeyStore manipulation The new operations are as follows: * generate-key-pair * generate-certificate-signing-request * import-certificate * export-certificate * change-alias
…t doesn't exist and wasn't flagged as required when attempting to store the KeyStore to the file
|
@darranl I've added a readme file with information about the new binary files. |
darranl
approved these changes
Feb 12, 2018
|
Thanks @fjuma looks good. |
|
Core - Full Integration Build 6675 outcome was FAILURE using a merge of 1b4ef35 Failed tests |
|
retest this please |
|
@fjuma I resolved the two WFCORE's mentioned in the title but I'll leave any EAP7 etc manipulation to you. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds more runtime-only management operations to the Elytron subsystem key-store resource for advanced KeyStore manipulation. The new operations are as follows:
generate-key-pair: This will generate a key pair and wrap the resulting public key in a self-signed X.509 certificate. The generated private key and self-signed certificate will be added to the KeyStore.
generate-certificate-signing-request: This will generate a PKCS Subsystem test framework tweaks #10 certificate signing request using a PrivateKeyEntry from the KeyStore. The generated certificate signing request will be output to a file.
import-certificate: This will import a certificate or certificate chain from a file into an entry in the KeyStore.
export-certificate: This will export a certificate from an entry in the KeyStore to a file.
change-alias: This will move an existing KeyStore entry to a new alias.
After executing the generate-key-pair, import-certificate, or change-alias operations, executing the store operation will persist the resulting changes to the file that backs the KeyStore.
This PR also adds a commit to fix WFCORE-3425. This ensures that for a file-backed key-store, if the file doesn’t exist and wasn’t flagged as required, then the file will get created properly when executing the store operation.
https://issues.jboss.org/browse/WFCORE-3305
https://issues.jboss.org/browse/WFCORE-3425
Documentation PR: wildfly/wildfly#10830