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

Extends the API with SecretKey types in addition to raw byte[] #5

Merged
merged 5 commits into from
Jun 13, 2019

Conversation

patrickfav
Copy link
Owner

This is to make the lib compatible to security framework that require
the use of the JCE to work properly (e.g. HSM front ends).

Breaks the hkdfMacFactory interface.

refs #4

This is to make the lib compatible to security framework that require
the use of the JCE to work properly (e.g. HSM front ends).

Breaks the hkdfMacFactory interface.

refs #4
@coveralls
Copy link

coveralls commented Jun 13, 2019

Coverage Status

Coverage decreased (-2.6%) to 97.436% when pulling 2ef777f on feat-4-secret-key into f9be6c4 on master.

* <p>
* See {@link #expand(byte[], byte[], int)} for description.
*
* @param pseudoRandomKey a pseudo random key of at least hmac hash length in bytes (usually, the output from the extract step)
Copy link
Contributor

Choose a reason for hiding this comment

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

But if you have byte[] returned from extract step, you would need to create a SecretKey out of it, right?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Correct - however this does not seem intuitive I believe it makes sense. The expand returns a piece of entropy which always will be a byte array known to the JVM when using this lib (the output of a HMAC basically).

The extract API using the SecretKey type is a special use case (I just wanted to include it to make the API complete), most times the extract will be used with the byte[] API even when using a HSM.

* @return new byte array of output keying material (OKM)
*/
public byte[] extractAndExpand(SecretKey saltExtract, byte[] inputKeyingMaterial, byte[] infoExpand, int outLengthByte) {
return new Expander(macFactory).execute(macFactory.createSecretKey(
Copy link
Contributor

Choose a reason for hiding this comment

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

This answers my above question: yes, you use macFactory to wrap the byte[] into SecretKey

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, but you can still use the old byte[] API

@@ -190,9 +235,9 @@ HkdfMacFactory getMacFactory() {
* if not provided, it is set to a array of hash length of zeros.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the "if not provided" line is about the salt and not the IKM

Copy link
Owner Author

Choose a reason for hiding this comment

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

True, this is a doc bug.


@Override
public SecretKey createSecretKey(byte[] rawKeyMaterial) {
if (rawKeyMaterial == null || rawKeyMaterial.length <= 0) {
Copy link
Contributor

@alisianoi alisianoi Jun 13, 2019

Choose a reason for hiding this comment

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

I think rawKeyMaterial.length == 0 should be sufficient

Edit: also, don't you want an IllegalArgumentException to fail hard and fast? I don't think anyone wants to supply or get a null when going for a secret key.

Edit 2: ah, I see it is applied to the salt, which can be null. Ok, makes sense to me now.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes. Altough I would also prefer a fail fast, null is an option here.

@alisianoi alisianoi mentioned this pull request Jun 13, 2019
@patrickfav patrickfav merged commit 7f5a3ff into master Jun 13, 2019
@patrickfav patrickfav deleted the feat-4-secret-key branch June 13, 2019 17:24
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.

3 participants