Opened 2 years ago

Closed 8 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 2 years ago by nickm

  • Status changed from new to assigned

comment:2 Changed 2 years ago by nickm

  • Keywords 027-triaged-1-in added

Marking more tickets as triaged-in for 0.2.7

comment:3 Changed 2 years ago by isabela

  • Keywords SponsorU added
  • Points set to large
  • Priority changed from normal to major
  • Version set to 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 changed from Tor: 0.2.7.x-final to Tor: 0.2.8.x-final

comment:6 Changed 22 months ago by nickm

  • Keywords 028-triaged added

comment:7 Changed 22 months ago by nickm

  • Keywords SponsorU removed
  • Sponsor set to SponsorU

Bulk-replace SponsorU keyword with SponsorU field.

comment:8 Changed 21 months ago by nickm

  • Keywords TorCoreTeam201511 added

comment:9 Changed 21 months ago by nickm

  • Type changed from defect to enhancement

comment:10 Changed 20 months ago by nickm

  • Keywords TorCoreTeam201512 201511-deferred added; TorCoreTeam201511 removed

Bulk-move uncompleted items to december. :/

comment:11 Changed 19 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 18 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 18 months ago by nickm

  • Keywords TorCoreTeam201602 added; TorCoreTeam201601 removed

comment:14 Changed 17 months ago by nickm

  • Milestone changed from Tor: 0.2.8.x-final to Tor: 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 16 months ago by isabela

  • Sponsor changed from SponsorU to SponsorU-must

comment:16 Changed 16 months ago by nickm

  • Keywords TorCoreTeam201602 removed

comment:17 Changed 16 months ago by mikeperry

  • Keywords tor-crypto-identity added

comment:18 Changed 15 months ago by nickm

  • Keywords tor-ed25519-proto added

comment:19 Changed 14 months ago by isabela

  • Points changed from large to 6

comment:20 Changed 14 months ago by nickm

  • Points changed from 6 to parent
  • Severity set to Normal

comment:21 Changed 11 months ago by nickm

  • Keywords TorCoreTeam201609 added

comment:22 Changed 10 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 10 months ago by nickm

(I will rebase this branch a lot)

comment:24 Changed 10 months ago by nickm

  • Keywords nickm-deferred-20161005 added
  • Milestone changed from Tor: 0.2.9.x-final to Tor: 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 8 months ago by nickm

  • Status changed from assigned to needs_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 8 months ago by nickm

  • Keywords review-group-12 added

comment:27 Changed 8 months ago by dgoulet

  • Reviewer set to dgoulet

comment:28 Changed 8 months ago by nickm

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

comment:29 Changed 8 months ago by dgoulet

  • Status changed from needs_review to needs_revision

Did a pass at it! Good work!

comment:30 Changed 8 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 8 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 8 months ago by dgoulet

  • Status changed from needs_revision to merge_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 8 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 8 months ago by nickm

  • Resolution set to implemented
  • Status changed from merge_ready to closed
Note: See TracTickets for help on using tickets.