Allow using password-protected private keys

[fghanmi]

Issue #, if available:
#796

Description of changes:

• Added support for signing TUF repositories with password-protected private keys.
• Implemented functionality to handle password protection during key management.
• add unit tests

If the password is invalid, the code will throw the following error:
Failed to sign repository: Unable to parse keypair: Unrecognized private key format

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[fghanmi]

Hello @bcressey, @jpculp,
Could you please take a look when you have a moment?
Thank you in advance!

[jpculp]

Hello @fghanmi! Thank you for this. I'll share this with the rest of the team, but while we take a look at it would you be able to do a rustfmt? The actions workflow will fail as is.

[jmt-lab]

So we have in the near past worked to remove openssl from the dependencies of tough and would prefer to keep it that way. Is there a way to adjust this implementation to rely on rustls instead?

[fghanmi]

I believe that reading password encrypted private keys is not yet supported by rustls(pemfile) as per this issue. They've suggested to use pkcs8 instead.
So, I've done the same and removed openssl.

[fghanmi]

Hello @fghanmi! Thank you for this. I'll share this with the rest of the team, but while we take a look at it would you be able to do a rustfmt? The actions workflow will fail as is.

Hello @jpculp,
I tried it and it failed. But, I tried it on develop branch and it fails as well. I am not sure if I am using a rust version that is incompatible with this project.

[jpculp]

I tried it and it failed.

I was able to run cargo fmt at the top level of the repo, but we can also push the adjustment if you'd rather we take care of it.

[fghanmi]

I tried it and it failed.

I was able to run cargo fmt at the top level of the repo, but we can also push the adjustment if you'd rather we take care of it.

@jpculp
Yes, cargo fmt is working fine without any errors on this PR branch.
I've updated the PR
Thanks!

[jpculp]

Thank you for the formatting fix! We are still evaluating this, but I left a couple of small changes in the code in the meantime.

[fghanmi]

Hello @jpculp ,
any news regarding this PR, please ?

[jpculp]

@fghanmi, apologies for the delays. It's still under review, but the gears are in motion. Something that might help is if you are able to include a small unit test for handling a password protected key. The same is true for adding a small test for SHA512 hashes in #814. Thank you for your patience in all of this.

[jpculp]

Thank you for the quick follow-up! It looks like if we bump pkcs8 to 0.10 the duplicate dependencies will drop in the check-licenses action, but looks like it requires a tweak in the code. I suspect it requires something close to:

    let encrypted_private_key_document = pkcs8::EncryptedPrivateKeyInfo::from_der(pem.contents())?;
    let decrypted_private_key_document =
        encrypted_private_key_document.decrypt(password.as_bytes())?;
    let decrypted_key_bytes: Vec<u8> = decrypted_private_key_document.as_bytes().to_vec();

[jpculp]

I think we're getting close here. @fghanmi, would you be able to include example output in the description if the password is incorrect?

[fghanmi]

Failed to sign repository: Unable to parse keypair: Unrecognized private key format

@jpculp , I hope I understood you correctly, I've added an error example to the PR description.

[jku]

I'm not a maintainer, so I'll leave this as a drive by comment, not a review:

In my opinion "--password" is an anti pattern in any cli: It practically guarantees users are going to make a mistake and the password is then available in shell history and /proc/.