Opened 7 months ago

Last modified 4 months ago

#27971 needs_revision defect

Still supports 1024 bit keys

Reported by: kroeckx Owned by: nickm
Priority: High Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: crypto regression?
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The code still contains a 1024 bit DH key. If you still want to support DH, can I suggest you switch to a key from rfc7919?

As far as I understand, since 0.2.4, ECDHE is prefered, and DHE shouldn't be used anymore. The 0.2.4 branch itself doesn't seem to be supported anymore.

#27344 changed things so that 1024 bit DH keys will always be allowed, and only seems to be added to support very old hosts that are known to have several security issues.

Child Tickets

Change History (11)

comment:1 Changed 7 months ago by nickm

Component: - Select a componentCore Tor/Tor
Milestone: Tor: 0.3.5.x-final
Priority: MediumHigh

comment:2 Changed 7 months ago by nickm

We need to allow 1024-bit RSA and DH at the crypto level, or else version 2 hidden services would stop working. Does blocking 1024-bit DH only affect TLS, or would it affect all DH usage in Tor?

comment:3 Changed 7 months ago by kroeckx

The SSL_CTX_set_security_level() only affects TLS. Directly calling crypto function does not enforce any limits.

comment:4 Changed 7 months ago by nickm

Keywords: crypto regression? added

comment:5 Changed 7 months ago by nickm

Owner: set to nickm
Status: newassigned

comment:6 Changed 7 months ago by nickm

Okay, I've investigated this a bit more.

Changing TLS_DH_PRIME to a 2048-bit prime is easy enough; after doing so, the 0.3.5 unit tests almost pass at security level 2, and "make test-network" passes.

In 0.2.9, increasing the TLS prime length to 2048 is also okay. However, in 0.2.9, raising the security level to 2 makes a few dozen unit tests fail.

Here's what I'd propose:

  • Raise the TLS prime size to the 2048-bit named prime from RFC7919 in 0.2.9 and forward, fixing unit tests as needed.
  • In versions before 0.3.4 or 0.3.5, move the SSL_CTX_set_security_level(1) call to only happen in the unit tests.
  • In 0.3.5, remove the SSL_CTX_set_security_level() call entirely, and fix the one failing unit test.

(I only tested this with OpenSSL 1.1.0i -- there might well be compatibility issues with other versions for us to sort out.)

comment:7 Changed 7 months ago by nickm

I've made a start with the ideas above, in bug27971_029, bug27971_034, and bug27971_035. But for some reason chutney isn't passing, so more work is needed.

comment:8 Changed 6 months ago by teor

Status: assignedneeds_revision

comment:9 Changed 5 months ago by nickm

Keywords: 035-rc-blocker? added

comment:10 Changed 5 months ago by nickm

I'm not convinced we need to do this in particular before 0.3.5 is out, but we definitely need a solid answer here before we ship.

comment:11 Changed 4 months ago by nickm

Keywords: 035-rc-blocker? removed
Milestone: Tor: 0.3.5.x-finalTor: unspecified

I think the best we can do here for now is to maintain the current checks in our code, and make sure we aren't doing anything to prevent ourselves from eventually throwing away the old onion keys.

I'd love it if anybody can help out trying to make chutney and the other tests pass with the tests above, but I don't think it's necessarily going to be easy.

Note: See TracTickets for help on using tickets.