Opened 3 years ago

Closed 11 months ago

#15056 closed enhancement (implemented)

Support ed25519 identities for circuit extension

Reported by: nickm Owned by: nickm
Priority: High Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version: Tor: 0.2.7
Severity: Normal Keywords: tor-relay, ed25519, prop-220, 027-triaged-1-in, 028-triaged, 201511-deferred, tor-crypto-identity, tor-ed25519-proto, TorCoreTeam201609, nickm-deferred-20161005, review-group-13
Cc: Actual Points:
Parent ID: #15054 Points: parent
Reviewer: dgoulet Sponsor: SponsorU-must

Description

Once #12498 is merged and #15055 is done, we can use ed25519 in circuit extension as documented in proposal 220.

Child Tickets

TicketTypeStatusOwnerSummary
#19301enhancementclosednickmAccept Ed25519 identities in EXTEND2 cells
#19302enhancementclosednickmSend ed25519 IDs in EXTEND2 cells
#19303enhancementclosednickmRevise {extend,create}_cell_format to use trunnel
#20354defectclosednickmprop220: tristate/consensus parameter to decide if we add Ed IDs to extend cells.
#20355defectclosednickmprop220: canonicity should consider all key types

Change History (34)

comment:1 Changed 3 years ago by nickm

Status: newassigned

comment:2 Changed 3 years ago by nickm

Keywords: 027-triaged-1-in added

Marking more tickets as triaged-in for 0.2.7

comment:3 Changed 3 years ago by isabela

Keywords: SponsorU added
Points: large
Priority: normalmajor
Version: Tor: 0.2.7

comment:4 Changed 2 years ago by nickm

Owner: set to nickm

comment:5 Changed 2 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:6 Changed 2 years ago by nickm

Keywords: 028-triaged added

comment:7 Changed 2 years ago by nickm

Keywords: SponsorU removed
Sponsor: SponsorU

Bulk-replace SponsorU keyword with SponsorU field.

comment:8 Changed 2 years ago by nickm

Keywords: TorCoreTeam201511 added

comment:9 Changed 2 years ago by nickm

Type: defectenhancement

comment:10 Changed 23 months ago by nickm

Keywords: TorCoreTeam201512 201511-deferred added; TorCoreTeam201511 removed

Bulk-move uncompleted items to december. :/

comment:11 Changed 22 months ago by nickm

Keywords: TorCoreTeam201601 added; TorCoreTeam201512 removed

These two are going to have to get deferred, because they _can_ be deferred. Not enough hacking days left this year.

comment:12 Changed 21 months ago by nickm

Bulk-modify: It is February 2016, and no longer possible that anything else will get done in January 2016. Time's arrow and all that.

comment:13 Changed 21 months ago by nickm

Keywords: TorCoreTeam201602 added; TorCoreTeam201601 removed

comment:14 Changed 20 months ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.2.9.x-final

These tickets, though owned by me, are not deliverables I can realistically deliver by the 0.2.8 freeze window.

comment:15 Changed 19 months ago by isabela

Sponsor: SponsorUSponsorU-must

comment:16 Changed 19 months ago by nickm

Keywords: TorCoreTeam201602 removed

comment:17 Changed 19 months ago by mikeperry

Keywords: tor-crypto-identity added

comment:18 Changed 18 months ago by nickm

Keywords: tor-ed25519-proto added

comment:19 Changed 17 months ago by isabela

Points: large6

comment:20 Changed 17 months ago by nickm

Points: 6parent
Severity: Normal

comment:21 Changed 14 months ago by nickm

Keywords: TorCoreTeam201609 added

comment:22 Changed 13 months ago by nickm

I've started a branch here, as 15056_wip, based on my #15055 branch. So far, it should (mostly) send ed25519 identities in extend2 cells as appropriate.

The accepting part will be a little harder.

comment:23 Changed 13 months ago by nickm

(I will rebase this branch a lot)

comment:24 Changed 13 months ago by nickm

Keywords: nickm-deferred-20161005 added
Milestone: Tor: 0.2.9.x-finalTor: 0.3.0.x-final

Deferring big/risky-feature things (even the ones I really love!) to 0.3.0. Please argue if I'm wrong.

comment:25 Changed 11 months ago by nickm

Status: assignedneeds_review

This is now implemented, and I believe ready for review. I'm going to be adding more unit tests on and off over the next week, but I think it's worth looking at.

The branch is 'feature15056_v1' in my public repository.

There is a gitlab merge request at https://gitlab.com/nickm_tor/tor/merge_requests/10

comment:26 Changed 11 months ago by nickm

Keywords: review-group-12 added

comment:27 Changed 11 months ago by dgoulet

Reviewer: dgoulet

comment:28 Changed 11 months ago by nickm

Keywords: review-group-13 added; review-group-12 removed

comment:29 Changed 11 months ago by dgoulet

Status: needs_reviewneeds_revision

Did a pass at it! Good work!

comment:30 Changed 11 months ago by dgoulet

Ok, did a second pass. I missed two files during initial review as Gitlab joyfully didn't expand them to me ... so not to myself and everyone, make sure to click the "Expand All" button before review!

comment:31 Changed 11 months ago by nickm

Okay, I think I've been over it all again, and added a little more fixing. How do you feel about it now?

comment:32 Changed 11 months ago by dgoulet

Status: needs_revisionmerge_ready

Fixup all lgtm! #20918 and #20895 have been opened to address some more engineering intensive improvement.

My public relay has been running this for almost 3 days now with ExtendByEd25519ID 1 on and so far it's happy. I've also tested this in chutney confirming that authentication is working with rsa and ed25519 together.

comment:33 Changed 11 months ago by nickm

Okay. I've squashed the branch down to feature15056_v1_squashed. You can verify that it has the same contents as feature15056_v1, except with fewer commits in it.

Merging that to master.

Let's see what happens now!

comment:34 Changed 11 months ago by nickm

Resolution: implemented
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.