Opened 4 years ago

Closed 4 years ago

#16769 closed defect (implemented)

add two new functions when manually calling --keygen for better management

Reported by: s7r Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version: Tor: 0.2.7.2-alpha
Severity: Keywords: ed25519, relay, keys, TorCoreTeam201509, PostFreeze027
Cc: Actual Points:
Parent ID: #16645 Points:
Reviewer: Sponsor:

Description

Currently when --keygen is automatically called by Tor, it will define the variables (datadirectory, SigningKeyLifetime, etc.) from torrc and/or init.d/rc scripts and use those values to look for the master ID key and save output files (signing cert and signing key). This is working excellent in ed25519_keygen branch and we should not change anything.

What we need to do is add more functions to --keygen when it is manuall called by the user, in order to make it possible to do simple things, such as: generating a signing cert and signing key from master ID key backed up on a non-writeable media. Also, since we offer the possibility to password protect the master ID key, we should also offer the possibility to change the password in future.

Again: all these should be only used when user manually calls --keygen. Tor knows what to do when it is called automatically.
Currently, when manually calling tor --keygen Tor, will only care about a --datadirectory argument, where it will look for the ed25519_master_id_secret_key(_encrypted) and also save the output files (ed25519_master_id_public_key; ed25519_signing_cert; ed25519_signing_secret_key). The current behavior when we call --keygen with --datadirectory is good and doesn't require any change. Few more functions needed:

1. Specify the exact location of the master ID key and location for the output files separately:

tor --masterkey /mnt/cdrom/relay_x_master_id_key --out /var/lib/tor/keys/ --keygen

  • The master ID secret key file can have any name, as opposite to --datadirectory (where Tor will only look for ed25519_master_id_secret_key(_encrypted)). Tor will detect if the key is encrypted or not and ask for the password if it is.
  • --out /path/to/folder will tell Tor the folder where it should save the output files (ed25519_master_id_public_key; ed25519_signing_cert; ed25519_signing_secret_key). In case there is no --out specified, save to current working directory where the command is run. The output files will be saved with their default filenames, ready to be moved to keys folder.
  • We create the files with the default lifetime of 30 days, unless user also specifies --SigningKeyLifetime 'n days/weeks/months' when calling, for example:

tor --masterkey /mnt/cdrom/relay_x_master_id_key --out /var/lib/tor/keys/ --SigningKeyLifetime '10 days' --keygen

2. Add a feature to add/remove or change password:

tor --masterkey /path/to/master_id_key --newpass --keygen

  • Here we can specify the exact master ID key file, it isn't a must to have the exact name: ed25519_master_id_secret_key(_encrypted).

tor --datadirectory /path/to/foolder --newpass --keygen

  • Here Tor will look for ed25519_master_id_secret_key(_encrypted) in the folder specified with --datadirectory.

If it is encrypted, we ask for the current password to decrypt it and 2 times for a new password. If new password and confirm new password fields are left blank, it means the user wants to decrypt it permanently. Vice versa, if it is not encrypted, and the user provides a password and confirms it, encrypt it with that password.
Here we modify the file in place, we delete the old one and save the new one with the same name (append _encrypted at the end of the filename if we just encrypted it or remove this suffix if we just decrypted it). Warn and exit in case we couldn't modify the file.

Child Tickets

Change History (17)

comment:1 Changed 4 years ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 Changed 4 years ago by s7r

Also, in ed25519_hup branch, when we manually call --keygen to generate medium term signing key and certificate, if it finds the master ID key it will generate and save them to disk as expected, but will show this output:

# tor --datadirectory ~/torsrc/data/ --SigningKeyLifetime '2 days' --keygen
Aug 18 16:32:23.968 [notice] Tor v0.2.7.2-alpha-dev (git-09326b2fe160eb4d) running on Linux with Libevent 2.0.19-stable, OpenSSL 1.0.1e and Zlib 1.2.7.
Aug 18 16:32:23.968 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Aug 18 16:32:23.968 [notice] This version is not a stable Tor release. Expect more bugs than usual.
Aug 18 16:32:23.968 [notice] Configuration file "/usr/local/etc/tor/torrc" not present, using reasonable defaults.
Aug 18 16:32:23.972 [warn] You are running Tor as root. You don't need to, and you probably shouldn't.
Aug 18 16:32:23.972 [notice] It looks like I need to generate and sign a new medium-term signing key, because I don't have one. To do that, I need to load (or create) the permanent master identity key

Could we change the last log line here (or add another log line, if that one is used/needed when Tor calls --keygen automatically) to something like: Successfully generated and saved to disk medium signing key and certificate for the mater identity provided.

comment:3 Changed 4 years ago by nickm

Keywords: TorCoreTeam201509 PostFreeze027 added; TorCoreTeam201508 removed

comment:4 Changed 4 years ago by nickm

I've got the --newpass part implemented in a branch feature16769. Working on the directory stuff. That's a bit trickier.

comment:5 Changed 4 years ago by nickm

Status: acceptedneeds_review

The --out part seems to be redundant with --datadirectory; I think it's okay to skip that.

I have --master-key implemented now in feature16769 too. Needs testing and review.

comment:6 Changed 4 years ago by s7r

--newpass:

Works, but I have noticed that whenever it encrypts/decrypts or changes the passphrase of the master key, it also generates new signing cert and medium term secret signing key, regardless if it still has valid ones or not. I think we should make --newpass care only about encrypting/decrypting or changing the passphrase of the master identity key and shouldn't generate cert/medium term signing key.

  1. Encryption (it kept the same ed25519_master_id_public_key the same [OK], and generated new cert and medium term signing key):

a) Before encryption (sha256sum):

589f0546da8bb3ae3b663c77fe21fca1609fbde28584b21945bb58adb543dbe6  ed25519_master_id_public_key
051525483aeaf7e82e4354576bf1def4d5312541be9dadb247ab808ec165aa66  ed25519_master_id_secret_key
6fc98db31dda5e018e20b02abc4f073809a8ee98fc3b18101455570689ebc543  ed25519_signing_cert
dd5b996913814e910a01cbe639169796dab9587d6a782318635b38896ffc2fa0  ed25519_signing_secret_key

b) After encryption (sha256sum):

589f0546da8bb3ae3b663c77fe21fca1609fbde28584b21945bb58adb543dbe6  ed25519_master_id_public_key
f0c2790507ccc93e17b9c86abade3c50371f4b95e8760282b9c84e2d30b2d900  ed25519_master_id_secret_key_encrypted
20df926d3bcf7f2da1068018f5a6e8066f403514fad1c7538d1ce35f95227c1a  ed25519_signing_cert
37e203316e3cd63a948ce5e40a92776abaf675bafe681a94d66f2ddd498c20c8  ed25519_signing_secret_key
  1. Decryption (it kept the same ed25519_master_id_public_key [OK], again generated new cert and medium term signing key):
589f0546da8bb3ae3b663c77fe21fca1609fbde28584b21945bb58adb543dbe6  ed25519_master_id_public_key
051525483aeaf7e82e4354576bf1def4d5312541be9dadb247ab808ec165aa66  ed25519_master_id_secret_key
f0c2790507ccc93e17b9c86abade3c50371f4b95e8760282b9c84e2d30b2d900  ed25519_master_id_secret_key_encrypted
844b4401b3b070a88aab32af5c5c78328ffd3a97122ce18492ded51dcb6a3c99  ed25519_signing_cert
84c8309c3eab316f1e27b97f2e02aefa79aa5f6c77fa39ebcd33dd23109a926a  ed25519_signing_secret_key

When --newpass is specified with --keygen, Tor should:
a) only modify the master id key file accordingly, don't generate cert and medium term signing key (it currently changes them every time we modify the master id key);
b) delete the _encrypted version of the master id key when we decrypt and vice versa delete the plaintext version when we encrypt (it already does this, maybe it's enough this way?).

--master-key:
Doesn't work as expected. Tor thinks that it should have the name ed25519_master_id_secret_key - while this is true for when we look for it in $datadirectory, when we manually specify --master-key it could have the any name, like 'apple'.

Example:
I had my ed25519_master_id_secret_key in /root named 'apple' (/root/apple).

Did a tor --master-key /root/apple --keygen (in working directory /root). Tor generated a file called apple_public_key in /root and created the folder $HOME/.tor/keys where it stored ed25519_signing_cert and ed25519_signing_secret_key.

If I specify tor --master-key /root/apple --datadirectory /root --keygen it creates a file called apple_public_key in /root and a /root/keys folder where it saves ed25519_signing_cert and ed25519_signing_secret_key.

When --master-key is specified with --keygen, Tor should:
a) only generate cert and medium term signing key, valid for 30 days unless otherwise specified with --SigningKeyLifetime. Tor knows how to generate this one by itself when started as a relay. If it is simpler for symmetry to let Tor generate it as well, so be it, it's not something wrong, but we have to save it in the same location with the cert and medium term signing key;
b) don't care what's the file name, only care if it's a valid ed25519 key file and further do not apply the name as a prefix for the master id public key. If the master ID secret key file name is 'apple' and we decide to also generate ed25519_master_id_public_key and not skip it, do not name it apple_public_key, name it ed25519_master_id_public_key and save it in the same location with the cert and medium term signing key;

--master-key + --newpass
Doesn't work at all. Doesn't save anything when I try to encrypt a master key, and because it's angry with us it also deletes the plaintext version.

General:
Tor still uses $HOME/.tor folder if --datadirectory is not specified with --keygen. Can we make it use the working directory where we run the command as a default if nothing else is specified?

--out shouldn't have to be redundant with --datadirectory. --datadirectory requires a keys subfolder, while --out should not, --out should just save the files there.

comment:7 Changed 4 years ago by nickm

Status: needs_reviewneeds_revision

comment:8 Changed 4 years ago by nickm

Fixup pushed. Now --keygen --newpass no longer forces a new signing key, though one will be generated if the current signing key is missing or old. You don't need to copy it over if you don't want to.

Next step, figuring out master_key

comment:9 Changed 4 years ago by nickm

Okay, there was a huge bug where after creating an encrypted signing key, we'd unlink the unencrypted one... even if they had the same filename! That would delete the one we made.

I bet master-key will work a little better now.

Perhaps instead of --out a --keydir would be okay? I'm hoping to get something that is more generally applicable.

comment:10 Changed 4 years ago by nickm

Status: needs_revisionneeds_review

I think given our timeframes, I'm not going to be able to add more to this one before the final cutoff. Let's review it as-is, since everything but --keydir should be in place, I think

comment:11 Changed 4 years ago by dgoulet

Status: needs_reviewneeds_revision

It's weirdly working for me and I'm not 100% sure if it's the intended behavior. Here is my scenario:

keys/$ ls
.  ..
keys/$ tor --keygen --master-key mykey
[...]
keys/$ ls
.  ..  ed25519_signing_cert  ed25519_signing_secret_key  mykey  mykey_public_key
keys/$ 

So good so far but then changing the password of mykey is not possible though. Maybe it's "upcoming feature", not sure. It was reported in the last s7r's comment but unclear if it was suppose to be fixed since tor is clearly trying to read "mykey".

keys/$ tor --newpass --keygen --master-key mykey
[...]
[warn] Unable to read mykey: Invalid argument

Second thing, if I use the default use case, tor --keygen, I get keys and a signing cert with its key. Then, right away, if I change the password with tor --keygen --newpass, I get a brand new signing cert/key. Again reported in s7r comment but comment:8 seems to indicate it's fixed?

Note also that those two new options without a manpage entry will not be very useful.

comment:12 Changed 4 years ago by s7r

Doesn't work as expected.

When --newpass is called it still generates new cert and medium term signing key regardless if it doesn't have any, or already has valid ones. I come back with my suggestion to make --newpass care only about the master secret identity key, and don't check or try to generate cert and medium term signing key. We are under the assumption that an user calling --newpass wants to change passphrase/encrypt/decrypt. If cert and medium term signing key are also needed, another --keygen without --newpass will do it.

I get the same error for --master-key. It doesn't read it second time and reports "Invalid argument". Also, if I call --master-key ~/something it will also look for / generate something_public_key.
$ tor --master-key ~12/test --keygen

Sep 09 08:01:47.690 [warn] No key found in /root/12/test or /root/12/test_public_key.
Sep 09 08:01:47.690 [warn] Missing identity key

comment:13 Changed 4 years ago by nickm

Resolution: disable --master-key for now and leave --newpass as it is and review this patch as it stands. --newpass as implemented today is better than nothing!

comment:14 Changed 4 years ago by nickm

Status: needs_revisionneeds_review

comment:15 Changed 4 years ago by dgoulet

This lgtm except for the changes file that is lying about --master-key :).

comment:16 Changed 4 years ago by nickm

opened #17127 to cover the part of this that I'm not enabling yet. Merging what there is with a fixed changes file. Thanks!

comment:17 Changed 4 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.