Opened 8 years ago

Closed 6 years ago

#7484 closed defect (fixed)

We allow */bits as an address-and-mask pattern

Reported by: nickm Owned by:
Priority: Low Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client, nickm-patch
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


When parsing bitmasks for policies, we allow a number of bits along with the "*" catch-all address. That's not what our specs say we allow, and it doesn't make any sense. We should treat it as a bad address-and-mask.

(If anybody is using this now, we might need to deprecate it instead of disabling it, but I don't think anyone is.)

Child Tickets

Change History (9)

comment:1 Changed 8 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: 0.2.5.x-final

Deferring to 0.2.5 as harmless.

comment:2 Changed 7 years ago by nickm

Priority: normalminor
Status: newneeds_review

The obvious fix is in branch "bug7484". I think it's harmless to add it, since at worst we'd make a situation where some authorities and clients stop accepting certain invalid router descriptors before others do... and as soon as the authorities are all up-to-date, that doesn't matter.

comment:3 Changed 7 years ago by andrea

Keywords: 025-triaged added

Triage: keeping for 0.2.5.x

comment:4 Changed 7 years ago by nickm

Keywords: 025-triaged removed
Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

Actually, it's a tiny bit tricksy here. It turns out that it's not so great to have Authority Bob reject a descriptor that Authority Alice accepts. Bob discards it, then notices that Alice lists it, then refetches it.

Deferring to 0.2.6 and adding a new ticket for that issue.

comment:5 Changed 7 years ago by nickm

Blocking on #11243

comment:6 Changed 6 years ago by nickm

Keywords: nickm-patch added

Apply a nickm-patch keyword to tickets in needs_review in 0.2.6 where I wrote the patch, so I know which ones I can('t) review myself.

comment:7 Changed 6 years ago by andrea

The patch looks fine, but this should not be merged until we've fixed #11243.

comment:8 Changed 6 years ago by andrea

(This sort of thing is why we should have a needs_merge state in which to put patches that aren't needs_review or needs_revision any more, but can't yet be merged)

comment:9 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged, now that we have #11243 merged

Note: See TracTickets for help on using tickets.