Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

Specifying a blank password upon new-cert still encrypts the .key file #44

Open
abourget opened this issue Jan 25, 2015 · 9 comments · May be fixed by #60
Open

Specifying a blank password upon new-cert still encrypts the .key file #44

abourget opened this issue Jan 25, 2015 · 9 comments · May be fixed by #60

Comments

@abourget
Copy link

I'd expect a blank passphrase to not encrypt the .key file. Is that a bug or a feature ?

@abourget abourget changed the title Specifying a blank password upon new-cert still encrypts the file Specifying a blank password upon new-cert still encrypts the .key file Jan 25, 2015
@yichengq
Copy link
Contributor

empty passphrase is still a valid passphrase.
If you wanna export the unencrpted key file, you can use export --insecure

@abourget
Copy link
Author

the message shown says Enter passphrase (empty for no passphrase):.. I'd expect no passphrase to mean there is no passphrase, like not encrypted.. that's what openssl would do.

In this case, when you hit enter for an empty passphrase, the key still gets encrypted. With what passphrase ? I don't know, but I know you can't decrypt it with openssl, because it will ask for a minimum of 4 characters. I don't know if it's a constraint by the encryption scheme or only by the openssl UI, but it makes the generated certificates useless in this case.

@abourget
Copy link
Author

Here's a small patch that implements what I mean:

diff --git a/cmd/new_cert.go b/cmd/new_cert.go
index 125458b..eacaeca 100644
--- a/cmd/new_cert.go
+++ b/cmd/new_cert.go
@@ -70,7 +70,13 @@ func newCertAction(c *cli.Context) {
        if err = depot.PutCertificateSigningRequest(d, name, csr); err != nil {
                fmt.Fprintln(os.Stderr, "Save certificate request error:", err)
        }
-       if err = depot.PutEncryptedPrivateKeyHost(d, name, key, passphrase); err != nil {
-               fmt.Fprintln(os.Stderr, "Save key error:", err)
+       if len(passphrase) == 0 {
+               if err = depot.PutPrivateKeyHost(d, name, key); err != nil {
+                       fmt.Fprintln(os.Stderr, "Save key error:", err)
+               }
+       } else {
+               if err = depot.PutEncryptedPrivateKeyHost(d, name, key, passphrase); err != nil {
+                       fmt.Fprintln(os.Stderr, "Save key error:", err)
+               }
        }
 }

Would that be useful ?

@yichengq
Copy link
Contributor

@abourget It can be encrypted with empty passphrase. I would prefer to fix the print message.
You can still export unencrypted key easily using export --insecure

@abourget
Copy link
Author

abourget commented Feb 1, 2015

ok. I see I can export with export --insecure, which outputs a .tar file.. that's a first inconvenience for me.. as I wouldn't want to deal with the .tar .. only picking up a .key file.

For consistency with openssl, I would have liked to have the same behavior here too.. I was surprised (in a bad way) when I saw the key was encrypted with a blank passphrase. Why create bad surprises ?

Anyway, I've made my points. I'll close this issue until someone wants to pick it up. Thanks

@abourget abourget closed this as completed Feb 1, 2015
@yichengq
Copy link
Contributor

yichengq commented Feb 1, 2015

After rethinking about it, i think it should keep the same convention as openssh one. I will improve it later.

@yichengq yichengq reopened this Feb 1, 2015
@abourget
Copy link
Author

what about my patch up here ? I stumbled upon this one once again :)

@abourget
Copy link
Author

I can make it a PR if you want.

@abourget abourget linked a pull request May 30, 2015 that will close this issue
@bkleef
Copy link

bkleef commented Aug 8, 2015

Can we please merge this. This would be very useful because Alpine Linux 3.2 x64 doesn't get the tar -e option and IMHO it's just useless to tar something to extract it directly afterwards.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants