Teach config.c to work with options configured in other modules

The last part of the groundwork for refactoring config.c will be to teach it about subsystems, so that it can find configuration options that it does not itself own. It needs to find them, parse them, validate them, pass them to the appropriate subsystems.

changed milestone to %Tor: 0.4.3.x-final

Trac:
Parent Ticket: #29211 (moved)
Child Ticket(s): #32406 (moved), #32382 (moved), #32319 (moved)

added actualpoints::4 component::core tor/tor milestone::Tor: 0.4.3.x-final owner::nickm parent::29211 points::2 priority::medium resolution::implemented reviewer::teor severity::normal sponsor::31-can status::closed type::enhancement labels

Mark a number of current 0.4.2.x "defects" as "enhancements."

Trac:
Type: defect to enhancement

Move some Sponsor31 config refactoring tasks into 0.4.3.

Trac:
Milestone: Tor: 0.4.2.x-final to Tor: 0.4.3.x-final

This one isn't ready for serious review yet, but I'd like a design review. It incorporates an improved #32319 (moved). Branch is config_subsys, with PR at https://github.com/torproject/tor/pull/1488.

Once #32339 (moved) and #32344 (moved) are merged, I want to rebase this to simplify the WIP part, and make it actually compile.

Hi, teor! If you can look at this for design, great, but otherwise I'll return to it once #32339 (moved) and #32344 (moved) are in.

Trac:
Reviewer: N/A to teor
Status: assigned to needs_review

I've pushed a new version of the branch; it incorporates an improved #32319 (moved), and is also based on #32339 (moved) and #32344 (moved). There's still more work to do.

I like the design here. I am looking forward to being able to disable the dirauth and relay options in a generic way.

I did a review of the code on GitHub - I had some questions, and found some typos.

I expect that we will have some parseconf tests in this branch, or some more unit tests (or both). I opened #32374 (moved) to make it easier to test options_act*(). It would have been helpful in #32213 (moved), and would also help with testing this ticket.

I resolved a conflict between #32339 (moved) and #32344 (moved) when merging to master, so this branch needs to be rebased.

Can we squash the "Use the confdecl.h macros to declare a crypto_options_t struct" and WIP commits, so that the eventual code is cleaner? Or just delete the draft "Use the confdecl.h macros to declare a crypto_options_t struct" ?

Trac:
Status: needs_review to needs_revision

I've rebased as config_subsys_v2 and started making the requested changes. There's a PR at https://github.com/torproject/tor/pull/1501 so I can keep an eye on coverage and CI.

Trac:
Status: needs_revision to needs_review

This looks great, here's how we want to move forward:

1. I'll do #32397 (moved)
2. nickm will add some tests for crypto NSS using test_parseconf.sh
3. we can merge

Optionally, if we want to do a lot of state refactoring: 4. I'll do #32396 (moved) 5. Someone will add some tests for mainloop state using test_parseconf.sh But this is not a high priority at this time.

(Please remember to add actual points!)

Trac:
Status: needs_review to needs_revision

I've run into a hitch with "nickm will add some tests for crypto NSS using test_parseconf.sh". There is no way to make crypto_openssl_late_init() fail. When it gets a nonexistent engine, it warns, but does not consider it an error. This behavior is probably reasonable, so I don't think we should change it.

I've added the test nonetheless, but I don't think the behavior will change for NSS: it is a success in both cases.

Trac:
Status: needs_revision to needs_review

Okay, I've done some more work here to make it better. I've done #32406 (moved) so that we can mark an accelerator as required, which lets us test failures in options_set().

And wouldn't you know it, there was a bug in options_set()! If options_act() failed while we did not have an event loop initialized, we would crash in tor_shutdown_event_loop_and_exit(). That's #32407 (moved): I've done a separate branch for that against 0.3.5.

I've squashed the fixups on the branch, rebased it on master, and added the #32407 (moved) fix. The new branch is config_subsys_v3. Its PR is https://github.com/torproject/tor/pull/1514.

I'm going to wait for CI to pass here, at least. I think that #32406 (moved) and #32407 (moved) are enough of a change that I should wait for review on them before I merge.

Trac:
Actualpoints: N/A to 4

FWIW, one of the CI tests is failing, but I think that isn't caused by this. It's caused by the fact that we reject DirCache 1 when running with the relay module disabled, but DirCache 1 is the default. I've opened #32410 (moved) for this issue.

I merged #32407 (moved) to 0.4.2 and later, and added a fix to #32410 (moved) for you to review.

I had some questions about the required engines feature, feel free to resolve them however you like, and then merge this ticket. You should probably squash and rebase first, there are some fixups and merge conflicts, and I merged #32407 (moved) to master.

Trac:
Status: needs_review to needs_revision

Okay, I've rebased once more as config_subsys_v4, and taken both of your suggestions on #32406 (moved). New PR is https://github.com/torproject/tor/pull/1516