| Commit message (Collapse) | Author | Age | Files | Lines |
| | |
|
| |
|
|
| |
Typos found with codespell
|
| |
|
|
|
|
| |
In other words kp_relaysign_ed.
Signed-off-by: David Goulet <[email protected]>
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It used to work for an initiator to set the link protocol once a
VERSIONS is received because initiator send their VERSIONS before. This
failed with responders because a responder channel sends their VERSIONS
after receiving one from the initiator.
This reverse logic means that the channel cell handler was transitionned
to the Handshake state before a responder was able to send a VERSIONS
cell leading to a failure because VERSIONS cell aren't allowed at the
Handshake state.
To fix this, the send/recv or recv/send is now explicit per channel type
and once this is done and successful, the link protocol is set. A
`set_link_protocol()` is added to the ChannelBaseHandshake trait so it
can be used to set the cell handler.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
| |
The initiator and responder are quite different. Building an
AUTHENTICATE cell is delicate and so this change differenticates clearly
between the two.
This allows us to remove the peer_cert_digest from an UnverifiedChannel
which is only something that makes sense for an initiator.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
| |
Both client and relay specialized channel now use it as their inner base
channel so they can use the same common verify() function since it is
the same validation for both.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Remove the last part from check_internal() that is specific to an
initiator channel.
At this commit, all three specialized channel do the verify process
within their own verify() function.
The client and relay initiator both look at the TLS cert (code
duplication unfortunately). And the relay responder looks at the
LINK_AUTH cert extracting the peer KP_link_ed key for validation.
The CERTS cell is removed from UnverifiedChannel as it is now only
useful within the verification process which is now specialized.
A series of TODO(relay) is added to point out the current problem and
how to fix them.
The next step is to create an UnverifiedInitiatorChannel that will hold
the verity_tls_cert() function and peer cert information which is only
relevant to an initiator. This will remove code duplication.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
| |
The responder channel will soon use it.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order to pull this off, make
UnverifiedChannel::check_relay_identities() to return a RelayIds that it
builds after checking if they match the peer we were expecting.
This part is moved in this commit so once check_relay_identities()
returns, we are certain of the relay identity validity on both "it
identified properly" and "it is the right expected relay".
This makes it that the check_relay_identities() returns the RelayIds,
the signing key and the RSA id digest (which is needed for
authentication later).
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
| |
This introduces verify_tls_cert() standalone function. It is such
because both client and relay initiator will use it.
For now, the check_internal() has been modified to use it. We are slowly
building towards having specialized check function per channel type.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is a loaded commit, apologize in advance but not many way around
this.
One thing that is generic to all verifiable channel (authenticated) is
that they all need to check the relay identities and signing key from
the CERTS cell.
This commits extracts that part into
UnverifiedChannel::check_relay_identities() which returns those said
identities and the signing key (KP_relaysign_ed).
The signing key is actually needed for only one context, the initiator
part because the TLS cert is signed with it. The LINK AUTH cert is
signed by the ed25519 identity key itself which is what the responder
will look for.
This commit has two side effects which I believe are OK:
1. The timeliness check of the identity certs is now done prior to the
other cert (TLS/LINK).
2. We no longer check signatures in batch mode as we can't batch ed25519
sig check with the RSA crosscert sig. It appears the batch validation
was there for performance and not for security purposes.
The end goal of this piece of work is that the specialized channel will
start by calling a generic check function that will call
check_relay_identities(). And then, the secondary certificates will get
checked depending on the side of the channel.
Expect also a variable rename commit at the end as the naming in this
function is really bad.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
| |
Move two inline functions located in UnverifiedChannel::check_internal()
into the UnverifiedChannel object itself.
Laying down the ground work for the more specialized objects to use
those as the check_internal() is about to get massively refactored into
more specific channel types.
Signed-off-by: David Goulet <[email protected]>
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
| |
The responder always sends an AUTH_CHALLENGE cell.
|
| |
|
|
|
|
|
| |
As far as I know, a responder will always send an AUTH_CHALLENGE cell
since it doesn't yet know if the initiator is a client or relay. The
spec also doesn't have any mention about the AUTH_CHALLENGE being
optional. So we should send it in our tests as well.
|
| |
|
|
|
|
| |
Rename them to respectively sensitive() and not_sensitive().
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
| |
This is used when we build an OwnedChanTarget using the builder. Instead
of going identities by identities at the callsite, we can use this
helper to get us a RelayIds builder and set it in the
OwnedChanTargetBuilder.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
| |
To make the code a bit better here. Also, at this commit, the
UnverifiedChannel::finish() and VerifiedChannel::finish() are basically
the exact same.
A refactoring to use a finish() helper would work nicely.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
| |
Every specific types know if the peer is sensitive or not so now the
finish() of each of these channel types builds the right PeerInfo with
MaybeSensitive.
This is passed on the Channel so from that point on, the Channel will
never leak peer data in the logs.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
| |
Only the R2R channel that the PeerAddr becomes unsensitive. The rest, we
keep it sensitive as it can be a client or a client's guard/bridge.
Signed-off-by: David Goulet <[email protected]>
|
| | |
|
| |\
| |
| |
| |
| | |
relay: Couple fixes related to channel creation
See merge request tpo/core/arti!3726
|
| | | |
|
| |/
|
|
|
|
|
| |
Before this commit, we would use the RsaIdentity which is a SHA1 digest.
We do the same for the peer RSA key.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
```text
Completed handshake with Ed25519Identity { ClByyE4zT+p3dudeCVkUfVA1zp2y2sOUe7u/HHLliWU } [(RsaIdentity { $547bb37f9cd8079eaf1cfcb73fff5887d6f58138 }, [9, 111, 131, 62, 212, 144, 149, 88, 211, 87, 175, 114, 18, 111, 189, 36, 205, 21, 66, 205, 217, 176, 139, 130, 70, 211, 40, 130, 79, 29, 144, 179])]
```
After:
```text
Completed handshake with ed25519:wo9NA5RXkPAc7sB2301JBpvdk1RvSOGZEzgwuWP/msA [$feebaba2b6faf53ae1dc8dda8681ce026537dfa4]
```
|
| |
|
|
|
| |
These were previously in a single `Option`, but now that the `Option`
was removed, I think it's nicer to make these separate fields.
|
| |
|
|
|
|
| |
I don't think that the `Option`s are needed anymore, since
unauthenticated channels no longer transition through the
`VerifiedChannel` state.
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
| |
Responder relay handshake requires the peer address at the very start as
it sends its NETINFO right away.
For initiators, we only need it during the finalization process which is
when the NETINFO is sent and the Channel is created.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
| |
This is a large change but it is basically using PeerAddr in the channel
builder through the channel handshake code and into the Channel itself.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
All handshake pass the NETINFO cell, the advertised addresses (if any)
and the peer address in order to build the Canonicity and build the
channel with it.
In order to pull this off, the "my_addrs" were added to several object
along the NETINFO cell.
We also pass the channel method when connecting (initiator) to a relay
as we need this for this canonicity build.
Signed-off-by: David Goulet <[email protected]>
|
| |\
| |
| |
| |
| |
| |
| | |
maint/add_warning: Deny clippy::unused_async
Closes #2328
See merge request tpo/core/arti!3613
|
| | | |
|
| | |
| |
| |
| |
| | |
This resolves a warning triggered by the newly-introduced
`#[deny(clippy::unused_async)]`.
|
| |/ |
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|