improve verify_config_tor() parser and safe_edit() file mangement

Other files managed by onionjuggler include ClientOnionAuthDir and HiddenServiceDir/authorized_clients.

The auth option does not have the safe edit safeguard yet.
What happens is that to fully verify tor config, the files needs to be present inside the dirs with correct names, and remove then if it fails to verify.

Issue: #48
I inserted a wrong configuration file to ClientOnionAuthrDir missing the onion address at the beginning:

:descriptor:x25519:<priv-key-base32>

The tor parser does not inform this is the problem:

[err] tor_assertion_failed_(): Bug: ../src/app/config/config.c:920: get_options_mutable: Assertion global_options failed; aborting. (on Tor 0.4.7.8 )
[err] Bug: Tor 0.4.7.8: Assertion global_options failed in get_options_mutable at ../src/app/config/config.c:920: . Stack trace: (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(log_backtrace_impl+0x57) [0x5afccea3cf07] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(tor_assertion_failed_+0x148) [0x5afccea47f88] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(escaped_safe_str+0xa2) [0x5afcceaccc92] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(hs_parse_address+0x5c) [0x5afcceb46b0c] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(hs_config_client_authorization+0x120) [0x5afcceb44310] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(hs_config_client_auth_all+0x19) [0x5afcceb49899] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(+0x17cc1a) [0x5afccead4c1a] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(+0xca7ef) [0x5afccea227ef] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(config_validate+0x115) [0x5afccea246c5] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(+0x17a0f2) [0x5afccead20f2] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(options_init_from_string+0x133) [0x5afccead2383] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(options_init_from_torrc+0x47c) [0x5afccead2a9c] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(tor_init+0x217) [0x5afcce9bf3f7] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(tor_run_main+0x91) [0x5afcce9bfd31] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(tor_main+0x49) [0x5afcce9bc2d9] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(main+0x19) [0x5afcce9bbeb9] (on Tor 0.4.7.8 )
[err] Bug:     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xea) [0x7f82404a9d0a] (on Tor 0.4.7.8 )
[err] Bug:     /usr/bin/tor(_start+0x2a) [0x5afcce9bbf0a] (on Tor 0.4.7.8 )

This is why it is important to first validate as much as possible the config before saving to be parsed by tor, and when possible, remove the file that was just inserted.

The --verify-config option did not caught the problem because it is using -f /usr/loca/etc/torrc.d/40_onionjuggler.conf, not /etc/tor/torrc.

This should be corrected on another issue, but anyway, auth option to remove file if tor fails to parse is still missing.

Originally posted by @nyxnor in #48 (comment)

verify_config_tor() has config="${tor_conf_tmp:-"${tor_conf}"}".
safe_edit() has file_name_tmp="$(mktemp "${file}.XXXXXX")".

To verify tor_conf_tmp, it should be named .conf by the mktemp, otherwise it will fail to get parsed on %include *.conf globs.

Something like file_name_tmp="$(mktemp "${file}.XXXXXX.conf")".

Also, the other problem is that while the torrc is being edited, it will have two dublicate files: 40_onionjuggler.conf and 40_onionjuggler.conf.x27423x.conf. It is a matter of parsing the temp file but not the original one. ~~Which I didn't find a way to do this yet, unless of course the original file is temporary moved to another location, but I find this undesired for now...~~

Edit: man torrc: Files starting with a dot are ignored.. So maybe rename the original file and put a dot on front? Seems reasonable.

Anyway, this wouldn't solve the problem because the auth file also needs to be added to be removed.

What needs to be done is something in the lines of vidoas set_trap_rm()

https://github.com/slicer69/doas/blob/master/vidoas#L70

remove files on script exit hup int term

trap posix

Ok, testing hiding the file with dot in front and verifying all files normally, including the temporary file, this seems to be the best solution so far.

it is perfect for changes made to tor_conf, but nothing made in progress to Client Authorization files.

It can be the same methodology, just need to implement it.

There is a problem with safe_edit and authorization files.

first, they are never edited, they are either, inserted, removed, or copied to
inserted can cause problems if user tries to use an invalid key, but only if the script doesn't catch it
remove will never have a problem
copied to is when user tries to import either priv or pub key and the script doesn't catch a misconfiguration
key pair generated by the script will never have a problem because it is conducted without relevant user input

What needs to be done is thoroughly check the configuration the user inserted manually, and if I could go further, is implement safe_edit to work with auth files that are different than tor_conf in the following matters:

they either don't exist yet as a file
they are not yet on the parsed dir as they need to be imported to it

Major improvements on configuration parsing and file name parsing made on commits linked to issue #48

This of course is mitigation, not as robust as removing the file if tor fails.

I hardcoded .conf recently because safe_edit was just being used for tor_conf

onionjuggler/usr/share/onionjuggler/defaults.sh

Line 203 in d586df2

file_name_tmp="$(mktemp "${file}.XXXXXX.conf")"

But this has problems:

not all torrcs need to end with .conf to be parsed
/etc/tor/torrc is the default file and some systems just use this one, so this name should be maintained with the temp file, not using XXXXXX
auth files can be .auth or .auth_private.

Also the XXXXXX needs to be at the end to parse the files in the right order according to the prefix, in lexical order.

Solution:

detect file ending
if file is /etc/tor/torrc or /usr/local/etc/tor/torrc or $HOME/.torrc
- if file name is torrc, maintain original file name for temp file can move the original to be hidden with a dot in front of the name
- if file name is .torrc, name the original file with a different name and the temp file as .torrc, do not put a dot in front as it already has one

About detecting file ending:

.auth and .auth_private are not possible to change, and that is good.
.conf is just a standard, could be anything that %include wants it to be (.cfg, .cnf, .torrc etc)

file="/usr/local/etc/torrc.d/40_onionjuggler.conf"
file_name="${file##*/}" ## 40_onionjuggler.conf
file_name_suffix="${file_name##*.}" ## .conf
file_name_prefix="${file_name%.*}" ## 40_onionjuggler

it turns out that a file can be named ..torrc-orig, with two dos preceding it, it is just a temp file anyway.

for torrc's, it is finished, now for auth files.

Current situtation:

File location
- When a new authorization is imported, the file being imported does not live yet in a tor's data directory, so the mktemp won't create it there.
- Also means that if the tor parsing fails, there wouldn't be an original file to remain (be restored) on the tor data dir

Tor ignores invalid auths:

[warn] Client authorization encoded base32 private key can't be decoded: 00UO7UPV34ICNMS3PUVHKIANGTCIKYHCCZCTA4537CFEVORP5VKQ
Configuration OK

onionjuggler-cli-auth-* checks detect this, I purposefully bypassed it to test what could happen otherwise.

https://gitlab.torproject.org/search?search=Client+authorization+encoded+base32&nav_source=navbar&project_id=426&group_id=478&search_code=true&repository_ref=main

log_warn and then goto err does not error the config, goto end does.

So safe_edit can't save for auth files because tor --verify-config does not save the day.