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

New option [libdefaults] socks4a_proxy. #1204

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

Conversation

riastradh
Copy link

@riastradh riastradh commented Jan 2, 2024

This is deliberately kept simple -- and is not generalized to SOCKS4 or SOCKS5 or other types of proxies -- so it is easy to audit for network and DNS leaks. (SOCKS4 works in IP addresses, and so invites DNS leaks. SOCKS5 can be OK, if used judiciously, but takes more work to implement.)

XXX Need to combine with #1155 to plug DNS leaks.

XXX Need to figure out where the socks4a.c code should go.

fix #1151

@riastradh riastradh force-pushed the riastradh-20240101-issue1151-socks branch from ec3bfbd to 078fa78 Compare January 3, 2024 01:14
@riastradh riastradh changed the title WIP: New option [libdefaults] socks4a_proxy. New option [libdefaults] socks4a_proxy. Jan 3, 2024
@riastradh riastradh force-pushed the riastradh-20240101-issue1151-socks branch from 078fa78 to d2f7729 Compare January 3, 2024 02:08
@nicowilliams
Copy link
Contributor

image

@riastradh riastradh force-pushed the riastradh-20240101-issue1151-socks branch from d2f7729 to fa1294a Compare January 4, 2024 13:39
@nicowilliams
Copy link
Contributor

Ahh, you need to also modify lib/krb5/NTMakefile to build socks4a.obj and link it.

You should probably name the _socks4s_* symbols _heim_socks4a_* or _krb5_socks4a_* just in case. Even though you don't export them they could still cause conflicts in apps linking statically with static Heimdal builds. (I hate static linking for this reason. It's a long rant that I'm going to elide here.)

@riastradh riastradh force-pushed the riastradh-20240101-issue1151-socks branch 3 times, most recently from 0f3d224 to c4cc19c Compare January 7, 2024 22:11
@riastradh
Copy link
Author

riastradh commented Jan 7, 2024

I updated the branch with changes that I think are correct and necessary for lib/krb5/NTMakefile, but the Windows build is still failing and I can't figure out how to read an understanding of the problem out of the log. Hints welcome!

Couple notes on the design:

Configuration syntax

To keep the implementation simple and commit early, commit often, I called the configuration parameter socks4a_proxy and made the syntax the same as other host configuration parameters. Only host[:port] is actually accepted, though, not, e.g., http://host or udp/host.

In the future, there might be other proxies, like SOCKS5 or HTTP CONNECT proxies -- I'm not implementing them now because it's not needed for Tor, but I can imagine someone wanting these. If we added socks5_proxy and http_connect_proxy, these might not combine well; it might be better to have something like proxy = socks4a:127.0.0.1:9050 or proxy = http:127.0.0.1:8080, so it's clear there's no question about how to combine multiple different types of proxy specification in the config since there's only one. But that's a little more work to implement.

If you'd rather see proxy = socks4a:127.0.0.1:9050 than socks4a_proxy = 127.0.0.1:9050, I can draft that -- just thought I'd get the bulk of it working first for an initial review of the main point. Let me know if you have opinions!

SOCKS4a user id

I implemented automatically passing the principal as the SOCKS4a user id. This isn't for authentication -- this is for Tor circuit isolation, so that stream connections which don't use the SOCKS4a user id don't use the same circuit and thus won't appear to be related. That way, your anonymous web browsing through Tor doesn't get correlated with your krb5 logins, and Tor gives you location privacy from the KDC when you do use krb5 logins. Kerberized applications that talk through the SOCKS4a proxy have to do the same, of course, to get the same isolation.

(ssh SOCKS4a proxies also accept a user id but ignore it.)

If there are users who might want to use SOCKS4a with non-Tor (and non-ssh) proxies, where the user id has some other significance, we might need some other configuration knob to manage that.

@nicowilliams
Copy link
Contributor

I updated the branch with changes that I think are correct and necessary for lib/krb5/NTMakefile, but the Windows build is still failing and I can't figure out how to read an understanding of the problem out of the log. Hints welcome!

You have click on the Windows check's details

image

and view the logs,

image

and search for "error" because the Windows build emits lots of noise after the fatal error, so you won't find the error of interest near the bottom of the logs (sad), but the way the GitHub UI works the whole logs won't be in the page, so you can't search using the normal browser search feature (ctrl-f), instead you either have to use the GitHub logs search feature

image

or you have to download the logs and grep / whatever for "error",

image

which will find:

.\socks4a.c(37): fatal error C1083: Cannot open include file: 'unistd.h': No such file or directory

image

I haven't done a local Windows build in ages, so I always deal with those in this way, meaning it's a lot of effort to debug build issues because of the need to iterate through GitHub Actions runs. To speed that up I make sure only the Windows checks run, which greatly speeds up the process. I either disable the checks using a suitable branch name, or delete workflows other than the Windows one, or just manually stop all actions other than the Windows workflow's action(s).

@riastradh riastradh force-pushed the riastradh-20240101-issue1151-socks branch from c4cc19c to 05e3fbc Compare January 10, 2024 02:57
Taylor R Campbell added 5 commits January 10, 2024 02:58
All network traffic to KDC goes through the SOCKS4a proxy if it is
configured.

This is deliberately kept simple -- and is not generalized to SOCKS4
or SOCKS5 or other types of proxies -- so it is easy to audit for
network and DNS leaks.  (SOCKS4 works in IP addresses, and so invites
DNS leaks.  SOCKS5 can be OK, if used judiciously, but takes more
work to implement.)

This only affects krb5_sendto -- the other initiator of network
traffic in libkrb5, krb5_change_password, will be fixed to respect
socks4a_proxy in a subsequent commit.

XXX Need to figure out where the socks4a.c code should go.

fix heimdal#1151
Default of UDP transport doesn't work over SOCKS4a anyway, so this
makes configuration with socks4a_proxy easier.
This enables Tor stream isolation.
@riastradh riastradh force-pushed the riastradh-20240101-issue1151-socks branch from 05e3fbc to ab546a1 Compare January 10, 2024 03:00
@riastradh riastradh marked this pull request as ready for review January 10, 2024 23:11
@riastradh
Copy link
Author

The code seems to work and passes all github actions checks, but see some notes in #1204 (comment) about ways you might want it to be changed before merging.

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.

SOCKS and/or HTTP CONNECT proxy
2 participants