#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:

Description

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 10 months ago by cypherpunks

This is also a good time to reintroduce http://www.openwall.com/lists/oss-security/2016/02/17/9.

comment:2 Changed 10 months ago by nickm

--enable-fragile-hardening

comment:3 Changed 10 months ago by nickm

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

comment:4 Changed 10 months ago by dgoulet

Owner: set to dgoulet
Status: newaccepted

comment:5 Changed 10 months ago by dgoulet

Status: acceptedneeds_review

See branch bug21290_030_01.

comment:6 Changed 10 months ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged!

Note: See TracTickets for help on using tickets.