| Commit message (Collapse) | Author | Age | Files | Lines |
| |\
| |
| |
| |
| | |
tor-netdoc: testing: Expose some more test utilities
See merge request tpo/core/arti!4250
|
| | |
| |
| |
| | |
Involves code motion. Review with --color-moved.
|
| | |
| |
| |
| | |
I don't understand why this suddenly, but whatever.
|
| | |
| |
| |
| | |
Involves lots of code motion. Review with --color-moved.
|
| | |
| |
| |
| | |
This whole module is gated by the very same condition.
|
| | |
| |
| |
| | |
I discovered this didn't work, when I tried to use it.
|
| | |
| |
| |
| | |
Better do it before the release so we can think about it a bit more.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We need to obtain the maximum of the lower bound and the minimum of the
upper bound instead of vice versa.
Another example on why we should replace this with a better
implementation.
Likewise, `expiry` in the legacy verification code is also obtained like
that.
|
| | |
| |
| |
| |
| | |
This makes it more clear, besides it will sound more "correct" with the
next commit applied.
|
| | | |
|
| | | |
|
| | | |
|
| | |
| |
| |
| | |
Otherwise clippy complains.
|
| | |
| |
| |
| |
| | |
This commit adds a comprehensive test for router descriptor verification
that tests various valid and invalid edge cases.
|
| | |
| |
| |
| |
| |
| |
| | |
This is a bad encode_sign() method for RouterDesc that is testing only
and will be used soon to implement testing for invalid router
descriptors, for which we may need to create invalid ones in the first
place.
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This commit implements verification for router descriptors. 🎉
For this, the following checks are performed:
* RouterDesc::identity_ed25519 is validly signed.
* RouterDesc::master_key_ed25519 is as implied by identity_ed25519.
* RouterDesc::fingerprint is as implied by RouterDesc::signing_key.
* RouterDesc::ntor_onion_key_crosscert is validly signed.
* RouterDesc::signing_key has correct length and exponent.
* All RouterDesc::family_cert elements are valid.
* The inner and outer RouterDescSignatures are valid.
Unfortunately, we now have two implementations for that, as the legacy
parse_internal() also implements its own verification logic for this.
It seems merging these two together however would probably cause more
harm than good, as the legacy verification is closely intertwined with
legacy parsing, making a commonly shared verification logic hard to
achieve. In other words: parse2 parses the descriptor in its entirety
first, followed by verification afterwards, whereas the legacy code
parses and verifies every field before advancing towards the next.
Instead, I suggest to read through RouterDesc::parse_internal() and
ensure that every verification related check present there is also
present here. The notable exception to this is everything TAP related,
which is absent on purpose here.
Right now, this code is untested. I will add unit tests shortly
afterwards.
|
| |/
|
|
| |
The underlying type also implements Copy. We will need it later.
|
| |
|
|
|
|
|
| |
`onion_key_crosscert` was already absent from `RouterDesc`, even
though its item `onion-key-crosscert` was processed by the old parser.
Delete it all.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
See prop350
https://spec.torproject.org/proposals/350-remove-tap.html
This is part of "Phase 3, Item 2: Remove vestigial TAP code in Arti".
Technically we are not at phase 3 yet, because we haven't yet
sunsetted C Tor 0.4.8 and made the dirauth changes in Phase 2.
However, this field is not used in Arti right now. RouterDescs are
used by client code for handling bridges (but we never use TAP keys),
and the RouterDesc type will be used for generation and mirroring by
by Arti Relay/Dirauth.
In prop350 we have decided that we won't be deploying Arti Relay until
this as been done.
|
| |\
| |
| |
| |
| | |
Fix some recently-appearing clippy lints
See merge request tpo/core/arti!4240
|
| | |
| |
| |
| | |
Placates recent clippy.
|
| |\ \
| | |
| | |
| | |
| | | |
Tidy some uses of Intern
See merge request tpo/core/arti!4233
|
| | | |
| | |
| | |
| | | |
We intern these in Microdesc, and should be consistent.
|
| | | | |
|
| | |/
| |
| |
| | |
Rather than converting it to an Arc. This is the new idiom for Intern.
|
| |\ \
| |/
|/|
| |
| | |
tor-netdoc testdata-live: More shell script, less macrology
See merge request tpo/core/arti!4238
|
| | |
| |
| |
| |
| | |
This gits rid of the macrology, apart from the generated macro module
file.
|
| |\ \
| | |
| | |
| | |
| | | |
tor-netdoc: Add a few affordances
See merge request tpo/core/arti!4235
|
| | | |
| | |
| | |
| | |
| | |
| | | |
As per
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4235#note_3439437
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4235#note_3439438
|
| | | |
| | |
| | |
| | |
| | | |
As per
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4235#note_3439436
|
| | | | |
|
| | | | |
|
| | | |
| | |
| | |
| | |
| | | |
I keep finding I want to encode things and then I have to prat about
with a NetdocEncoder. Let's provide potted versions.
|
| |\ \ \
| |/ /
|/| /
| |/
| | |
tor-netdoc testdata-live: Export for the benefit of other crates
See merge request tpo/core/arti!4229
|
| | |
| |
| |
| |
| |
| |
| |
| | |
Add a new testdata_live module which is exposed with the testing
features, containing the testdata-live in string constants.
This avoids the need for test cases in other crates to walk the
filesystem to an area outside their own crate path.
|
| | |
| |
| |
| |
| | |
As per
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4223#note_3438348
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This function returns a `TimeRangeBound`. That implies a
responsibility on the caller to check the time. It doesn't make sense
for this function to do the check as well.
But, it turns out that in tor-hsclient, the `TimeRangeBound<HsDesc>`
is sometimes processed with `.dangerously` on the assumption that it
was checked earlier. I considered changing this, and storing plain
`HsDesc` and a separate `TimeRange` - but that's not right, because
there are places where the `TimeRangeBound<HsDesc>` is used well after
it was verified.
Instead, in this commit, I (effectively) move the `.check_valid_at`
call from `parse_decrypt_validate` to its principal call site.
This involves a change to the error representation. Previously,
validity time errors ended up as `DescriptorErrorDetail::Descriptor`
containing an `HsDescError::OuterValidation` HsDescError::
InnerValidation`, which in turn contains a
`tor_netdoc::Error`. (`tor_netdoc::Error` is a rather awkward type.)
Now we have our own error variant. The overall behaviour is
unchanged.
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Replace open-coding of various is_valid_at and various dangerously and
intersect. In more detail:
* Do most of the processing inside `TimeRangeBound::build_intersect`
* Replace uses of dangerously_peek etc. with `TimeBound::unwrap_with`
* The timebound machinery now takes care of doing the intersection
* Remove the individual `.is_valid_at` calls and replace them with
one at the end, on the intersection. This preserves the current
behaviour except that sometimes time validity errors will now be
reported as having occurred the wrong level. We'll deal with this
in a moment (by deleting these checks from here entirely).
* There is no need to handle a `None` from `intersect` any more.
TimeBound handles conflicting time ranges differently: it
allows ranges which are empty due to being ill-formed.
|
| | |
| |
| |
| |
| | |
This variable had a different name inside the block, to outside. This
was confusing, and, fixing it makes the next commit clearer.
|
| | | |
|
| | |
| |
| |
| |
| | |
I find this names confusing. To my mind "is" implies a function
returning `bool`.
|
| | |
| |
| |
| |
| |
| |
| | |
I find these names confusing. To my mind "check" implies a function
returning `Result<(), _>`.
Some other APIs use `unwrap` here but I think `if` is good.
|
| | |
| |
| |
| |
| | |
It is better to return a more cooked type. `TimeRange` aka
`TimeRangeBound<()>` is perfect for this.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| | |
It was confusing that one of these functions had "which bound"
mentioned in its name, but the other didn't. So add `end` and switch
from `tolerance` to `bound` (see previous commit message).
*This* commit should deal only in `extend_tolerance` and `end` and
shouldn't touch `extend_start_bound` or `extend_pre_tolerance`.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Although it is often used to apply a tolerance, it doesn't make sense
to say that this is extending the "tolerance" of a `TimeRangeBound`.
A `TimeRangeBound` doesn't have a tolerance, only bounds.
Also we should be consistent in our terminology, and use `start`
rather than `pre`.
We'll rename the other method too. Doing them one at a time will
makes it easier to spot any "pre/start" vs "<nothing>/end" slips:
*this* commit should deal only in `pre` and `start` and shouldn't
touch `extend_tolerance`.
|
| |/
|
|
|
|
|
|
|
|
| |
This just returns a tuple.
We're going to introduce a new method that returns a `TimeRagne` and
will want to be called `bounds`.
That method will want to be in the `TimeBound` trait, but for now we
add it here. Various call sites will be added in forthcoming commits.
|
| | |
|