Opened 2 years ago

Last modified 17 months ago

#22963 new enhancement

Make relay integrity digests harder to guess by padding cells with random bytes

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: security, 034-triage-20180328, 034-removed-20180328
Cc: Actual Points:
Parent ID: #22948 Points:
Reviewer: Sponsor:

Description

The tor spec says we should put random bytes in padding cells:
https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1534

But we don't currently do this (see #22948).
And we don't put random bytes in other cells.

This makes it easier to guess the circuit integrity digest, which makes some DoS and malleability attacks easier.

Should we pad all cells with random bytes?

Child Tickets

Change History (10)

comment:1 Changed 2 years ago by teor

Parent ID: #22948

comment:2 Changed 2 years ago by teor

We could random pad the unused bytes in RELAY_DATA cells, because we're unlikely to extend them.

But any other types are really hard to random-pad, because we want that space to add future fields in, and zero is a useful default.

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

Replying to teor:

we want that space to add future fields in

No, that's not a concern. It's easy to transmit whatever future stuff may be wanted by using structure in the 'randomness', i.e. the way relays currently recognize cells that are for them. And remember that the spec says it should be random, so other implementations will have made it random.

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

Replying to cypherpunks:

Replying to teor:

we want that space to add future fields in

No, that's not a concern. It's easy to transmit whatever future stuff may be wanted by using structure in the 'randomness', i.e. the way relays currently recognize cells that are for them.

This makes it hard to do what we did when we added IPv6 Exits, which was to add a field with bits:
0: Use IPv4
0: Don't use IPv6
0: Prefer IPv6

This worked because the field was zero in the old version of the cell.

If it were random, then old clients would get a random selection of these options.
And at least one option combination is non-functional on most sites (110) and several are either nonsensical or non-functional (1x0, x01, 00x).

And remember that the spec says it should be random, so other implementations will have made it random.

The spec says that padding cells should be filled with random bytes (but tor doesn't do this, see #22948). But it says fixed-length non-padding cells should be filled with zeroes after their payload. This ticket is about changing the non-padding cell case.

comment:5 in reply to:  4 Changed 2 years ago by cypherpunks

Replying to teor:

Replying to cypherpunks:

Replying to teor:

we want that space to add future fields in

No, that's not a concern. It's easy to transmit whatever future stuff may be wanted by using structure in the 'randomness', i.e. the way relays currently recognize cells that are for them.

This makes it hard to do what we did when we added IPv6 Exits, which was to add a field with bits:
0: Use IPv4
0: Don't use IPv6
0: Prefer IPv6

This worked because the field was zero in the old version of the cell.

If it were random, then old clients would get a random selection of these options.
And at least one option combination is non-functional on most sites (110) and several are either nonsensical or non-functional (1x0, x01, 00x).

Random data can be replaced with encrypted authenticated data, which can be recognized as non-random by implementations that support it, keeping compatibility with implementations that do use actual random data. Relays currently recognize cells that are for them in this way.

And remember that the spec says it should be random, so other implementations will have made it random.

The spec says that padding cells should be filled with random bytes (but tor doesn't do this, see #22948). But it says fixed-length non-padding cells should be filled with zeroes after their payload. This ticket is about changing the non-padding cell case.

Cells may be full, in which case user data will be there.

comment:6 Changed 2 years ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final

comment:7 Changed 19 months ago by nickm

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final
Type: defectenhancement

Label a bunch of (arguable and definite) enhancements as enhancements for 0.3.4.

comment:8 Changed 17 months ago by nickm

Keywords: 034-triage-20180328 added

comment:9 Changed 17 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:10 Changed 17 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if time permits.

Note: See TracTickets for help on using tickets.