Different TLS certs for incoming vs outgoing

We should learn to present different TLS certs for incoming connections vs outgoing connections.

The motivating example is bridges. They want to show the same identity to people who connect, yet behave like clients when they connect to other relays (e.g. change keys when they change IP addresses).

(Of course, this change would provide a new way to test for bridges: if a Tor connects to you, connect back and see if the cert is different. But at least that's an active test that requires the bridge to connect to you first. But then, the attack I describe above only kicks in when the bridge connects to you. Hm.)

[Automatically added by flyspray2trac: Operating System: All]

changed milestone to %Tor: 0.2.2.x-final

added component::core tor/tor milestone::Tor: 0.2.2.x-final priority::high resolution::fixed status::closed type::enhancement version::0.2.0.34 labels

This shouldn't be hard, but it probably wants a proposal.

See bug988 in git://repo.or.cz/tor/rransom.git .

That branch is built on my bug1994-v3 branch (see #1994 (moved)), so that I could see correct documentation for a few functions before I mutilated them.

See bug988-v2 ( !git://repo.or.cz/tor/rransom.git bug988-v2 ) for another try.

See bug988-v3 ( !git://repo.or.cz/tor/rransom.git bug988-v3 ).

Trac:
Keywords: N/A deleted, N/A added
Parent: N/A to N/A
Status: new to needs_review
Description: We should learn to present different TLS certs for incoming connections vs outgoing connections.

The motivating example is bridges. They want to show the same identity to people who connect, yet behave like clients when they connect to other relays (e.g. change keys when they change IP addresses).

(Of course, this change would provide a new way to test for bridges: if a Tor connects to you, connect back and see if the cert is different. But at least that's an active test that requires the bridge to connect to you first. But then, the attack I describe above only kicks in when the bridge connects to you. Hm.)

[Automatically added by flyspray2trac: Operating System: All]

to

We should learn to present different TLS certs for incoming connections vs outgoing connections.

The motivating example is bridges. They want to show the same identity to people who connect, yet behave like clients when they connect to other relays (e.g. change keys when they change IP addresses).

(Of course, this change would provide a new way to test for bridges: if a Tor connects to you, connect back and see if the cert is different. But at least that's an active test that requires the bridge to connect to you first. But then, the attack I describe above only kicks in when the bridge connects to you. Hm.)

[Automatically added by flyspray2trac: Operating System: All]

See bug988-v4 ( !git://repo.or.cz/tor/rransom.git bug988-v4 ) (rebased on current bug1994-v3; no other changes).

I'm reworking my fixes at the moment; bug988-v4 considered broken.

See bug988-v5 ( git://repo.or.cz/tor/rransom.git bug988-v5 ) for a (hopefully working) set of fixes.

See bug988-v6 ( git://repo.or.cz/tor/rransom.git bug988-v6 ) for a (hopefully working) set of fixes. This time, for sure!

This looks pretty good, modulo a couple of places I'd like to add more documentation. How have you tested this? What more tests are needed?

One thing in particular we need to think about is whether anything here breaks if the Tor process turns on or off server mode while it's running.

Replying to nickm:

This looks pretty good, modulo a couple of places I'd like to add more documentation. How have you tested this? What more tests are needed?

I've been using it as my regular Tor client. I toggled BridgeRelay on without stopping Tor, then off, then on again several days later, and had no unexpected issues. I have since restarted Tor with BridgeRelay on, and it has been running (and receiving and handling bridge users' connections properly) without trouble since then.

I have not yet tested:

1. running this Tor in public server mode
2. toggling to and from public server mode without stopping Tor
3. whether the certs Tor sends on incoming and outgoing connections do, in fact, have different public keys in bridge mode

Turning BridgeRelay off segfaulted it.

Got time to debug that, or want some help? In the latter case, please ask on #tor-dev; I bet somebody will be interested. :)

I can't reproduce any crash. As far as review, I think public_server_mode() is an unfortunate function name. To me it immediately implied that we'd be dealing with the PublishServerDescriptor options, instead of a simple bridge check. I generally wonder if it is necessary to make this distinction at all, because I can't see how it would hurt to maintain separate client and server certs for public relays. Maybe I'm missing something here?

The functions want better documentation wrt accepted parameters and return values, I think. Also a changes file is missing.

Replying to Sebastian:

As far as review, I think public_server_mode() is an unfortunate function name.

If anyone can suggest a less bad term for a non-bridge relay than ‘public server’, I will happily rename public_server_mode. I couldn't come up with one.

To me it immediately implied that we'd be dealing with the PublishServerDescriptor options, instead of a simple bridge check.

That test is done in functions whose names contain publishable_server. ‘Publishable server’ is probably a bad term for that, as the documentation states that a Tor controller could publish descriptors for a Tor server with PublishServerDescriptor off, thus making the Tor server publishable.

I generally wonder if it is necessary to make this distinction at all, because I can't see how it would hurt to maintain separate client and server certs for public relays. Maybe I'm missing something here?

See Tor spec §5.3.1. If an OR X opens a TLS connection to an OR Y, and a client asks Y to extend a circuit to X, that circuit should generally go over the existing TLS connection between X and Y, even though X was the client (and used its client certificates) for that connection.

The functions want better documentation wrt accepted parameters and return values, I think.

Yes. Also, the tor_tls_context_t reference manipulation should be refactored out into utility functions, because I can no longer tell at a glance whether tor_tls_context_t references are referenced and freed properly.

Replying to rransom:

Turning BridgeRelay off segfaulted it.

I reproduced the crash, and nickm diagnosed it as an unrelated bug.

The observed crash is now bug2097. With my bug2097 fix, does switching options on and off in a running Tor now work for you?

Replying to nickm:

The observed crash is now bug2097. With my bug2097 fix, does switching options on and off in a running Tor now work for you?

Toggling BridgeRelay didn't crash Tor most of the times that I tried it, and Sebastian tested it multiple times and couldn't reproduce the crash. bug988-v6 doesn't seem to introduce any crashes.

Merged. We should also maybe consider a backport to 0.2.1.x, so instead of closing this ticket, I'm reassigning it to 0.2.1.x-final.

Trac:
Milestone: post 0.2.1.x to Tor: 0.2.1.x-final

Apparently this one is missing a changes file, even though it's already merged into the upcoming 0.2.2.18? Is there a changes file somewhere, or should somebody write one?

See email; subject line starting with [attn arma]

Trac:
Priority: minor to major

Backported as bug988_021. Needs careful review.

Not sure if we care, the comment in my commit message for the refcounting fix about fixing up a2bb0bf is bogus because this was cherry-picked.

Otherwise the backport seems OK to me.

Replying to nickm:

Backported as bug988_021. Needs careful review.

It looks fine to me. I also grepped for any references to get_identity_key and get_client_identity_key that escaped being replaced with get_tlsclient_identity_key or get_server_identity_key, and didn't find any.

I'm nervous about backporting this one to 0.2.1, since it touches a lot of things and it was tough to get right. Leaving 0.2.1 as-is (and working on making it obsolete) is probably the more robust plan.

So, how do we proceed on this now? nick?

We should decide whether to backport it, or no. I personally think it might be a better idea spending our effort trying to get 0.2.2.x stable. This is enough like a new feature that leaving it out of 0.2.1.x is fine by me.

rransom (or anybody) -- want to argue for a backport?

Without objections, calling this done.

Trac:
Milestone: Tor: 0.2.1.x-final to Tor: 0.2.2.x-final
Status: needs_review to closed
Resolution: None to fixed

For the record:

• This change did break things on 0.2.2.x and later. In particular, it seems to have introduced #2433 (moved), which introduced #2572 (moved), although I can't quite see how that happened now.
• This change was backported to 0.2.1.x anyway in late October 2011, as a prerequisite for the fix for a major anonymity issue.

Trac:
Points: N/A to N/A
Actualpoints: N/A to N/A

Replying to rransom:

• This change did break things on 0.2.2.x and later. In particular, it seems to have introduced #2433 (moved), which introduced #2572 (moved), although I can't quite see how that happened now.

Er, it introduced #2433 (moved), which seems to have introduced #2572 (moved).

Trac:
Component: Tor Relay to Tor

closed

mentioned in issue #2980 (moved)

moved to tpo/core/tor#988 (closed)