identity-ed25519 is undocumented

On reflection suspect you'd rather #16227 (moved) to be specifically about documenting future expansion. Spec 228 doesn't include a proposal for what would be added to the dir-spec. As such controllers like Stem, metrics-lib, and BridgeDB don't have a clear description on how they should be parsing this new content.

Happy to review it when we have a proposed spec addition.

changed milestone to %Tor: 0.2.7.x-final

added TorCoreTeam201508 component::core tor/tor milestone::Tor: 0.2.7.x-final owner::nickm priority::high resolution::fixed status::closed type::defect labels

Should the priority of this be bumped? Proposal 228 was incomplete, and if this wasn't an important security improvement I'd argue we should be rolling it back until that's fixed.

Karsten is moving forward with descriptor sanitation. The following was my reply to his request for comment...

Hi Karsten. My thought is that the existing ED stuff doesn't have a
specification, I don't know what it does, and think that should be
fixed before we worry about how it's sanitized. The spec was
incomplete and personally think we should either prioritize that or
roll back the feature. But maybe that's impractical. I already voiced
this earlier so feel free to proceed as you'd like. You have my
complete confidence. :P

Documenting (or rolling back) this feature should be easy so I'm surprised this is still an issue.

Apologies if this ruffles feathers, but personally I think the proposal process for this was mishandled. We shouldn't be merging major features that lack a spec, then leave them undocumented for a full month afterward. :(

On reflection think I was looking at the wrong proposal. Proposal 220 discusses the descriptor fields, though it also lists itself as being a draft (not merged) so still tad confused about the state of things. :)

Ok, think I'm gonna be a madman and bump the severity on this. Unless I'm missing something we merged a major feature and have left it completely out of the spec for months. I know we're all kept busy but this feels like a miss, and a tad frustrated it's been six weeks of hearing crickets.

Trac:
Priority: normal to major

Tweaking the ticket description to be more precise about what I care about. I'm waiting for this before Stem will recognize the new descriptor fields. Presently there's 45 server descriptors with the new fields...

% ./tor-prompt
>>> from stem.descriptor import remote
>>> downloader = remote.DescriptorDownloader()
>>> query = downloader.get_server_descriptors()
>>> results = query.run()
>>> for desc in results:
...   if desc.get_unrecognized_lines():
...     print desc.get_unrecognized_lines()
[u'identity-ed25519 \n-----BEGIN ED25519 CERT-----\nAQQABhpQARSbXrElkqVNjtI8JDPBdghiviwQmwoygc/rdTcnqCHiAQAgBACVQEQ/\nWeqTHXR+mXH/aCVJ9uZqUIpu3s0AuMwDm7Ki9jqXpHNbZfrPR3qaQhCM5I660RFM\nfCHYKQ1+DHJyJoh6s4nStl0JPkq+U5WEQfUlAHlU0bhSuimwNdxNM0oEQgU=\n-----END ED25519 CERT-----', u'master-key-ed25519 lUBEP1nqkx10fplx/2glSfbmalCKbt7NALjMA5uyovY', u'onion-key-crosscert \n-----BEGIN CROSSCERT-----\nnqsunFCwziRKIvVomzJRKtGEloFavu7NLAAZqdZeJjJPiFAcCdSvh1KG2fg/hhWQ\nrIqlFyNLXWAa1Mnhw/Ovcn4tzQexUU9vHnixZUOfITId4DGoGfCd17ZB8FzTQ306\naTlRwNYQVfmyv8TvHf8M/vKSFC+NC3TIzh4xfZ+z/KM=\n-----END CROSSCERT-----', u'ntor-onion-key-crosscert 0\n-----BEGIN ED25519 CERT-----\nAQoABhiFAZVARD9Z6pMddH6Zcf9oJUn25mpQim7ezQC4zAObsqL2AK34+b2/L0a9\nOyvXaT0Q8BJJjJ3VSm2E2+prUDOemahUxqdRXj1ICGLPVC3JYJ6hSLep/a2JPhpz\nIl6WsM9bkwE=\n-----END ED25519 CERT-----', u'router-sig-ed25519 V9jQvck/gA1p3VWi7yDOy71H44eo3pNroNiukWaPrvqfWYYlR0/zfwXLk/liLKi8SIpjCUKp+nUDPWn85tihDg']
[u'identity-ed25519 \n-----BEGIN ED25519 CERT-----\nAQQABhpoAScsjb4GMSGjE6T7s+22l5ZR4gNdu3GOgIAEPj8KjyaOAQAgBACRjdda\n5NzyMPPGKOiU8sI0JGUiElfupqmHHDgPeijms7yAq1jUQYB3uCyuaG2DhH4gxsd4\n4j3Ty1QiaDiiPLCXX3KP9E2Nw9FUrDBGSePo3J3AWzrdXOtAbnOgCaD4xgQ=\n-----END ED25519 CERT-----', u'master-key-ed25519 kY3XWuTc8jDzxijolPLCNCRlIhJX7qaphxw4D3oo5rM', u'onion-key-crosscert \n-----BEGIN CROSSCERT-----\nBQCW3hG5y/9wWDe9zwsXyeFKW62IoS7LNDCPletOmzlw2mo7wXnxugMBG0Y2VFH8\nGlenh0Wps7EpjGZ43caS0/mGaAaXWS4wPSPieikxYSlv5cIkCE6dneaHIO2PVVGU\noQSRbhKMzwdnXJXm+dCA/AKpCVFbpkbYNBTnWYPAeuE=\n-----END CROSSCERT-----', u'ntor-onion-key-crosscert 1\n-----BEGIN ED25519 CERT-----\nAQoABhh8AZGN11rk3PIw88Yo6JTywjQkZSISV+6mqYccOA96KOazABwUZc+dbkBP\nBUzzLzit8cKl6LRiWXRoURVNmY5zdnTQtwgQQbsoWfVSeuWgIRFTIxvp2HRn1QS8\nPu0nBlsuZw0=\n-----END ED25519 CERT-----', u'router-sig-ed25519 oWxFdlGijXjo5CdwoV1WEXUeDQwLyfjvLPTqtXvYUjhRAVf74PpfzkcN3CYhV684S+uh3PztadAYEMeszszODw']
...

Presently only spot this is mentioned is spec 220...

% torspec$ grep -R 'identity-ed25519' ./*
./proposals/220-ecc-id-keys.txt:      "identity-ed25519" NL "-----BEGIN ED25519 CERT-----" NL certificate
./proposals/220-ecc-id-keys.txt:   When an identity-ed25519 element is present, there must also be a
./proposals/220-ecc-id-keys.txt:      * If the identity-ed25519 line is present, it must be well-formed,
./proposals/220-ecc-id-keys.txt:   Extra-info documents now include "identity-ed25519" and

Trac:
Summary: Proposal 228 needs spec documentation to identity-ed25519 is undocumented

Trac:
Status: new to accepted
Milestone: N/A to Tor: 0.2.7.x-final
Keywords: N/A deleted, TorCoreTeam201508 added
Owner: N/A to nickm

I've started this as a branch called ed25519. More work to do.

Trac:
Status: accepted to needs_review

I'm merging the branch as-is as "better than nothing". Please reopen, or open a new ticket, if more is needed.

Trac:
Status: needs_review to closed
Resolution: N/A to implemented

Thanks Nick! Added Stem support for the new fields. Spec's almost perfect, only a couple minor questions...

"identity-ed25519" NL "-----BEGIN ED25519 CERT-----" NL certificate
       "-----END ED25519 CERT-----" NL

    [At most once, in second or first position in document]

Above the 'router' says it's at the start and appears exactly once. How can identity-ed25519 be in the first position? Is there a reason we care about its position in the server descriptor? If not then might as well just say "[At most once]" like everything else.

"id" SP "ed25519" SP base64-encoded-ed25519-identity NL

  [At most once]

Lets merge this with the above entry for 'id rsa1024'. Can microdescriptors and router status entries have multiple 'id' lines? This makes it sound like it can but Stem assumed from the rsa id line's wording that it wouldn't.

Trac:
Resolution: implemented to N/A
Status: closed to reopened

How can identity-ed25519 be in the first position?

It will in some future version of the spec. But for now, it's second-position.

Is there a reason we care about its position in the server descriptor? If not then might as well just say "[At most once]" like everything else.

I'd like implementors to be able to do a parsing algorithm that gets the key and checks the signature first, and only does parsing if the key checks.

Can microdescriptors and router status entries have multiple 'id' lines?

Yes, at most once per key type. Tried to clarify that in the spec.

Changes are in commit 09ff9e202d4a169e95d6962c710bb05fcf062a49 . Thanks!

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

Great, thanks Nick!

Just for the future the prior reading of the 'id rsa1024' lines read to me as 'there is only one id line'...

"id" SP "rsa1024" SP base64-encoded-identity-digest NL

   [At most once]
...

Not at all a big whoop, but I'll need to rejigger things a bit in Stem in ways I try to avoid. For me how many times a field appears is important since it determines the data structures I use. If we can keep forward comparability in mind early it's helpful.

The field as it is now is perfect though - thanks!

Added support for multiple 'id' lines to Stem (change).

closed

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