Skip to content

Include standalone HKDF implementation for v1.local tokens#6

Merged
bdemers merged 3 commits into
paseto-toolkit:masterfrom
zbiljic:feature/extensions-crypto-hkdf
Apr 27, 2020
Merged

Include standalone HKDF implementation for v1.local tokens#6
bdemers merged 3 commits into
paseto-toolkit:masterfrom
zbiljic:feature/extensions-crypto-hkdf

Conversation

@zbiljic
Copy link
Copy Markdown
Contributor

@zbiljic zbiljic commented Apr 25, 2020

I have created new V1LocalCryptoProvider class that replaces Bouncy Castle dependency for HKDF project (https://github.com/patrickfav/hkdf).

For anyone needing only 'v1.local' tokens this should result in MUCH smaller dependency (~11 KB for HKDF vs. ~4.3 MB for Bouncy Castle).
My idea is that this can be used when size of end JAR may be constraint, such is for Android or AWS Lambda functions.

Please note update to README.md file, that is only part I question whether I made acceptable change. Main change should be simple.

As far as testing goes, I have copied V1LocalIT and changed one method, where the encryption is now done via HKDF provider and decryption using Bouncy Castle provider. I felt this was enough, as other test cases would require changes to core library (or some other creative approach) as the library depends on using first implementation for provider found using ServiceLoader.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2020

Codecov Report

Merging #6 into master will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #6      +/-   ##
============================================
+ Coverage     82.82%   82.94%   +0.11%     
- Complexity      246      250       +4     
============================================
  Files            52       54       +2     
  Lines           722      727       +5     
  Branches         53       53              
============================================
+ Hits            598      603       +5     
  Misses           94       94              
  Partials         30       30              
Impacted Files Coverage Δ Complexity Δ
...ouncycastle/BouncyCastleV1LocalCryptoProvider.java 100.00% <100.00%> (ø) 2.00 <1.00> (-6.00)
...jpaseto/crypto/hkdf/HKDFV1LocalCryptoProvider.java 100.00% <100.00%> (ø) 2.00 <2.00> (?)
...jpaseto/impl/crypto/BaseV1LocalCryptoProvider.java 100.00% <100.00%> (ø) 8.00 <8.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec5f808...2b8a74e. Read the comment docs.

@bdemers
Copy link
Copy Markdown
Contributor

bdemers commented Apr 25, 2020

Nice! I’ll take a look!

Now that we have more than one option for V1 Local tokens HKDF and BC, a `BaseV1LocalCryptoProvider` has been added.
This reduces what is needed to implement a V1LocalCryptoProvider to a single method.
@bdemers
Copy link
Copy Markdown
Contributor

bdemers commented Apr 25, 2020

@zbiljic, I hadn't seen this lib before, very nice.

I sent a few tweaks to your branch: https://github.com/zbiljic/jpaseto/pull/1
Mostly adding a base class that both HKDF and BC impls can extend.
(and some housekeeping for the version and readme, that was mostly my fault, to begin with 😄)

If you are happy with that merge it into, your branch, and then I'll pull it in, and cut a release.

After that, I'll try to add a table to the readme and list out the advantages of each implementation (and the spec versions they support), BC, HKDF, libsodium.

Thoughts?

@zbiljic
Copy link
Copy Markdown
Contributor Author

zbiljic commented Apr 25, 2020

Yeah, your changes look great.

(I did not want to make some core edits to someones project.)

@bdemers bdemers merged commit d028cd6 into paseto-toolkit:master Apr 27, 2020
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.

2 participants