Opened 5 weeks ago

Closed 4 weeks ago

#21290 closed defect (implemented)

Rename and/or redocument --enable-expensive-hardening

Reported by: nickm Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


The hardening from "--enable-expensive-hardening" is not only expensive, it is also fragile.

The compiler options that "--enable-exensive-hardening" uses (asan, ubsan, trapv) work by converting different classes of C bugs into program crashes. If those bugs are remotely triggerable, we replace them with denial-of-service bugs.

Some possible underlying bugs here are actually harmless -- like the integer bug in TROVE-2017-001, or the read-one-extra-byte bug of TROVE-2016-12-002. So long as any bugs like these bugs exist, "expensive-hardening" will make your Tor more vulnerable to remote denial of service.

But some possible underlying bugs are potential trouble -- like if we had an actual stack overflow bug or a heap overflow bug. "expensive-hardening" can replace some of these with aborts too. So long as any bugs like these bugs exist, "expensive-hardening" makes it a little more difficult to do RCE or heartbleed-style leaks against your Tor.

The first kind of bug seems much more common in practice over Tor's history. But the impact of the second kind of bug would be significantly worse.

So using "--enable-expensive-hardening" in production means "Make me much more vulnerable to remote DoS, but (maybe? probably?) less vulnerable to RCE or heartbleed."

We should document this, and maybe rename the option, so that it's clear that CPU cycles aren't the only issue.

Child Tickets

Change History (6)

comment:1 Changed 5 weeks ago by cypherpunks

This is also a good time to reintroduce

comment:2 Changed 4 weeks ago by nickm


comment:3 Changed 4 weeks ago by nickm

(The old option should still _work_, though. Either one should produce a message.)

comment:4 Changed 4 weeks ago by dgoulet

  • Owner set to dgoulet
  • Status changed from new to accepted

comment:5 Changed 4 weeks ago by dgoulet

  • Status changed from accepted to needs_review

See branch bug21290_030_01.

comment:6 Changed 4 weeks ago by nickm

  • Resolution set to implemented
  • Status changed from needs_review to closed


Note: See TracTickets for help on using tickets.