-
Notifications
You must be signed in to change notification settings - Fork 28
Add decryption of AES-encrypted private keys #26
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit message title is misleading. It sounds like you're adding support for decrypting AES (symmetric) keys. This is about decryption of AES-encrypted private keys (of an asymmetric key pair). Please rephrase it.
| public class TpmPrivateKey { | ||
| static { | ||
| Security.insertProviderAt(new org.spongycastle.jce.provider.BouncyCastleProvider(), 1); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a newline after this
|
|
||
| if (key == null) | ||
| return new Blob(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't add this newline
| byte[] encodingBytes = new byte[10]; | ||
| encodingBytes = new byte[encoding.remaining()]; | ||
| encoding.get(encodingBytes, 0, encodingBytes.length); | ||
| encoding.clear(); | ||
| encodingBytes = new byte[encoding.capacity()]; | ||
| encoding.get(encodingBytes, 0, encodingBytes.length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not following this code, why do you need to do all this? encodingBytes is allocated 3 times?
| * This replaces any existing private key in this object. | ||
| * @param encoding The byte buffer with the private key encoding. | ||
| * @param password The password for decrypting the private key, which should | ||
| * have characters in the range of 1 to 127. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does this limitation come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's leftover from the code I replaced. I'll remove it.
| } catch (IOException | OperatorCreationException | PKCSException ex) { | ||
| throw new TpmPrivateKey.Error | ||
| ("loadEncryptedPkcs8: Error parsing PrivateKey info: " + ex); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indentation
| try { | ||
| SafeBag safebag = new SafeBag(testKey); | ||
| fixture_.keyChain_.importSafeBag(safebag, password.buf()); | ||
| } catch (Throwable ex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indentation
|
You should squash the two commits together and force-push to the branch in order to properly update the PR. |
8427626 to
acf5d1b
Compare
Adds support for AES encrypted private keys exported by ndn-cxx.