tpm2-software / tpm2-totp

Attest the trustworthiness of a device against a human using time-based one-time passwords
https://tpm2-software.github.io
BSD 3-Clause "New" or "Revised" License
159 stars 35 forks source link

Password from stdin #89

Closed EvilBit closed 2 years ago

EvilBit commented 3 years ago

fixes #86

At the moment, this forbids \0 in input stream, but allows \n, etc. Not sure if we want to restrict --password to single-line, printable ASCII only, or arbitrary binary secrets (and what about \0 in that case).

codecov[bot] commented 3 years ago

Codecov Report

Merging #89 (c9cbceb) into master (d4b8937) will increase coverage by 0.08%. The diff coverage is 61.90%.

:exclamation: Current head c9cbceb differs from pull request most recent head 2d29a8a. Consider uploading reports for the commit 2d29a8a to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   81.67%   81.75%   +0.08%     
==========================================
  Files           4        4              
  Lines         802      822      +20     
==========================================
+ Hits          655      672      +17     
- Misses        147      150       +3     
Impacted Files Coverage Δ
src/tpm2-totp.c 76.57% <61.90%> (-0.53%) :arrow_down:
src/libtpm2-totp.c 85.74% <0.00%> (+0.71%) :arrow_up:

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 d4b8937...2d29a8a. Read the comment docs.

EvilBit commented 2 years ago

LGTM! Could you rebase to include 8cf5b58 ("test: should not recover secret with incorrect password") into b2500a6 ("feat: read password from stdin"), please? This will make future bisects easier because you don't have to deal with transient CI failures (in this case from scan-build).

Sure. But I believe you meant c9cbceb fix: close potential memory leak in reading from stdin, not 8cf5b58 test: should not recover secret with incorrect password?

diabonas commented 2 years ago

Sure. But I believe you meant c9cbceb fix: close potential memory leak in reading from stdin, not 8cf5b58 test: should not recover secret with incorrect password?

Oops, you are right, I meant to squash c9cbcebc191ca5701204d0dcfd53f7cffd514b92 into b2500a634a7160d8fc3fd04aa90f85aff9e6263f. Also please rebase onto the current master to resolve the conflict after merging #88 :)

EvilBit commented 2 years ago

Also please rebase onto the current master to resolve the conflict after merging #88 :)

Sure ;) I knew that would eventually be necessary when submitting multiple PRs simultaneously, but I wanted to develop all features independently based on current master, in case one wouldn't be accepted upstream.

If you know of a nicer way to manage this kind of situation, please let me know. :)