Opened 23 months ago

Last modified 14 months ago

#27971 needs_revision defect

Still supports 1024 bit keys

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


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 (13)

comment:1 Changed 23 months ago by nickm

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

comment:2 Changed 23 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 23 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 23 months ago by nickm

Keywords: crypto regression? added

comment:5 Changed 22 months ago by nickm

Owner: set to nickm
Status: newassigned

comment:6 Changed 22 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 22 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 22 months ago by teor

Status: assignedneeds_revision

comment:9 Changed 21 months ago by nickm

Keywords: 035-rc-blocker? added

comment:10 Changed 21 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 20 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.

comment:12 Changed 14 months ago by nickm

Owner: nickm deleted
Status: needs_revisionassigned

comment:13 Changed 14 months ago by nickm

Status: assignedneeds_revision

None of these revisions are in my near-term plans.

Note: See TracTickets for help on using tickets.