Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20650 closed defect (fixed)

confusing "I need to load the permanent master identity key" line

Reported by: arma Owned by: s7r
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version: Tor: 0.2.7.3-rc
Severity: Normal Keywords: 029-backport, review-group-12
Cc: s7r Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor:

Description

I ran my Tor as a bridge for the first time in apparently a while, and it gave me this message on startup:

Nov 13 02:26:37.814 [notice] It looks like I need to generate and sign a new medium-term signing key, because the one I have is expired. To do that, I need to load the permanent master identity key.

I've never done anything about permanent master identity keys or anything on this Tor. I assume that this log line means that Tor did everything for me? The "I need to load" part is confusing because it makes me think that there's something more to be done -- there were no follow-up log lines about how it worked or succeeded or is happy or anything now.

Looks related to commit c88a8a7c, which went into Tor 0.2.7.3-rc and might have come from #16685.

Child Tickets

Change History (13)

comment:1 Changed 3 years ago by arma

Summary: confusing "I need to load the permanent master identity key" lineconfusing "I need to load the permanent master identity key" line

comment:2 Changed 3 years ago by s7r

The bridge in question was not started for the very first time (when Tor takes care of everything) and it had older temporary signing key and certificate in $datadirectory/keys - the default lifetime if not set different is 30 days. I think Tor prints a notice just for information purpose every time it wants to load the master identity key (temporary signing key is about to expire soon) and every time it needs to load the master identity key (temporary signing key is already expired). If the master key is not offline, no further action is required from the operator and Tor just does everything by itself and continues normally.

We wanted to print these notices only when OfflineMasterKey 1 is set, but this doesn't suite the operators that do not configure this setting, allow Tor for the first start to generate the master identity key and move it from $datadirectory/keys immediately after. It also doesn't cover the operators that leave the master identity key there, but encrypt it with a passphrase. So OfflineMasterKey is more about instructing Tor never even try to generate or load a master identity key, so it doesn't touch the hard drive of that server (dirauth use case). The log messages in OfflineMasterKey 1 cases should be different anyway and who enables this feature knows that further action on regular basis is required.

So, what we could do here is append to the notices printed both when Tor wants to load or needs to load the master identity key stating something like:

If you did not either move the master identity key away or encrypt it with a passphrase, no further action is required and this notification can be ignored. Otherwise, please use --keygen and provide new valid medium term signing key and certificate.

What do you think? I am thinking if we should ditch the last part or maybe it's useful?

comment:3 Changed 3 years ago by s7r

We have follow up messages but only in case of failure, where Tor exits and states why.

Another solution would be to leave the existent notifications as they are and print an additional message after Tor successfully done it automatically and is happy. Something like:

Tor was able to load the master identity key and automatically renewed the medium-term signing key. No further action is required, proceeding normally.

Which solution do you think is better? I have a feeling that just modifying the current notices is trivial and pretty much covers it.

comment:4 Changed 3 years ago by s7r

Owner: set to s7r
Status: newassigned

comment:5 Changed 3 years ago by s7r

Keywords: backport added
Status: assignedneeds_review

See my ticket_20650 branch on https://github.com/gits7r/tor.git commit e2de839

The message is the same when --keygen is called manually so adapted the words in a way that I think sounds fine for all cases. If you think this is OK, we can put this in merge ready.

comment:6 Changed 3 years ago by dgoulet

Keywords: 029-backport added; backport removed
Milestone: Tor: 0.3.0.x-final

Worth fixing in 030. Let's consider this as well for 029 as we are still in alpha but after that we are stable so not sure this one qualifies as valid for stable backport.

comment:7 Changed 3 years ago by nickm

Keywords: review-group-12 added

comment:8 Changed 3 years ago by nickm

Reviewer: nickm
Status: needs_reviewmerge_ready

Thanks! This looks good for me, with two issues:

1) The commit message is really vague.
2) Some of the lines are wide, so "make check-spaces" will complain.

I'm okay with fixing those up as I merge, unless you'd rather fix them up yourself.

comment:9 Changed 3 years ago by s7r

Thanks for the review. I tried to cover both of the issues and another small grammar fix in the latest commit 215cc0d in my ticket_20650 branch on https://github.com/gits7r/tor

comment:10 Changed 3 years ago by nickm

Status: merge_readyneeds_review

comment:11 Changed 3 years ago by nickm

Much better. Merging this to master. Thank you!

comment:12 Changed 3 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

comment:13 Changed 3 years ago by nickm

Except, it didn't compile. Please remember to check that before submitting a patch?

Note: See TracTickets for help on using tickets.