[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Passphrase not working for ssh_pki_export_privkey_file


On Fri, 06 Feb 2015 09:35:56 +0100
Andreas Schneider <asn@xxxxxxxxxxxxxx> wrote:

> On Thursday 05 February 2015 16:21:03 Julian Lunz wrote:
> > I had time to dig a bit further.
> > 
> > The function pki_private_key_to_pem in src/pki_crypto.c +554
> > is missing a cipher in case of passphrase != NULL.
> > 
> > ssh-keygen uses AES-128-CBC therefore this is used in the attached
> > patch.
> > 
> > Is the mailing list the preferred way for patches or better via
> > Redmine?
> >
> 
> Thank you very much for your contribution. It is fine to send patches
> to the mailing list. 
> 
> However to add the patch to the libssh repository we also need a test
> for it!
> 
> Please take a look at tests/unittests/torture_pki.c and add a test.
> You get the unit tests if you install cmocka [1] and run 'cmake
> -DUNIT_TESTING=ON ..'
> 
> 
> Cheers,
> 
> 
> 	-- andreas
> 
> 
> 
> [1] http://cmocka.org
> 
> 

Sure, please find attached a patch series with tests included.

# 0001-Fix-pki_private_key_to_pem-by-adding-cipher.patch
Contains the fix which adds cipher to ssh_string pki_private_key_to_pem.


# 0002-tests-Add-encrypted-keys-export-for-rsa-dsa-ecdsa.patch
Contains updated test for torture_pki_write_privkey_[rsa,dsa,ecdsa]
+ added private keys for ecdsa.


I changed the existing calls to ssh_pki_export_privkey_file which had
"" as a passhrase since NULL != "".

If PEM_write_bio_RSAPrivateKey has no cipher set, as it was before 0001,
keys are always written in unencrypted form.

The documentation for ssh_pki_export_privkey_file says:

passphrase The passphrase to use to encrypt the key with or
            NULL. An empty string means no passphrase.

If this should behave like that a check for an empty string in addition
to check for NULL is needed.

Julian
From 05ea65c9f554b85b5dce6cbd176d0f8ab9910ab8 Mon Sep 17 00:00:00 2001
From: Julian Lunz <git@xxxxxxxx>
Date: Thu, 5 Feb 2015 16:12:28 +0100
Subject: [PATCH 1/3] Fix pki_private_key_to_pem by adding cipher

---
 src/pki_crypto.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/pki_crypto.c b/src/pki_crypto.c
index b53bba2..77fa14c 100644
--- a/src/pki_crypto.c
+++ b/src/pki_crypto.c
@@ -586,7 +586,7 @@ ssh_string pki_private_key_to_pem(const ssh_key key,
             } else {
                 rc = PEM_write_bio_DSAPrivateKey(mem,
                                                  key->dsa,
-                                                 NULL, /* cipher */
+                                                 EVP_aes_128_cbc(), /* cipher */
                                                  NULL, /* kstr */
                                                  0, /* klen */
                                                  NULL, /* auth_fn */
@@ -611,7 +611,7 @@ ssh_string pki_private_key_to_pem(const ssh_key key,
             } else {
                 rc = PEM_write_bio_RSAPrivateKey(mem,
                                                  key->rsa,
-                                                 NULL, /* cipher */
+                                                 EVP_aes_128_cbc(), /* cipher */
                                                  NULL, /* kstr */
                                                  0, /* klen */
                                                  NULL, /* auth_fn */
@@ -636,7 +636,7 @@ ssh_string pki_private_key_to_pem(const ssh_key key,
             } else {
                 rc = PEM_write_bio_ECPrivateKey(mem,
                                                 key->ecdsa,
-                                                NULL, /* cipher */
+                                                EVP_aes_128_cbc(), /* cipher */
                                                 NULL, /* kstr */
                                                 0, /* klen */
                                                 NULL, /* auth_fn */
-- 
2.2.2

From b949b7c61f2f020cfe7cad6e9917f3c3c24cdfec Mon Sep 17 00:00:00 2001
From: Julian Lunz <git@xxxxxxxx>
Date: Sun, 8 Feb 2015 10:31:05 +0100
Subject: [PATCH 2/3] tests: Add encrypted keys export for rsa, dsa, ecdsa

---
 tests/torture.c               |  39 +++++++++++-
 tests/unittests/torture_pki.c | 138 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 171 insertions(+), 6 deletions(-)

diff --git a/tests/torture.c b/tests/torture.c
index c139f6a..d886638 100644
--- a/tests/torture.c
+++ b/tests/torture.c
@@ -169,6 +169,16 @@ static const char torture_ecdsa256_testkey_pub[] =
         "YAAABBBMfvbnfPEORlrS3fsjLWGmqQvOYPtmS6e1bRRwNBGzR6gVEMaIfiJPPTJa+w"
         "FMXBT3fpAqPjROsqv5jUHC+xOok= aris@kalix86\n";
 
+static const char torture_ecdsa256_testkey_pp[] =
+        "-----BEGIN EC PRIVATE KEY-----\n"
+        "Proc-Type: 4,ENCRYPTED\n"
+        "DEK-Info: AES-128-CBC,319046776365CE0CA7347018E955A5E7\n"
+        "\n"
+        "x7p+v1SWYSdbWVsmnteZp7TgQnCFICGT/2s6rYhLlanKUxNaQ97INwaDvVzcCv1P\n"
+        "qD6wpAMn4Ohiq2hLeYnGCGX75Qu3rixmn+c3NdIV11vyRgM7FkwmhPKuj41fHWqM\n"
+        "0ZCPhjAaYAUK6p5KlDwvZBLiCmQKXAAZEHBudVfNhTw=\n"
+        "-----END EC PRIVATE KEY-----\n";
+
 static const char torture_ecdsa384_testkey[] =
         "-----BEGIN EC PRIVATE KEY-----\n"
         "MIGkAgEBBDBY8jEa5DtRy4AVeTWhPJ/TK257behiC3uafEi6YA2oHORibqX55EDN\n"
@@ -183,6 +193,17 @@ static const char torture_ecdsa384_testkey_pub[] =
         "0sB3/DunsMkt3O0nRtijJPhXcHdmpH1HIarqZgKOReVzlhtgeO54FunSh41eqxcc0B"
         "ZBmg== aris@kalix86";
 
+static const char torture_ecdsa384_testkey_pp[] =
+        "-----BEGIN EC PRIVATE KEY-----\n"
+        "Proc-Type: 4,ENCRYPTED\n"
+        "DEK-Info: AES-128-CBC,B46262FD09E7221B1C74F9657EC7C206\n"
+        "\n"
+        "gM/6WGF4YiaVdUeuMOFl0vWGl1K3nT8OpSKncpFoXRhuoumaF5Ky/4Ko+1B42JHZ\n"
+        "i0tyzHy56TVpwlD2UitGdpqm7vhODVvgkqo+Af6hT/2jWUYDMXrr+hNYU9t98Yzx\n"
+        "PwLB0/QI9lyidXjBLY5j3r8mpWZ1V6ssZ1FM23+4uvSJ7xyxKpz4OQAidWAOa2Ru\n"
+        "WvtfZL6RmPfYrJ9ptJpM7XDsDD3pNDhDCc5OGRn69kI=\n"
+        "-----END EC PRIVATE KEY-----\n";
+
 static const char torture_ecdsa521_testkey[] =
         "-----BEGIN EC PRIVATE KEY-----\n"
         "MIHbAgEBBEG83nSJ2SLoiBvEku1JteQKWx/Xt6THksgC7rrIaTUmNzk+60f0sCCm\n"
@@ -198,6 +219,18 @@ static const char torture_ecdsa521_testkey_pub[] =
         "V262vIC+AE3fXUJ7sJ/CkFIdk/8/gQEY1jyoXB3Bsee16VwhJGsMzGGh1FJ0XXhRJj"
         "UbG18qbH9JiSgE1N4fIM0zJG68fAyUxRxCI1wUobOOB7EmFZd18g== aris@kalix86";
 
+static const char torture_ecdsa521_testkey_pp[] =
+        "-----BEGIN EC PRIVATE KEY-----\n"
+        "Proc-Type: 4,ENCRYPTED\n"
+        "DEK-Info: AES-128-CBC,7D4323D6296E2B75E5A4FC6C98D8B04D\n"
+        "\n"
+        "XRh464kGFiM7D+8szR5TAe87p1JQfDkzJ37MuD/QwXWuPXq6fODWwibwS9S3yQBL\n"
+        "KEfzZQMBWEZJPabJF8YO2GA80/Djh80N4R+iOQqqdblP+QxrTiHNPlqBknbqYgDQ\n"
+        "yepy1YE5gSKLbm/zBCosJlw9kfUfZTqijlDJSiJBJotVcm5NdewZEKb/0Pv3a8hu\n"
+        "BjDnZNuQd9TqELR96UwMIHwd2Rg8rLIkHTf2PPpWUcOF0Ss3lvc/t8fP6HxoX+QX\n"
+        "u7i0NdbQ1weZfqNCHSY0gT2MLNGJsE4xLbj7X/LbsYY=\n"
+        "-----END EC PRIVATE KEY-----\n";
+
 static const char torture_ed25519_testkey[]=
         "-----BEGIN OPENSSH PRIVATE KEY-----\n"
         "b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW\n"
@@ -599,14 +632,14 @@ static const char *torture_get_testkey_internal(enum ssh_keytypes_e type,
             if (pubkey) {
                 return torture_ecdsa521_testkey_pub;
             } else if (with_passphrase) {
-		return NULL;
+                return torture_ecdsa521_testkey_pp;
             }
             return torture_ecdsa521_testkey;
         } else if (bits == 384) {
             if (pubkey) {
                 return torture_ecdsa384_testkey_pub;
             } else if (with_passphrase){
-		return NULL;
+                return torture_ecdsa384_testkey_pp;
             }
             return torture_ecdsa384_testkey;
         }
@@ -614,7 +647,7 @@ static const char *torture_get_testkey_internal(enum ssh_keytypes_e type,
         if (pubkey) {
             return torture_ecdsa256_testkey_pub;
         } else if (with_passphrase){
-		return NULL;
+                return torture_ecdsa256_testkey_pp;
         }
         return torture_ecdsa256_testkey;
     case SSH_KEYTYPE_ED25519:
diff --git a/tests/unittests/torture_pki.c b/tests/unittests/torture_pki.c
index 027322a..bc93a3e 100644
--- a/tests/unittests/torture_pki.c
+++ b/tests/unittests/torture_pki.c
@@ -6,9 +6,13 @@
 #include <fcntl.h>
 
 #define LIBSSH_RSA_TESTKEY "libssh_testkey.id_rsa"
+#define LIBSSH_RSA_TESTKEY_PASSPHRASE "libssh_testkey.id_rsa_passphrase"
 #define LIBSSH_DSA_TESTKEY "libssh_testkey.id_dsa"
+#define LIBSSH_DSA_TESTKEY_PASSPHRASE "libssh_testkey.id_dsa_passphrase"
 #define LIBSSH_ECDSA_TESTKEY "libssh_testkey.id_ecdsa"
+#define LIBSSH_ECDSA_TESTKEY_PASSPHRASE "libssh_testkey.id_ecdsa_passphrase"
 #define LIBSSH_ED25519_TESTKEY "libssh_testkey.id_ed25519"
+#define LIBSSH_ED25519_TESTKEY_PASSPHRASE "libssh_testkey.id_ed25519_passphrase"
 
 const unsigned char HASH[] = "12345678901234567890";
 
@@ -17,11 +21,14 @@ static void setup_rsa_key(void **state) {
 
     unlink(LIBSSH_RSA_TESTKEY);
     unlink(LIBSSH_RSA_TESTKEY ".pub");
+    unlink(LIBSSH_RSA_TESTKEY_PASSPHRASE);
 
     torture_write_file(LIBSSH_RSA_TESTKEY,
                        torture_get_testkey(SSH_KEYTYPE_RSA, 0, 0));
     torture_write_file(LIBSSH_RSA_TESTKEY ".pub",
                        torture_get_testkey_pub(SSH_KEYTYPE_RSA, 0));
+    torture_write_file(LIBSSH_RSA_TESTKEY_PASSPHRASE,
+                       torture_get_testkey(SSH_KEYTYPE_RSA, 0, 1));
 }
 
 static void setup_dsa_key(void **state) {
@@ -29,11 +36,14 @@ static void setup_dsa_key(void **state) {
 
     unlink(LIBSSH_DSA_TESTKEY);
     unlink(LIBSSH_DSA_TESTKEY ".pub");
+    unlink(LIBSSH_DSA_TESTKEY_PASSPHRASE);
 
     torture_write_file(LIBSSH_DSA_TESTKEY,
                        torture_get_testkey(SSH_KEYTYPE_DSS, 0, 0));
     torture_write_file(LIBSSH_DSA_TESTKEY ".pub",
                        torture_get_testkey_pub(SSH_KEYTYPE_DSS, 0));
+    torture_write_file(LIBSSH_DSA_TESTKEY_PASSPHRASE,
+                       torture_get_testkey(SSH_KEYTYPE_DSS, 0, 1));
 }
 
 #ifdef HAVE_OPENSSL_ECC
@@ -43,11 +53,14 @@ static void setup_ecdsa_key(void **state, int ecdsa_bits) {
 
     unlink(LIBSSH_ECDSA_TESTKEY);
     unlink(LIBSSH_ECDSA_TESTKEY ".pub");
+    unlink(LIBSSH_ECDSA_TESTKEY_PASSPHRASE);
 
     torture_write_file(LIBSSH_ECDSA_TESTKEY,
                        torture_get_testkey(SSH_KEYTYPE_ECDSA, ecdsa_bits, 0));
     torture_write_file(LIBSSH_ECDSA_TESTKEY ".pub",
                        torture_get_testkey_pub(SSH_KEYTYPE_ECDSA, ecdsa_bits));
+    torture_write_file(LIBSSH_ECDSA_TESTKEY_PASSPHRASE,
+                       torture_get_testkey(SSH_KEYTYPE_ECDSA, ecdsa_bits, 1));
 }
 
 static void setup_ecdsa_key_521(void **state) {
@@ -88,12 +101,16 @@ static void teardown(void **state) {
 
     unlink(LIBSSH_DSA_TESTKEY);
     unlink(LIBSSH_DSA_TESTKEY ".pub");
+    unlink(LIBSSH_DSA_TESTKEY_PASSPHRASE);
 
     unlink(LIBSSH_RSA_TESTKEY);
     unlink(LIBSSH_RSA_TESTKEY ".pub");
+    unlink(LIBSSH_RSA_TESTKEY_PASSPHRASE);
+
 
     unlink(LIBSSH_ECDSA_TESTKEY);
     unlink(LIBSSH_ECDSA_TESTKEY ".pub");
+    unlink(LIBSSH_ECDSA_TESTKEY_PASSPHRASE);
 }
 
 static char *read_file(const char *filename) {
@@ -1228,10 +1245,11 @@ static void torture_pki_write_privkey_rsa(void **state)
     unlink(LIBSSH_RSA_TESTKEY);
 
     rc = ssh_pki_export_privkey_file(origkey,
-                                     "",
+                                     NULL,
                                      NULL,
                                      NULL,
                                      LIBSSH_RSA_TESTKEY);
+
     assert_true(rc == 0);
 
     rc = ssh_pki_import_privkey_file(LIBSSH_RSA_TESTKEY,
@@ -1244,6 +1262,44 @@ static void torture_pki_write_privkey_rsa(void **state)
     rc = ssh_key_cmp(origkey, privkey, SSH_KEY_CMP_PRIVATE);
     assert_true(rc == 0);
 
+    /* Test with passphrase */
+    ssh_key_free(origkey);
+    ssh_key_free(privkey);
+    rc = ssh_pki_import_privkey_file(LIBSSH_RSA_TESTKEY_PASSPHRASE,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     &origkey);
+    assert_true(rc == 0);
+
+    unlink(LIBSSH_RSA_TESTKEY_PASSPHRASE);
+    rc = ssh_pki_export_privkey_file(origkey,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     LIBSSH_RSA_TESTKEY_PASSPHRASE);
+
+    assert_true(rc == 0);
+
+    /* Test if import fails with invalid passwort */
+    /* libcrypto asks for a passphrase if it is NULL so use invalid phrase */
+    rc = ssh_pki_import_privkey_file(LIBSSH_RSA_TESTKEY_PASSPHRASE,
+                                     "wrong passphrase !!",
+                                     NULL,
+                                     NULL,
+                                     &privkey);
+    /* expect fail */
+    assert_true(rc == SSH_ERROR);
+
+    rc = ssh_pki_import_privkey_file(LIBSSH_RSA_TESTKEY_PASSPHRASE,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     &privkey);
+
+    rc = ssh_key_cmp(origkey, privkey, SSH_KEY_CMP_PRIVATE);
+    assert_true(rc == 0);
+
     ssh_key_free(origkey);
     ssh_key_free(privkey);
 }
@@ -1268,7 +1324,7 @@ static void torture_pki_write_privkey_dsa(void **state)
     unlink(LIBSSH_DSA_TESTKEY);
 
     rc = ssh_pki_export_privkey_file(origkey,
-                                     "",
+                                     NULL,
                                      NULL,
                                      NULL,
                                      LIBSSH_DSA_TESTKEY);
@@ -1284,6 +1340,44 @@ static void torture_pki_write_privkey_dsa(void **state)
     rc = ssh_key_cmp(origkey, privkey, SSH_KEY_CMP_PRIVATE);
     assert_true(rc == 0);
 
+    /* Test with passphrase */
+    ssh_key_free(origkey);
+    ssh_key_free(privkey);
+    rc = ssh_pki_import_privkey_file(LIBSSH_DSA_TESTKEY_PASSPHRASE,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     &origkey);
+    assert_true(rc == 0);
+
+    unlink(LIBSSH_DSA_TESTKEY_PASSPHRASE);
+    rc = ssh_pki_export_privkey_file(origkey,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     LIBSSH_DSA_TESTKEY_PASSPHRASE);
+
+    assert_true(rc == 0);
+
+    /* Test if import fails with invalid passwort */
+    /* libcrypto asks for a passphrase if it is NULL so use invalid phrase */
+    rc = ssh_pki_import_privkey_file(LIBSSH_DSA_TESTKEY_PASSPHRASE,
+                                     "wrong passphrase !!",
+                                     NULL,
+                                     NULL,
+                                     &privkey);
+    /* expect fail */
+    assert_true(rc == SSH_ERROR);
+
+    rc = ssh_pki_import_privkey_file(LIBSSH_DSA_TESTKEY_PASSPHRASE,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     &privkey);
+
+    rc = ssh_key_cmp(origkey, privkey, SSH_KEY_CMP_PRIVATE);
+    assert_true(rc == 0);
+
     ssh_key_free(origkey);
     ssh_key_free(privkey);
 }
@@ -1309,7 +1403,7 @@ static void torture_pki_write_privkey_ecdsa(void **state)
     unlink(LIBSSH_ECDSA_TESTKEY);
 
     rc = ssh_pki_export_privkey_file(origkey,
-                                     "",
+                                     NULL,
                                      NULL,
                                      NULL,
                                      LIBSSH_ECDSA_TESTKEY);
@@ -1325,6 +1419,44 @@ static void torture_pki_write_privkey_ecdsa(void **state)
     rc = ssh_key_cmp(origkey, privkey, SSH_KEY_CMP_PRIVATE);
     assert_true(rc == 0);
 
+    /* Test with passphrase */
+    ssh_key_free(origkey);
+    ssh_key_free(privkey);
+    rc = ssh_pki_import_privkey_file(LIBSSH_ECDSA_TESTKEY_PASSPHRASE,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     &origkey);
+    assert_true(rc == 0);
+
+    unlink(LIBSSH_ECDSA_TESTKEY_PASSPHRASE);
+    rc = ssh_pki_export_privkey_file(origkey,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     LIBSSH_ECDSA_TESTKEY_PASSPHRASE);
+
+    assert_true(rc == 0);
+
+    /* Test if import fails with invalid passwort */
+    /* libcrypto asks for a passphrase if it is NULL so use invalid phrase */
+    rc = ssh_pki_import_privkey_file(LIBSSH_ECDSA_TESTKEY_PASSPHRASE,
+                                     "wrong passphrase !!",
+                                     NULL,
+                                     NULL,
+                                     &privkey);
+    /* expect fail */
+    assert_true(rc == SSH_ERROR);
+
+    rc = ssh_pki_import_privkey_file(LIBSSH_ECDSA_TESTKEY_PASSPHRASE,
+                                     torture_get_testkey_passphrase(),
+                                     NULL,
+                                     NULL,
+                                     &privkey);
+
+    rc = ssh_key_cmp(origkey, privkey, SSH_KEY_CMP_PRIVATE);
+    assert_true(rc == 0);
+
     ssh_key_free(origkey);
     ssh_key_free(privkey);
 }
-- 
2.2.2

From 3af5b7e3193eb3007162b0ac2fbd1e3cc8fbda9d Mon Sep 17 00:00:00 2001
From: Julian Lunz <git@xxxxxxxx>
Date: Sun, 8 Feb 2015 09:56:36 +0100
Subject: [PATCH 3/3] Fix typo

---
 src/pki_crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pki_crypto.c b/src/pki_crypto.c
index 77fa14c..5b2c53c 100644
--- a/src/pki_crypto.c
+++ b/src/pki_crypto.c
@@ -542,7 +542,7 @@ int pki_key_compare(const ssh_key k1,
             }
 #endif
         case SSH_KEYTYPE_ED25519:
-            /* ed25519 keys handled globaly */
+            /* ed25519 keys handled globally */
         case SSH_KEYTYPE_UNKNOWN:
         default:
             return 1;
-- 
2.2.2


References:
Re: Passphrase not working for ssh_pki_export_privkey_fileJulian Lunz <git@xxxxxxxx>
Re: Passphrase not working for ssh_pki_export_privkey_fileAndreas Schneider <asn@xxxxxxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org