webauthn-open-source / fido2-lib

A node.js library for performing FIDO 2.0 / WebAuthn server functionality
https://webauthn.io
MIT License
407 stars 120 forks source link

3.1.6 Update dependencies. Fix algorithm bug. Bump pkijs major 2->3. #90

Closed Hexagon closed 2 years ago

Hexagon commented 2 years ago

Changelog

Comments

After pkijs major update: decodeFidoAaguid is no longer needed, and decodeU2FTransportType could be simplified. cbor-x had to be frozen as 1.3.0 blocks Deno tests for some reason, need to have a look at that later. EdDSA is fully untested and blocked from being used. RSA-PSS is open for use but untested. Yubikey 5 refuses to give me options for these algs.

JamesCullum commented 2 years ago

Thanks for the great work as usual. I understand that you added the text label for some algorithms, but they cannot be used. Whats the advantage to add an algorithm that cannot be used and is untested?

We could just move the cleanup and dependency updates into 3.1.6 and do the new algorithms into 3.2.0 once they are actually finished, wouldn't that make more sense?

Hexagon commented 2 years ago

At least EdDSA and RSA-PSS (PS256) should be usable with the right authenticator. Guess my Yubikey is just too old to test it. The thought was that including the algorithms in source would raise interest, so that someone who actually have the right hardware can enable it (by passing for example -37 to crypoParams, or disable the EdDSA throw), to test it :)

I can split and leave a open pull request for algos. Makes more sense when i think about it 👍

codecov-commenter commented 2 years ago

Codecov Report

Merging #90 (9e9e487) into master (ba4eaf3) will increase coverage by 0.51%. The diff coverage is 97.78%.

:exclamation: Current head 9e9e487 differs from pull request most recent head b8ccdfd. Consider uploading reports for the commit b8ccdfd to get more accurate results

@@            Coverage Diff             @@
##           master      #90      +/-   ##
==========================================
+ Coverage   92.42%   92.93%   +0.51%     
==========================================
  Files          15       15              
  Lines        5584     5818     +234     
==========================================
+ Hits         5161     5407     +246     
+ Misses        423      411      -12     
Impacted Files Coverage Δ
lib/toolbox.js 89.68% <94.11%> (+0.24%) :arrow_up:
lib/keyUtils.js 95.70% <97.89%> (+3.47%) :arrow_up:
lib/attestations/tpm.js 88.73% <100.00%> (ø)
lib/certUtils.js 90.55% <100.00%> (+0.92%) :arrow_up:
lib/main.js 96.66% <100.00%> (ø)
lib/parser.js 88.43% <100.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 2f53ef0...b8ccdfd. Read the comment docs.

JamesCullum commented 2 years ago

I see - while it probably makes sense to raise interest, from my experience in open-source software there is usually more interest in adding a missing feature than fixing an implementation that isn't working. Its also usually a thin line between raising interest and people feeling mislead about what features are actually available.

Thanks for trimming it down - is it ready now? 👍

Hexagon commented 2 years ago

Agree! It feels ready now :)