Opened 4 months ago

Closed 3 months ago

#30644 closed enhancement (fixed)

Add our own base-64 encoded ed25519 public key in dirserv_add_own_fingerprint()

Reported by: teor Owned by: neel
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-ed25519
Cc: neel Actual Points:
Parent ID: #22029 Points:
Reviewer: Sponsor:

Description

We need to add our own ed25519 identity when we add our own RSA fingerprint.

We should have separate static functions for RSA and ed25529, and call them from dirserv_add_own_fingerprint().

Child Tickets

Change History (8)

comment:1 Changed 4 months ago by neel

Owner: set to neel
Status: newassigned

comment:2 Changed 3 months ago by neel

Status: assignedneeds_review

comment:3 Changed 3 months ago by teor

Keywords: tor-ed25519 added
Milestone: Tor: unspecifiedTor: 0.4.2.x-final
Status: needs_reviewneeds_revision

Thanks for this change!

This code is independent of the rest of #22029, so please make a separate branch for it.
Then someone else can review that branch in this ticket.

The code needs these changes:

  • add_rsa_fingerprint_to_dir() can fail and return -1, if it does, dirserv_add_own_fingerprint() should return -1
  • add_ed25519_to_dir() can fail and return -1, if it does, dirserv_add_own_fingerprint() should return -1

You can do the check and the warning like this:
https://github.com/torproject/tor/pull/970/commits/f1a236a8360ce0af2ad1bece4e89960463e57e3f#diff-c535b81e06d7fed27d5d3cdaabf1c4b8R160
But please use a different log message for each warning.

comment:4 Changed 3 months ago by teor

neel created this pull request for this ticket:
https://github.com/torproject/tor/pull/1125

comment:5 in reply to:  3 Changed 3 months ago by teor

Replying to teor:

The code needs these changes:

  • add_rsa_fingerprint_to_dir() can fail and return -1, if it does, dirserv_add_own_fingerprint() should return -1
  • add_ed25519_to_dir() can fail and return -1, if it does, dirserv_add_own_fingerprint() should return -1

You can do the check and the warning like this:
https://github.com/torproject/tor/pull/970/commits/f1a236a8360ce0af2ad1bece4e89960463e57e3f#diff-c535b81e06d7fed27d5d3cdaabf1c4b8R160
But please use a different log message for each warning.

The code should also have unit tests.

Please remove all the #22029 commits from the branch: this branch should start with "Add our own base-64 encoded ed25519 public key in dirserv_add_own_fingerprint()" and any fix ups and tests.

comment:6 Changed 3 months ago by neel

Here's a problem: add_ed25519_to_dir() was introduced in #22029 and the commits before "Add our own base-64 encoded ed25519 public key in dirserv_add_own_fingerprint()".

This commit adds add_ed25519_to_dir():

Allow ed25519 keys to be banned in approved-routers
commit 7267177b89e7db4f9482e33614f813564c198ec1

comment:7 Changed 3 months ago by neel

I have added a test, but it uses the same branch with the #22029 commits.

The test is in commit: 84d95f3.

I am unable to do this PR separate from the #22029 commits because #22029 introduces the ed25519 key handling.

comment:8 Changed 3 months ago by teor

Resolution: fixed
Status: needs_revisionclosed

Ok, let's put all the commits for this ticket and #22029 on https://github.com/torproject/tor/pull/970 then.
It has the pull request review comments, and all the code that this ticket needs.

If I want to split up the branch for merging, I'll do that after I review it.

Note: See TracTickets for help on using tickets.