| Commit message (Collapse) | Author | Age | Files | Lines |
| |\
| |
| |
| |
| | |
Initialize extra-info module
See merge request tpo/core/arti!4284
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This commit initializes the doc::extra_info module in tor-netdoc, which
is still marked as incomplete and will be extended in the future.
Right now, it is very barebones by only supporting the introduction item
as well as the bare minimum required for verification.
It also adds a simple unit test, which iterates over the available test
data, parses and verifies it.
|
| | |
| |
| |
| | |
Needed for tor-dirserver testing.
|
| |\ \
| | |
| | |
| | |
| | | |
tor-netdoc: Fix left-over incompletes
See merge request tpo/core/arti!4273
|
| | | |
| | |
| | |
| | |
| | | |
This removes two left over incomplete feature flags.
Follow-up to c20df8c5f879806f7044c6e26d9e69404c88cfa3.
|
| |\ \ \
| |_|/
|/| |
| | |
| | | |
tor-netdoc: Remove outdated top-level RouterDesc comment
See merge request tpo/core/arti!4274
|
| | |/
| |
| |
| |
| |
| |
| |
| | |
This feature is no longer present and we now always compile with
router descriptors enabled, deferring the optimization to the
Rust compiler and linker respectively.
Follow-up to arti!3972
|
| |\ \
| | |
| | |
| | |
| | | |
Make RouterDesc item docs consistent
See merge request tpo/core/arti!4279
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Right now, the documentation for the items is inconsistent in the sense
that some items contain specification links whereas others contain
copies of the specification.
This has historic reasons during the development, but ultimately links
to the specification are superior, as they provide up-to-date
information and are the canonical source.
|
| | |
| |
| |
| | |
No functional change, purely formatting.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This commit moves the remainder of misc::routerdesc into a new module
called types::descriptor, which contains the shared types needed in
router descriptors as well as extra-info documents.
Please review with --color-moved --color-moved-ws=allow-indentation-change,
as this is mainly code motion, except for parts where inputs and type
aliases had to be fixed.
|
| |/
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit moves all non-signature related parts from
types::misc::routerdesc into doc::routerdesc, because they are only
required for that specific netdoc.
The signature related parts will be moved in the next commits, as we
will need those for extra info documents as well.
Review with --color-moved --color-moved-ws=allow-indentation-change.
This is mostly code motion with small import/type name fixes being the
notable exception.
|
| | |
|
| |
|
|
|
| |
No functional change, please review with
--minimal --color-moved --color-moved-ws=allow-indentation-change.
|
| |
|
|
|
|
|
|
|
|
|
| |
This commit utilizes TimeRangeBoundBuilder for the verification of
router descriptors by replacing all calls of
`.dangerously_unwrap_timely()` with `.unwrap_with()`, while also
wrapping the actual logic inside a closure which is then called by
`TimeRangeBound::build_intersect()` in the end.
Keep in mind, that the replacement also leads to the removal of the
use of tuples, as `.unwrap_with()` only returns the actual item.
|
| |\
| |
| |
| |
| | |
tor-netdoc: testing: Expose some more test utilities
See merge request tpo/core/arti!4250
|
| | |
| |
| |
| | |
Involves code motion. Review with --color-moved.
|
| | |
| |
| |
| | |
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.
|
| |
|
|
|
|
|
| |
`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.
|
| | |
| |
| |
| | |
We intern these in Microdesc, and should be consistent.
|
| | | |
|
| |/
|
|
| |
Rather than converting it to an Arc. This is the new idiom for Intern.
|
| | |
|
| | |
|
| |
|
|
|
| |
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.
|