Patch dir-spec with the shared randomness info

We have proposal250 specifying the shared randomness feature, but that's not enough.

We should extract all the directory-related info from prop250, and patch dir-spec.txt to specify all the changes in vote documents and consensuses.

changed milestone to %Tor: 0.2.9.x-final

added TorCoreTeam201606 component::core tor/tor milestone::Tor: 0.2.9.x-final owner::asn parent::16943 points::1 priority::medium resolution::fixed review-group-3 severity::normal sponsor::R-can status::closed tor-hs tor-spec type::defect labels

Trac:
Milestone: N/A to Tor: 0.2.8.x-final

Throw most 0.2.8 "NEW" tickets into 0.2.9. I expect that many of them will subsequently get triaged out.

Trac:
Milestone: Tor: 0.2.8.x-final to Tor: 0.2.9.x-final

I made the required changes to dir-spec.txt. You can find it in my branch prop250_dirspec: https://gitweb.torproject.org/user/asn/torspec.git/log/?h=prop250_dirspec

When we decide this is the final design, I will merge to torspec.git .

Hi asn. Great start but I'd appreciate if you could document the format of the line attributes. For example, you don't indicate what characters are valid in 'AlgName', 'Identity', etc. See the params line below it for an example...

    "params" SP [Parameters] NL

        [At most once]

        Parameter ::= Keyword '=' Int32
        Int32 ::= A decimal integer between -2147483648 and 2147483647.
        Parameters ::= Parameter | Parameters SP Parameter

Replying to atagar:

Hi asn. Great start but I'd appreciate if you could document the format of the line attributes. For example, you don't indicate what characters are valid in 'AlgName', 'Identity', etc. See the params line below it for an example...

You are right.

Please pull my branch again and check the latest fixup commit. Is that better for you? :)

Trac:
Reviewer: N/A to N/A

Looks good to me, thanks asn! When this becomes live please shoot me an example of these in actual descriptors (for unit tests) and I'd be happy to add it to Stem.

Just a couple minor nitpicks left...

AlgName ::= Any number of ASCII characters

I suspect this can be pinned down a bit more. How about the following?

AlgName ::= 1*(ALPHA / DIGIT / "_" / "-")

Identity ::= 40 hexadecimal characters ([A-Fa-f0-9])

Actually, if you want to use 'Fingerprint' that's already defined...

https://gitweb.torproject.org/torspec.git/tree/control-spec.txt#n206

Trac:
Sponsor: N/A to SponsorR-can

Trac:
Points: N/A to small/medium

Trac:
Cc: dgoulet, atagar to atagar
Owner: N/A to asn
Keywords: N/A deleted, tor-hs added
Points: small/medium to small
Status: new to assigned

Trac:
Status: assigned to needs_revision

Replying to atagar:

Looks good to me, thanks asn! When this becomes live please shoot me an example of these in actual descriptors (for unit tests) and I'd be happy to add it to Stem.

Just a couple minor nitpicks left...

AlgName ::= Any number of ASCII characters

I suspect this can be pinned down a bit more. How about the following?

AlgName ::= 1*(ALPHA / DIGIT / "_" / "-")

Identity ::= 40 hexadecimal characters ([A-Fa-f0-9])

Actually, if you want to use 'Fingerprint' that's already defined...

https://gitweb.torproject.org/torspec.git/tree/control-spec.txt#n206

I addressed both issues you raised above and pushed a fixup commit in my prop250_dirspec.

(FWIW, we don't actually use the dollar sign in fingerprints, so I didn't put it there.)

Trac:
Status: needs_revision to needs_review

AlgName is the hash algorithm that is used

What are some expected values of AlgName here? For example, what is the default value that the new code is going to use? Usually we'll want to specify at least one value that must be supported.

Identity is the authority's SHA1 identity fingerprint

I bet you mean the authority's SHA1 v3 identity fingerprint? If I wanted to be really confused I might think you meant the authority's SHA1 relay identity fingerprint.

"shared-rand-commit"

You say "any number" for votes. Are there constraints on what is allowed to show up in a set of these lines in a given vote? For example, I would expect that maybe there must not be more than one shared-rand-commit line with the same identity fingerprint in a single vote?

Commit is the encoded commitment value in base64

Are there commitment values that are unacceptable? What's the format of it?

And here's where it gets exciting: what is the process of how authorities should take in these shared-rand-commit lines, and then compute the shared-rand-current-value line for their consensus? This needs to be specified, or somebody trying to build a directory authority from this spec won't be able to do it.

Similarly, how do authorities generate the "shared-rand-previous-value" values? I guess they're taken out of some previous consensuses or other past state? What should a dir auth do to generate a shared-rand-previous-value that will conform to spec and be what clients expect?

Min: 1. Max: <Total number of dirauths>. Default: <Total number of dirauths>.

What does "max" mean in dir-spec? I worry that it means that a value higher than max means that the consensus or vote is not conforming to the spec, i.e. that it's an invalid consensus or vote. And if different people have different views on how many dirauths there are, I could totally imagine somebody being surprised by a high number here. Maybe we just want Max as INT32_MAX?

Trac:
Keywords: N/A deleted, review-group-1 added

Trac:
Status: needs_review to needs_revision

Trac:
Points: small to 1

Everything in review-group-1 has had at least 1 review.

Trac:
Keywords: review-group-1 deleted, review-group-2 added

Trac:
Keywords: N/A deleted, , TorCoreTeam201606 added

Replying to arma:

AlgName is the hash algorithm that is used

What are some expected values of AlgName here? For example, what is the default value that the new code is going to use? Usually we'll want to specify at least one value that must be supported.

Identity is the authority's SHA1 identity fingerprint

I bet you mean the authority's SHA1 v3 identity fingerprint? If I wanted to be really confused I might think you meant the authority's SHA1 relay identity fingerprint.

"shared-rand-commit"

You say "any number" for votes. Are there constraints on what is allowed to show up in a set of these lines in a given vote? For example, I would expect that maybe there must not be more than one shared-rand-commit line with the same identity fingerprint in a single vote?

Commit is the encoded commitment value in base64

Are there commitment values that are unacceptable? What's the format of it?

And here's where it gets exciting: what is the process of how authorities should take in these shared-rand-commit lines, and then compute the shared-rand-current-value line for their consensus? This needs to be specified, or somebody trying to build a directory authority from this spec won't be able to do it.

Similarly, how do authorities generate the "shared-rand-previous-value" values? I guess they're taken out of some previous consensuses or other past state? What should a dir auth do to generate a shared-rand-previous-value that will conform to spec and be what clients expect?

Min: 1. Max: <Total number of dirauths>. Default: <Total number of dirauths>.

What does "max" mean in dir-spec? I worry that it means that a value higher than max means that the consensus or vote is not conforming to the spec, i.e. that it's an invalid consensus or vote. And if different people have different views on how many dirauths there are, I could totally imagine somebody being surprised by a high number here. Maybe we just want Max as INT32_MAX?

OK I worked on the comments above, and pushed the changes in my prop250_dirspec branch.

FWIW, I ended up not explaining the whole logic behind the computation of shared-rand-current-value as was requested. Instead I just linked to the right sections of proposal 250. This makes dir-spec.txt less self-contained which is bad, however explaining all that logic would basically mean copying prop250 in dir-spec.txt, and I didn't know how to do this without messing up dir-spec even more. e.g. just pasting the SRV computation formula would not be very useful without specifying which commits are considered active and how commits are validated. Let me know if you think there is a better middle ground here.

Trac:
Status: needs_revision to needs_review

I am going to call this merge-ready, unless somebody objects. This should get merged when the prop250 implementation gets merged.

Trac:
Status: needs_review to merge_ready

These have all had at least one review during review-group-2.

Trac:
Keywords: , review-group-2 deleted, review-group-3 added

#16943 (moved) has been merged. This is now upstream in torspec.

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

closed

changed time estimate to 8h

moved to tpo/core/tor#17435 (closed)