Opened 17 months ago

Last modified 5 months ago

#30642 needs_review enhancement

Add an ed25519-identity file to the data directory

Reported by: teor Owned by: neel
Priority: Medium Milestone: Tor: 0.4.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-relay easy 044-can
Cc: neel, nickm Actual Points:
Parent ID: Points: 1
Reviewer: dgoulet Sponsor:

Description

Relays print their RSA fingerprint to a "fingerprint" file in their data directory.

We need an equivalent file for base-64 encoded ed25519 public keys.

Child Tickets

Change History (26)

comment:1 Changed 17 months ago by neel

Owner: set to neel
Status: newassigned

comment:2 Changed 17 months ago by teor

Please make a separate branch for this change, it is independent of the parent ticket.

Please write unit tests, and update the files section in the man page.

comment:3 Changed 14 months ago by teor

Owner: neel deleted

This ticket hasn't made progress, so I'm un-assigning it.

comment:4 Changed 10 months ago by nickm

Keywords: easy added
Parent ID: #22029

comment:5 Changed 10 months ago by neel

Owner: set to neel

#22029 was accepted and will get merged, so re-assigning this to me.

comment:6 Changed 10 months ago by neel

Status: assignedneeds_review

comment:7 Changed 10 months ago by neel

Status: needs_reviewneeds_revision

Don't have tests, setting as needs revision.

comment:8 Changed 10 months ago by neel

Status: needs_revisionneeds_review

Added tests, setting as needs review.

comment:9 Changed 10 months ago by nickm

Keywords: 043-can added
Milestone: Tor: unspecifiedTor: 0.4.3.x-final

comment:10 Changed 10 months ago by dgoulet

Reviewer: mikeperry

comment:11 Changed 10 months ago by teor

Status: needs_reviewneeds_revision

I made a suggestion on the pull request:

Rather than copying and pasting code from router_write_fingerprint(), we could add an argument to that function that makes it write the ed25519 identity.

Last edited 10 months ago by teor (previous) (diff)

comment:12 Changed 10 months ago by neel

Status: needs_revisionneeds_review

comment:13 Changed 9 months ago by ahf

Reviewer: mikeperry

comment:14 Changed 9 months ago by dgoulet

Reviewer: dgoulet
Status: needs_reviewneeds_revision

teor's suggestion has not been fixed but I also see they were waiting for mikeperry to weigh in.

I think that is a good request. Within keys/, we use the underscore but at the root of the DataDirectory everything uses a dash - so lets go with that which is ed25519-identity file name.

comment:15 Changed 8 months ago by teor

Summary: Add an ed25519_identity file to the data directoryAdd an ed25519-identity file to the data directory

comment:16 Changed 8 months ago by teor

Keywords: prop311-can added; 043-can removed
Milestone: Tor: 0.4.3.x-finalTor: 0.4.4.x-final
Parent ID: #33050
Sponsor: Sponsor55-can

0.4.3 is in feature freeze, this will have to go in 0.4.4 or later.

comment:17 Changed 8 months ago by teor

Keywords: prop311-can removed
Sponsor: Sponsor55-can

We don't need to do this task as part of the Relay IPv6 project any more, because in #33428 we loaded the key file directly into chutney.

comment:18 Changed 8 months ago by teor

Parent ID: #33050

comment:19 Changed 7 months ago by neel

Status: needs_revisionneeds_review

comment:20 Changed 5 months ago by dgoulet

Cc: nickm added

Ok so this looks great. Works great.

The only thing I worry is the filename actually. ed25519-identity super sounds like a "secret key" to me as in the private portion...

Here are my proposals, not sure what is the best but I also don't want to bikeshed this so no strong opinion but this file should really just denote the concept of "public" and ed25519 terminology is _very_ technical and probably wont say much to relay operators in general.

What if we append the ed25519 base64 pubkey to the fingerprint file. I worry that there will be confusion on "what line should I send you?" when someone is asking for a fingerprint. But, also any line should be fine to find the relay, maybe that doesn't matter. And I forsee rare occasion someone will ask "Oh give me the RSA fingerprint"...

Creating a second file here, I worry much about confusion and something that seems not related to a "relay fingerprint". Because in the end, that is just ed25519_master_id_public_key file in base64.

comment:21 in reply to:  20 Changed 5 months ago by teor

Replying to dgoulet:

What if we append the ed25519 base64 pubkey to the fingerprint file. I worry that there will be confusion on "what line should I send you?" when someone is asking for a fingerprint. But, also any line should be fine to find the relay, maybe that doesn't matter. And I forsee rare occasion someone will ask "Oh give me the RSA fingerprint"...

That will break any tools that read the fingerprint file.

comment:22 Changed 5 months ago by dgoulet

That will break any tools that read the fingerprint file.

Right... so then it will be another file and we can call it something like fingerprint-new or ... Just not sure the use of ed25519 is wise here but maybe...

comment:23 Changed 5 months ago by nickm

In an IRC conversation I just endorsed fingerprint-ed25519

comment:24 Changed 5 months ago by neel

Sorry for the delay.

A new PR is here: https://github.com/torproject/tor/pull/1917

This uses fingerprint-ed25519. A new PR is needed because I had to rename the commit messages.

comment:25 Changed 5 months ago by nickm

Keywords: 043-can added

comment:26 Changed 5 months ago by nickm

Keywords: 044-can added; 043-can removed
Note: See TracTickets for help on using tickets.