Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#18221 closed enhancement (implemented)

Validate our DH parameters to prevent socat-type fails.

Reported by: yawning Owned by:
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: tor-core crypto
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

http://www.openwall.com/lists/oss-security/2016/02/01/4

Was stupid and is easily avoided. While we use hard coded known good primes taken from sensible locations, the code should validate the parameters used as a defense in depth measure.

Child Tickets

Change History (14)

comment:1 Changed 4 years ago by yawning

Status: newneeds_review

comment:2 Changed 4 years ago by nickm

Looks correctly written. I'm not clear what exactly the threat model is here, though. "We replace the primes with something we think is prime, but we forget to check"? "An attacker backdoors our software but doesn't figure out how to remove this check, or can't for some reason"?

comment:3 in reply to:  2 ; Changed 4 years ago by yawning

Replying to nickm:

Looks correctly written. I'm not clear what exactly the threat model is here, though. "We replace the primes with something we think is prime, but we forget to check"? "An attacker backdoors our software but doesn't figure out how to remove this check, or can't for some reason"?

More the former than the latter. We no longer have DynamicDHGroups so this isn't as big of a deal as it used to be (and our current p/g sets pass the checks). My rationale is a combination of:

  • ~10 ms is a trivial amount of startup time to add to prevent foot + gun type issues
  • If we have lots of sanity checks, we can view commits that mess with certain areas of the code with Extreme Suspicion.

comment:4 in reply to:  3 ; Changed 4 years ago by cypherpunks

Replying to yawning:

Replying to nickm:

Looks correctly written. I'm not clear what exactly the threat model is here, though. "We replace the primes with something we think is prime, but we forget to check"? "An attacker backdoors our software but doesn't figure out how to remove this check, or can't for some reason"?

More the former than the latter.

If the threat is the former, why is it necessary to perform the check on every startup? Isn't a build-time unit test sufficient?

comment:5 in reply to:  4 Changed 4 years ago by yawning

Replying to cypherpunks:

If the threat is the former, why is it necessary to perform the check on every startup? Isn't a build-time unit test sufficient?

Was my phrasing overly idiomatic? More still means both...

The test is dirt cheap as long as it won't be done on every TLS connection (and it isn't, just once during initialization). It could be moved to the unit test code, but that involves exposing the currently opaque crypto_dh_t internals, which doesn't feel great since there's zero reason for the internals of the struct to be visible.

comment:6 Changed 4 years ago by bugzilla

It's a simple "check for fools". Not interesting. Can you specify ">=2048 bit" requirement for DH and prevent fallbacks, like Mozilla export-grade epic fail?

comment:7 in reply to:  6 Changed 4 years ago by yawning

Replying to bugzilla:

It's a simple "check for fools". Not interesting. Can you specify ">=2048 bit" requirement for DH and prevent fallbacks, like Mozilla export-grade epic fail?

Duh? It's a trivial check to prevent really silly mistakes, as a defense in depth measure. It's not intended to be interesting.

As to your question, "Not completely, no".

There's 2 places in Tor that currently use non-elliptic curve DH:

  • The old TAP handshake (superceded by ntor, except for the current HSes) which is hardcoded to use 1024 bit DH. Changing this breaks backwards compatibility and will break hidden services. This use case is on the way out due to ntor and the prop 224 work.
  • TLS when one party does not support modern suites (ECDHE is prioritized). This also is less and less likely as time goes on due to ECDHE support in 0.2.7.x and later being required.

I wouldn't object to changing the TLS DH parameters to a 2048 bit group, but that's not all that interesting when the correct solution is "Use P-256".

comment:8 Changed 4 years ago by bugzilla

TAP handshake

It's not ephemeral and has one IIRC "key" for all HSes, or not?

less likely

If an adversary could make a fallback in TLS session, then it'd be seamless for the user.

Use P-256

It's not so good as it seems. 256-bit PK is theoretically strong as 128-bit AES key, but 112-bit can be broken, and the same for 128-bit in the near future. And what's then? Urgently disable P-256 fallback from P-384?

comment:9 in reply to:  8 Changed 4 years ago by yawning

Replying to bugzilla:

If an adversary could make a fallback in TLS session, then it'd be seamless for the user.

That requires breaking TLS, or the relay being malicious. In both cases, you lose regardless of what cipher suite you're using.

Use P-256

It's not so good as it seems. 256-bit PK is theoretically strong as 128-bit AES key, but 112-bit can be broken, and the same for 128-bit in the near future. And what's then? Urgently disable P-256 fallback from P-384?

Sigh.

If anything I'd move to X448 over P-384, but there's not much point when ntor is X25519 based, and relay identities are signed with Ed25519.

Assuming you aren't doing anything clever with batch attacks (which aren't applicable to properly implemented P-256, X25519, or X448), public key cryptography with 112/128 bit security levels require a quantum computer to break.

It's also worth nothing that to get a 128 bit security level with classic DH, you need a group that is at least 3248 bits, which would have catastrophic performance implications.

Anyway, this is orthogonal to the ticket.

comment:10 Changed 4 years ago by bugzilla

Replying to yawning:

That requires breaking TLS

Of course. MITM FREAK attack and others here: https://en.wikipedia.org/wiki/Transport_Layer_Security

you lose regardless of what cipher suite you're using.

If weak suites are disabled, everything will be fine.

require a quantum computer to break.

Using NSA resources for several years adversary can achieve this without it.

Anyway, this is orthogonal to the ticket.

Validate our DH parameters

Key length is the main parameter ;)

comment:11 Changed 4 years ago by cypherpunks

Why do we use hardcoded primes at all? Generating a new prime seems like a better solution, it is not so expensive not to do it.

comment:12 Changed 4 years ago by nickm

Merged. Thanks, Yawning!

Why do we use hardcoded primes at all? Generating a new prime seems like a better solution, it is not so expensive not to do it.

(It's too expensive to generate a safe prime on a smartphone every time we start. "prime" and "safe prime" aren't really the same thing.)

comment:13 Changed 4 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

comment:14 Changed 4 years ago by cypherpunks

It's too expensive to generate a safe prime on a smartphone every time we start.

1 there are PCs. Why not to generate every time at least on PCs?
2 you can generate at least at first start
3 any possibilities of hw accel? any possibilities of GPU acceleration?

Last edited 4 years ago by cypherpunks (previous) (diff)
Note: See TracTickets for help on using tickets.