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

Bug33545 043 #1842

Open
wants to merge 3 commits into
base: maint-0.4.3
Choose a base branch
from
Open

Bug33545 043 #1842

wants to merge 3 commits into from

Conversation

asn-d6
Copy link
Member

@asn-d6 asn-d6 commented Mar 30, 2020

No description provided.

The client auth protocol allows attacker-controlled x25519 private keys being
passed around, which allows an attacker to potentially trigger the all-zeroes
assert for client_auth_sk in hs_descriptor.c:decrypt_descriptor_cookie().

We fixed that by making sure that an all-zeroes client auth key will not be
used.

There are no guidelines for validating x25519 private keys, and the assert was
there as a sanity check for code flow issues (we don't want to enter that
function with an unitialized key if client auth is being used). To avoid such
crashes in the future, we also changed the assert to a BUG-and-err.
And also disallow all-zeroes keys from the filesystem; add a test for it too.
@coveralls
Copy link

coveralls commented Mar 30, 2020

Pull Request Test Coverage Report for Build 8728

  • 11 of 11 (100.0%) changed or added relevant lines in 3 files are covered.
  • 519 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.0001%) to 63.599%

Files with Coverage Reduction New Missed Lines %
src/app/main/main.c 1 21.85%
src/lib/err/torerr.c 10 82.93%
src/lib/log/log.c 87 73.8%
src/feature/hs/hs_descriptor.c 103 88.88%
src/lib/sandbox/sandbox.c 318 1.41%
Totals Coverage Status
Change from base Build 8565: 0.0001%
Covered Lines: 50216
Relevant Lines: 78957

💛 - Coveralls

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