Opened 5 months ago

Last modified 3 months ago

#27198 needs_information defect

protover doesn't forbid version zero

Reported by: cyberpunks Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: 0.2.9.4-alpha
Severity: Normal Keywords:
Cc: nickm Actual Points:
Parent ID: Points:
Reviewer: teor Sponsor:

Description

dir-spec.txt says:

Int = NON_ZERO_DIGIT
Int = Int DIGIT

But the code in protover.c allows Foo=0.

Child Tickets

TicketStatusOwnerSummaryComponent
#27201needs_informationrust/protover doesn't forbid version zeroCore Tor/Tor

Change History (7)

comment:1 Changed 5 months ago by cyberpunks

See the protozero1 branch at ​​​https://gitgud.io/onionk/tor.git

comment:2 Changed 5 months ago by nickm

Milestone: Tor: 0.3.5.x-final
Status: newneeds_review

comment:3 Changed 5 months ago by teor

Cc: nickm added

I think we might want to fix the spec to permit 0 instead.
What do you think, nickm?

Also, I think that it's ok to accept "01" and treat it as "1", but authorities should not put leading zeroes in the consensus.

comment:4 Changed 5 months ago by nickm

I agree with you on both counts.

comment:5 in reply to:  3 ; Changed 5 months ago by cyberpunks

Problem: it looks like there's already code that depends on there being no version 0, in protover_all_supported().

comment:6 in reply to:  5 ; Changed 5 months ago by teor

Status: needs_reviewneeds_information

Replying to cyberpunks:

Problem: it looks like there's already code that depends on there being no version 0, in protover_all_supported().

Can you be more specific?

Which code allows 0 versions, and which code doesn't?
And what happens in the vote and the consensus when a 0 version is specified by a relay?

comment:7 in reply to:  6 Changed 3 months ago by teor

Reviewer: teor

Replying to teor:

Replying to cyberpunks:

Problem: it looks like there's already code that depends on there being no version 0, in protover_all_supported().

Can you be more specific?

Which code allows 0 versions, and which code doesn't?
And what happens in the vote and the consensus when a 0 version is specified by a relay?

We can't resolve the spec issues, #27201, and #27198 until these questions are answered.

Also, this branch seems to have a lot of unrelated changes in it.
Can you please cherry-pick the "zero" changes on top of master?

If you can't do these things, just let us know, and we will do them eventually.

Note: See TracTickets for help on using tickets.