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

Fix LGTM warning about overflow checks in SP800-108 KDF #2412

Merged
merged 1 commit into from
Oct 2, 2020

Conversation

randombit
Copy link
Owner

@randombit randombit commented Oct 2, 2020

It turns out some KDFs do verify that the output length is not
too large, making the KDF truncation bug (#2347) even nastier.

Add comments in KDFs where we are truncating so they can be fixed
later. Also fix SP800-56C which would return the original key length
rather than the possibly truncated key the KDF generated.

It turns out some KDFs *do* verify that the output length is not
too large, making the KDF truncation bug (#2437) even nastier.

Add comments in KDFs where we are truncating so they can be fixed
later. Also fix SP800-56C which would return the original key length
rather than the possibly truncated key the KDF generated.
@lgtm-com
Copy link

lgtm-com bot commented Oct 2, 2020

This pull request fixes 3 alerts when merging c67a874 into b7e2ccf - view on LGTM.com

fixed alerts:

  • 3 for Comparison result is always the same

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2020

Codecov Report

Merging #2412 into master will decrease coverage by 0.01%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2412      +/-   ##
==========================================
- Coverage   92.60%   92.59%   -0.02%     
==========================================
  Files         557      557              
  Lines       61215    61220       +5     
  Branches     6432     6433       +1     
==========================================
- Hits        56688    56686       -2     
- Misses       4527     4534       +7     
Impacted Files Coverage Δ
src/lib/kdf/hkdf/hkdf.cpp 100.00% <ø> (ø)
src/lib/kdf/kdf1/kdf1.cpp 100.00% <ø> (ø)
src/lib/kdf/kdf1_iso18033/kdf1_iso18033.cpp 100.00% <ø> (ø)
src/lib/kdf/kdf2/kdf2.cpp 100.00% <ø> (ø)
src/lib/kdf/prf_x942/prf_x942.cpp 100.00% <ø> (ø)
src/lib/kdf/sp800_108/sp800_108.cpp 96.77% <80.00%> (+0.22%) ⬆️
src/lib/kdf/sp800_56c/sp800_56c.cpp 100.00% <100.00%> (ø)
src/lib/asn1/der_enc.cpp 80.12% <0.00%> (-2.49%) ⬇️
src/lib/misc/cryptobox/cryptobox.cpp 93.65% <0.00%> (-1.59%) ⬇️
src/lib/tls/msg_client_hello.cpp 91.62% <0.00%> (-0.56%) ⬇️
... and 1 more

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 b7e2ccf...c67a874. Read the comment docs.

@randombit randombit merged commit cc6aa8a into master Oct 2, 2020
@randombit randombit deleted the jack/kdf-truncate branch October 2, 2020 15:18
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