Adopt sslib keygen interface encryption changes #1191
Conversation
|
Would it be better to have an option to decide how to do this? If you want the caller to not encrypt the file on disk, we should make them pass in some scary flag, I think. |
I agree. Storing keys unencrypted by default is a sort of a backwards-incompatible change, and not a great one. |
|
@JustinCappos, @trishankatdatadog, I agree with you but there also seems to be a consensus that a default prompt in an interface function is a bad idea (see secure-systems-lab/securesystemslib#288 (comment) for the related discussion). |
Makes sense. If that is the case, then perhaps we need to make a backwards-incompatible change, and split the function into two: one for generating unencrypted keys, and one for encrypted keys (no prompt, because the passphrase must be explicitly specified by the caller). This should follow the principle of least surprise to users. What do you think? |
I think Trishank's suggestion is a good way to do what I was trying to suggest. I more meant that we should force the behavior to specify some option (which sort of means there is no default). I wasn't really meaning to keep the current behavior by default. |
|
@trishankatdatadog and @JustinCappos, I posted a recap of the discussion at secure-systems-lab/securesystemslib#288 (comment). Let's discuss there what makes sense most. |
lukpueh
force-pushed
the
adopt-sslib-interface-changes
branch
from
bc470b8
to
ed9c4a7
Compare
|
The issue of non-default encryption has been resolved in #288 and adopted here. May I kindly ask for a re-review? |
|
Note that tests fail, because the changes in sslib have not yet been released. The |
| @@ -74,8 +74,14 @@ | |||
|
|
|||
| from securesystemslib.interface import ( | |||
| generate_and_write_rsa_keypair, | |||
| generate_and_write_rsa_keypair_with_prompt, | |||
There was a problem hiding this comment.
Do we actually use these new functions here, or are they just being imported w/o being used?
There was a problem hiding this comment.
The latter is correct. They are just added to the TUF namespace (see comments above), because a lot of docs suggest to do from repository_tool import * to access them.
I started this in #919 (see 7306446 for details), in order to remove many oneliner function wrappers that served the same purpose but added cruft to the code base and duplicated the maintenance burden above all in regards to docstrings.
A better but also more invasive fix would be to remove these functions from the tuf namespace altogether, and update the docs to directly use securesystemslib akin to what we now do in in-toto (see in-toto/in-toto#402, in-toto/in-toto#408 and https://in-toto-lukpueh.readthedocs.io/en/latest/api.html#key-utilities).
lukpueh
force-pushed
the
adopt-sslib-interface-changes
branch
from
ed9c4a7
to
24bc182
Compare
lukpueh
force-pushed
the
adopt-sslib-interface-changes
branch
from
3e440aa
to
4e43d38
Compare
secure-systems-lab/securesystemslib#288 changes the key generation interface functions in such a way that it is clear if a call opens a blocking prompt, or writes the key unencrypted. To do this two functions are added per key type: - `generate_and_write_*_keypair_with_prompt` - `generate_and_write_unencrypted_*_keypair` The default `generate_and_write_*_keypair` function now only allows encrypted keys and only using a passed password. This respects the principle of secure defaults and least surprise. sslib#288 furthermore adds a protected `_generate_and_write_*_keypair`, which is not exposed publicly because it does not encrypt by default, but is more flexible and thus convenient e.g. to consume all arguments from a key generation command line tool such as 'repo.py'. This commit adds the new public functions to the tuf namespace and adopts their usage accordingly. NOTE regarding repo.py: This commit does not fix any problematic password behavior of 'repo.py' like default passwords, etc. (see theupdateframework#881). It only adopts the sslib#288 changes to maintain the current behvior, plus removing one glaringly obsolete password prompt. NOTE regarding key import: The securesystemslib private key import functions were also changed to no longer auto-prompt for decryption passwords , TUF, however, only exposes custom wrappers (see repository_lib) that do auto-prompt. sslib#288 changes to the prompt texts are nevertheless propagated to tuf and reflected in this commit. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh
force-pushed
the
adopt-sslib-interface-changes
branch
from
4e43d38
to
dc20fdb
Compare
|
Closes #1204 |
Blocks on secure-systems-lab/securesystemslib#288 + release
Fixes # -
Description of the changes being introduced by the pull request:
secure-systems-lab/securesystemslib#288 changes the key generation interface functions to no longer auto-prompt for an encryption password if no password is passed, in order to not suprise the caller with a blocking prompt.
The downside of this change is that the keys are stored in plain text per default, which may be mitigated by recommending encryption in the docs.This PR updates related TUF documentation.
NOTE: The securesystemslib private key import functions do not auto-prompt for decryption passwords either, however TUF only exposes custom wrappers (see repository_lib) that do auto-prompt.
See commit message for details.
Please verify and check that the pull request fulfills the following
requirements: