Security and Privacy #59
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aarani
force-pushed
the
miscSec
branch
2 times, most recently
from
9658bae to
933fb55
Compare
aarani
changed the title
aarani
force-pushed
the
miscSec
branch
4 times, most recently
from
2e15f49 to
048742c
Compare
aarani
changed the title
aarani
force-pushed
the
miscSec
branch
5 times, most recently
from
9f764fc to
b0edcfc
Compare
|
Since this is a rabbit hole we need to flush these every few commits |
aarani
force-pushed
the
miscSec
branch
6 times, most recently
from
2aab074 to
7c45b97
Compare
knocte
reviewed
Apr 25, 2023
knocte
reviewed
Apr 25, 2023
knocte
reviewed
Apr 25, 2023
knocte
reviewed
Apr 25, 2023
aarani
force-pushed
the
miscSec
branch
3 times, most recently
from
fdc84a1 to
9b4d817
Compare
knocte
reviewed
Apr 27, 2023
Apparently, clients don't have to report their IP addresses.
According to spec: Initiators SHOULD use "this OR's address" to make sure that they have connected to another OR at its canonical address.
According to spec: Clients SHOULD send "0" as their timestamp, to avoid fingerprinting.
According to spec:
```
To authenticate the responder as having a given RSA identity only,
the initiator MUST check the following:
* The CERTS cell contains exactly one CertType 1 "Link" certificate.
* The CERTS cell contains exactly one CertType 2 "ID" certificate.
* Both certificates have validAfter and validUntil dates that
are not expired.
* The certified key in the Link certificate matches the
link key that was used to negotiate the TLS connection.
* The certified key in the ID certificate is a 1024-bit RSA key.
* The certified key in the ID certificate was used to sign both
certificates.
* The link certificate is correctly signed with the key in the
ID certificate
* The ID certificate is correctly self-signed.
In both cases above, checking these conditions is sufficient to
authenticate that the initiator is talking to the Tor node with the
expected identity, as certified in the ID certificate(s).
```
Open
|
typo in one of the commit msgs: validatation |
Why was it broken? |
knocte
reviewed
May 1, 2023
knocte
reviewed
May 1, 2023
knocte
reviewed
May 1, 2023
knocte
reviewed
May 1, 2023
The previous implementation did not support the optional digest-algorithm property in the directory-signature object which is present in micro-consensus.
Fixed |
Added to the commit msg |
Making sure consensus data is signed by majority of trusted authorities is probably the most important security check in TOR which was missing from NOnion, this commit fixes that. This commit also fixes an issue with parsing directory signatures, adds digest calculation to NetworkStatus and changes networkstatus.json to use Indented formating to help with manual validation.
This commit moves the auth_dirs.inc file to EmbeddedResource so end users don't have to carry the list around with their applications.
This commit removes janky pem reader code in favour of Bouncycastle's PemReader.
knocte
approved these changes
May 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal for this PR is to implement as much security checks we can.