]> git.ipfire.org Git - thirdparty/FORT-validator.git/commit
Oops!... I Did It Again
authorAlberto Leiva Popper <ydahhrk@gmail.com>
Wed, 29 Dec 2021 00:04:55 +0000 (18:04 -0600)
committerAlberto Leiva Popper <ydahhrk@gmail.com>
Wed, 29 Dec 2021 00:04:55 +0000 (18:04 -0600)
commitf5b1b456b9c8123a10c24be8c5d5f6c02ebe46b4
tree795ae71f4919d84368ca362f70f773818f4bb003
parent34eb43b82e1d2371ce201d4736f5333d31c962cd
Oops!... I Did It Again

Goddamnit. This started as a couple of RRDP bugfixes that sprawled into
another massive refactor thorough.

Lots of stuff was simplified. The RRDP code was pretty filthy; some
functions tried to do too much at once, error codes were being
transformed unnecessarily, argument lists were too long, there were
outdated comments, and some awkward logic suggests clumsy and misguided
bugfixes have taken place.

I know I fixed the following bugs, but the refactor was very aggressive.
I have no hope of this list being exhaustive:

**** Corrupted HTTP download on retry ****

If an HTTP dowload failed mid-transfer, Fort would end up appending the
file to itself on retry, because the file descriptor wasn't reset.

**** HTTP code 304 being treated as an error ****

So the "If-Modified-Since" HTTP header was pure liability.

**** Per-TAL namespaces, not per-RPP namespaces ****

This is the original problem:
https://mailarchive.ietf.org/arch/msg/sidrops/FrAjMFWY5a_cofpOoCEO5Yr_ZLI/

Basically, RRDP files (snapshots and deltas) declare URIs for their
contained files (RPKI objects), and there's nothing intrinsically
stopping a malicious CA's RRDP file from overriding some other CA's RPKI
object. So the RP needs to create per-RPP namespaces.

Existing solution was using per-TAL namespaces. This prevented CAs from
one tree from overriding files from a different tree, but not files on
the same tree.

**** Bad fnstack management ****

Some RRDP error messages were referencing the wrong file, because the
file name was pushed too late to the stack.

**** Was always redownloading all RRDP on startup ****

RRDP session IDs and serials were cached on RAM. On top of that,
RRDP files (including notifications) were always being deleted
immediately after use.

This meant that, on startup, there was absolutely no way of telling
whether an RRDP RPP was outdated or not, so the code had to download
everything all over again.

This was particularly egregious in standalone mode, because that counts
as *always* startup. Ugh.

I purged the session cache, as well as that weird eager delete gimmic.
Fort now queries existing notification files to find out RRDP sessions
IDs and serials.

------------------------------------------------------------------------

New deprecations:

**** `rsync.strategy` ****

The `root` and `root-except-ta` strategies were naive optimizations
because they assumed rsync was the only available transfer protocol.
They just cause redundant transfers now that RRDP not only exists,
but has actually eclipsed rsync.

rsync always works in `strict` mode now, and `rsync.strategy` is
deprecated and ignored.

**** `shuffle-uris` ****

I'm willing to return this one if someone argues in its favor, but I
strongly suspect nobody cares.

Weird, annoying, seemingly pointless feature. And I hope no one's using
it, because it encourages rsync when HTTP is obviously preferred.

RFC 8630:

   In the case where a TAL contains multiple TA URIs, an RP MAY use a
   locally defined preference rule to select the URI to retrieve the
   self-signed RPKI CA certificate that is to be used as a TA.  Some
   examples are:

   o  Using the order provided in the TAL

   o  Selecting the TA URI randomly from the available list

   o  Creating a prioritized list of URIs based on RP-specific
      parameters, such as connection establishment delay

It sounds to me the spec is just throwing out ideas, not mandating the
existence of this flag. We don't implement the third point anyway.

`shuffle-uris` is now deprecated and a no-op. TAL URIs are always
queried top-to-bottom, as this is assumed to be the RIR's preference.

------------------------------------------------------------------------

It's gotten increasingly evident that I'm going to be maintaining this
project indefinitely and alone, so I feel like I need to move to a more
stateless model. I deleted or reimplemented the following tables:

- `reqs_errors`: This was very disproportionate overhead for a very
  small (and kind of optional) feature (failed download logging on the
  operation logs), which is going to be superceded by the Prometheus
  endpoint anyway.
  The Prometheus endpoint implementation has reached new levels of
  urgent.
- `visited_uris`: I think the repository cache should be cleansed by way
  of the "Accessed" cached files property, not from a list of "visited
  URIs."
  I'm still working out the details of this mechanism, but I (admittedly
  recklessly) already deleted the existing code to simplify the
  refactor.
- `db_rrdp_uris`: The validation cycle's temporary URI download status
  cache aspect of this sounds useful, so I extracted it into a new
  module: `rpp_dl_status`.
  On the other hand, the RRDP session ID and serial cache aspect was,
  as previously mentioned, problematic, so I removed it.
- `db_rrdp`: Its whole point was to cache the `db_rrdp_uris` and the
  "RRDP workspace" over different validation cycles. Since neither of
  them exist anymore (the latter was purged as part of the "per-RPP
  namespaces" bugfix), `db_rrdp` naturally followed.

------------------------------------------------------------------------

This is a big commit because most of the problems were intertwined. It's
also an unstable checkpoint; I left a bunch of TODOs, and no testing has
taken place. The rest can ship in proper incremental commits.
89 files changed:
docs/usage.md
src/Makefile.am
src/abbreviations.txt
src/asn1/signed_data.c
src/cert_stack.c
src/cert_stack.h
src/certificate_refs.c
src/certificate_refs.h
src/common.c
src/common.h
src/config.c
src/config.h
src/config/rsync_strategy.c
src/config/rsync_strategy.h
src/config/sync_strategy.c [deleted file]
src/config/sync_strategy.h [deleted file]
src/crypto/hash.c
src/data_structure/path_builder.c [new file with mode: 0644]
src/data_structure/path_builder.h [new file with mode: 0644]
src/delete_dir_daemon.c [deleted file]
src/delete_dir_daemon.h [deleted file]
src/extension.c
src/file.c
src/file.h
src/http/http.c
src/http/http.h
src/internal_pool.c [deleted file]
src/internal_pool.h [deleted file]
src/log.c
src/main.c
src/object/certificate.c
src/object/certificate.h
src/object/ghostbusters.c
src/object/ghostbusters.h
src/object/manifest.c
src/object/manifest.h
src/object/name.c
src/object/roa.h
src/object/tal.c
src/object/tal.h
src/random.c
src/reqs_errors.c [deleted file]
src/reqs_errors.h [deleted file]
src/resource.c
src/rpp/rpp.c [moved from src/rpp.c with 89% similarity]
src/rpp/rpp.h [moved from src/rpp.h with 82% similarity]
src/rpp/rpp_dl_status.c [new file with mode: 0644]
src/rpp/rpp_dl_status.h [new file with mode: 0644]
src/rrdp/db/db_rrdp.c [deleted file]
src/rrdp/db/db_rrdp.h [deleted file]
src/rrdp/db/db_rrdp_uris.c [deleted file]
src/rrdp/db/db_rrdp_uris.h [deleted file]
src/rrdp/delta.c [new file with mode: 0644]
src/rrdp/delta.h [new file with mode: 0644]
src/rrdp/notification.c [new file with mode: 0644]
src/rrdp/notification.h [new file with mode: 0644]
src/rrdp/rrdp.c [new file with mode: 0644]
src/rrdp/rrdp.h [new file with mode: 0644]
src/rrdp/rrdp_loader.c [deleted file]
src/rrdp/rrdp_loader.h [deleted file]
src/rrdp/rrdp_objects.c [deleted file]
src/rrdp/rrdp_objects.h [deleted file]
src/rrdp/rrdp_parser.c [deleted file]
src/rrdp/rrdp_parser.h [deleted file]
src/rrdp/snapshot.c [new file with mode: 0644]
src/rrdp/snapshot.h [new file with mode: 0644]
src/rrdp/types.c [new file with mode: 0644]
src/rrdp/types.h [new file with mode: 0644]
src/rsync/rsync.c
src/rsync/rsync.h
src/state.c
src/state.h
src/str_token.c
src/thread_var.c
src/thread_var.h
src/types/uri.c
src/types/uri.h
src/types/uri_list.c [new file with mode: 0644]
src/types/uri_list.h [new file with mode: 0644]
src/validation_handler.c
src/validation_handler.h
src/visited_uris.c [deleted file]
src/visited_uris.h [deleted file]
src/xml/relax_ng.c
src/xml/relax_ng.h
test/impersonator.c
test/rrdp_objects_test.c
test/rsync_test.c
test/tal_test.c