Opened 2 years ago

Closed 2 years ago

#23748 closed defect (fixed)

inconsistent/redundant handling of hs_ed25519_public_key file

Reported by: cathugger Owned by:
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version: Tor: 0.3.2.2-alpha
Severity: Normal Keywords: tor-hs, prop224
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When creating directory for v3 onion services, tor generates 3 files: "hs_ed25519_secret_key" consisting 64 bytes expanded secret key, "hs_ed25519_public_key" consisting 32 bytes public key and "hostname" consisting base32 representation of public key plus checksum, version identifier and ".onion" suffix.
When reading existing v3 onion service directory, however, tor reads only "hs_ed25519_secret_key" file, completely ignoring "hs_ed25519_public_key", even if it holds public key not matching secret key in "hs_ed25519_secret_key".
This can lead to potentially confusing behavior, when hs_ed25519_secret_key is changed, tor subsequently generates new hostname, but hs_ed25519_public_key stays old.
Either hs_ed25519_public_key should not be generated in the first place, as it's unnecessary and redundant, or it should be checked for correctness.

Child Tickets

Attachments (1)

addsplitcheck.patch (740 bytes) - added by cathugger 2 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 2 years ago by dgoulet

Milestone: Tor: 0.3.2.x-final

The public key is really not useful to tor right now.

The reason we have tor write it to disk is because of the not implemented offline key feature where tor will need the public key to generate the .onion but won't load any private key. Instead, it will load a series of blinded key pre-generated by the operator.

That being said, we don't have this feature anyway but we still put the public key on disk so if we get the offline keys let say next version, an operator would be able to remove the private key and tor would work out with the public key.

But true, tor should write the public key on disk if only the private key is found.

comment:2 Changed 2 years ago by cathugger

So it looks like currently keeping public key around is good idea.
I'll add patch which add proper checking of public_key and recreating it if it gets deleted.
INIT_ED_KEY_MISSING_SECRET_OK flag can be added later for blinded keys usecase.

Changed 2 years ago by cathugger

Attachment: addsplitcheck.patch added

comment:3 Changed 2 years ago by dgoulet

Status: newneeds_review

comment:4 Changed 2 years ago by dgoulet

Status: needs_reviewmerge_ready

See branch: bug23748_032_01

It takes cathugger's patch and add a changes file.

comment:5 Changed 2 years ago by asn

LGTM!

Perhaps worth crediting cathugger in the changes file tho.

comment:6 Changed 2 years ago by nickm

Resolution: fixed
Status: merge_readyclosed

merged; added credit.

Note: See TracTickets for help on using tickets.