Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tor-hs: use named sections #21642

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

tor-hs: use named sections #21642

wants to merge 12 commits into from

Conversation

stokito
Copy link
Contributor

@stokito stokito commented Jul 23, 2023

Maintainer: @ja-pa
Compile tested: no
Run tested: TurrisOS 6.3.3

Description: minor changes to improve usability, see commit messages.

@stokito stokito changed the title Tor hs tor-hs minor changes Jul 23, 2023
@stokito
Copy link
Contributor Author

stokito commented Jul 23, 2023

I also wanted to discuss how the tor config can be simplified. Here are current default configs:

config tor 'conf'
	option default '/etc/tor/torrc'
	option generated '/tmp/torrc'
	list tail_include '/etc/tor/torrc_generated'

The /tmp/torrc looks like

## This file was automatically generated please do not edit here !
%include /etc/tor/torrc
%include /etc/tor/torrc_generated

And the /etc/config/tor-hs

config tor-hs common
	option GenConf "/etc/tor/torrc_generated"

It looks for me like too many of moving parts. Why would I need to edit the tor.conf.default option?
Essentially this all can be replaced with a few includes directly in the torrc file. By default it already has a line
%include /etc/torrc.d/*.conf but it's commented out.
Maybe just add a patch to uncomment the line directly in the torrc and this will simplify a lot of things and many options and creation of a temp file won't be needed.

I created a topic Default folder for onion services where proposed to the Tor team to do that on their side. Also to create a standard location for hidden services.

The tor-hs itself has a problem that the generated file is stored to /etc/tor/torrc_generated which may kill NAND flash eventually. Instead we may generate the file into /var/run/tor older which is an in-memory temp folder.
The generation itself may be skipped if last modify time of uci config file is before the creted time of the /etc/tor/torrc_generated. The simple optimization but may safe some time on restart.

stokito added 2 commits July 23, 2023 23:33
Using substring instead of awk.
It changes behaviour when only one port is specified.
Previously:
value="80" => public="80" local=""
Now:
value="80" => public="80" local="80"

It simplifies configuration of one-to-one ports.

Signed-off-by: Sergey Ponomarev <[email protected]>
@BKPepe
Copy link
Member

BKPepe commented Jul 24, 2023

I am not sure if @ja-pa continues using his Turris router together with tor, but to be safe about this. Let's also ping guys from @turris-cz, especially packagers (@paper42 , @miska), to look into it further because they could be interested in it. They do have Tor listed in reForis under package management:
image

@stokito
Copy link
Contributor Author

stokito commented Jul 24, 2023

I tested on my Turris but I installed the tor from opkg. Now I enabled the Tor in packages but looks like nothing changed. How to check what exactly was installed?

Currently the tor package itself doesn't have any UCI options. From what I understand the tor by default creates a socks proxy on localhost:9050 but this can be changed to SOCKSPort 192.168.1.1:9050 so that anyone from a local network can use it. I created a proxy PAC file so that all .onion domains are resolved with the local tor. Maybe add the SOCKSPort as UCI option?
Basically other settings are for bridge or exit node and looks like not needed.

@stokito
Copy link
Contributor Author

stokito commented Aug 4, 2023

Also it may be better to introduce a new option Target or Destination instead of the Ipv4 because it can be also IPv6 or even a domain. The IPv4 name is highly confusing.

stokito added 3 commits August 4, 2023 17:20
The Name is used as a HS folder name and can't be empty.

Signed-off-by: Sergey Ponomarev <[email protected]>
Replace boolean "true"/"false" with more frequently used 1/0.
This may avoid configuration mistakes which is critical for Tor.
The Luci app anyway will set it as 1/0.

Make sections named. This is not required but again safes from mistakes when executing uci command.

Uncomment sections but disable them by default.
Then in a Luci app a user can quickly figure out what to change.
Ideally a user may just enable the config and start using it.

In the nextcloud config use a single 80 instead of 80;80.
This simpler configuration is now supported.

Instead of "Hidden service" the Tor team now uses "Onion service".

Signed-off-by: Sergey Ponomarev <[email protected]>
@stokito
Copy link
Contributor Author

stokito commented Aug 4, 2023

I added 83e19fb to prevent error when Name is empty

@BKPepe
Copy link
Member

BKPepe commented Aug 18, 2023

Let's ping anyone from @turris-cz to look into it.

@stokito
Copy link
Contributor Author

stokito commented Aug 18, 2023

the changes are really minimal and can be reviewed just by eyes. The only questionable change is 83e19fb where onion services without a required Name are skipped.
I think it would be better to add a uci validation instead. But if the validation failed then as far I remember the whole service won't start. This may broke existing configuration if just one hidden service is broken.
Honestly I don't want to waste time on this because anyway this highly unlikely to happen. If a user will use a luci-app-tor then the Name will be always set anyway.

@stokito
Copy link
Contributor Author

stokito commented Feb 4, 2024

@ja-pa @BKPepe please merge

@BKPepe
Copy link
Member

BKPepe commented Feb 4, 2024

No one reviewed it yet and it seems like Turris guys are not interested in this anymore. I am not going to merge this unless I will or anyone else will review this.

@stokito stokito force-pushed the tor-hs branch 2 times, most recently from 01472f3 to 3a243fd Compare February 5, 2024 07:41
@stokito stokito changed the title tor-hs minor changes tor-hs: use named sections Feb 5, 2024
Remove unused description.
Quote variables.
Use hostname_file variable.
Remove unnecessary quotes around "common".
Use echo -n to truncate a TORRC_FILE.

Signed-off-by: Sergey Ponomarev <[email protected]>
@stokito
Copy link
Contributor Author

stokito commented Feb 6, 2024

I made additional cleanup and added validation.

Signed-off-by: Sergey Ponomarev <[email protected]>
Previously the chown/chmod was performed each time even if the folder already existed.

Signed-off-by: Sergey Ponomarev <[email protected]>
On each tor-hs service restart it generates a config file /etc/tor/torrc_generated.
The /etc/ is stored on a disk and kills it and slow.

Instead create a dedicated tor service Runtime Dir in the temp /var/run/.
It will be accessible only to the tor user.

Signed-off-by: Sergey Ponomarev <[email protected]>
Mark the /etc/tor folder to keep during sysupgrade.
The folder contains hidden_services folder with keys.

Signed-off-by: Sergey Ponomarev <[email protected]>
Add me as a second maintainer.
Remove outdated README.md but put a link to a Wiki instead.
Use Onion Service instead of Hidden Service.

Signed-off-by: Sergey Ponomarev <[email protected]>
@stangri
Copy link
Member

stangri commented Feb 8, 2024

LGTM. Not sure if it's still required to squash commits, otherwise GTG.

@BKPepe
Copy link
Member

BKPepe commented Feb 8, 2024 via email

@stokito
Copy link
Contributor Author

stokito commented Feb 8, 2024

In the 21bd809 I added myself as a second maintainer

PKG_MAINTAINER:=Jan Pavlinec [email protected], Sergey Ponomarev [email protected]

@BKPepe
Copy link
Member

BKPepe commented Feb 8, 2024 via email

@stokito
Copy link
Contributor Author

stokito commented Feb 8, 2024

I don't see any problem to have two maintainers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants