Opened 4 years ago

Closed 4 years ago

#16580 closed defect (fixed)

Reload keypins on SIGHUP? Or provide some other way to undo a single keypin?

Reported by: nickm Owned by: nickm
Priority: Very High Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: TorCoreTeam201507
Cc: Actual Points:
Parent ID: #16530 Points:
Reviewer: Sponsor:

Description (last modified by nickm)

Right now, there isn't a way to undo a buggy key-pin without stopping the authority, editing the keypin file, and restarting it. Not good: authority operators shouldn't have to reboot just because we had a bug.

We should fix this before we release 0.2.7.2-alpha.

I see two four six options here.

  1. Make it okay to edit the key-pinning journal on a running Tor. That's not so great; we need to be able to append to it, and editors may have swap-file races with it.
  2. Add a torrc option to unpin an existing key. This would only need to be stuck into the torrc once; it would remove the pin, and allow a new key pin to occur.
  3. No fix; hope that this situation never happens again; tell the authoritiy ops to edit the keypinning file when they upgrade, or give them a script to do it.
  4. One-off fix: undo the pin in software for the two specific keypairs affected, and hope this never happens again.
  5. As 3, but tell the ops to remove the file.
  6. As 5, but have Tor use a new file name, and remove the old one it exists, so that the ops don't have to do anything at all.

Child Tickets

Change History (9)

comment:1 Changed 4 years ago by nickm

Description: modified (diff)

comment:2 Changed 4 years ago by nickm

Description: modified (diff)

comment:3 Changed 4 years ago by nickm

Status: newneeds_review

feature_16580 implements approach 6.

comment:4 Changed 4 years ago by rl1987

Looks good to me.

comment:5 Changed 4 years ago by nickm

Keywords: TorCoreTeam201507 added

comment:6 Changed 4 years ago by nickm

Okay. I think I should hold off on merging this till we've given the changes I just merged for #16581 and #16582 to settle in: it would be silly to wipe keypinning at the exact moment when I introduce a serious key storage bug.

(Which I hope I didn't do, but you never know)

comment:7 Changed 4 years ago by nickm

Owner: set to nickm
Status: needs_reviewassigned

comment:8 Changed 4 years ago by nickm

Status: assignedneeds_review

comment:9 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Nothing seems to have exploded yet. Merged to master.

Note: See TracTickets for help on using tickets.