Merge branch 'refs/heads/t/rm_asn'
authorAndre Noll <maan@tuebingen.mpg.de>
Sun, 9 Apr 2017 11:47:15 +0000 (13:47 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Sun, 9 Apr 2017 11:50:24 +0000 (13:50 +0200)
A few cleanups, and one patch which removes an obsolete feature,
getting rid of an open-coded ASN parser.

Cooking for three months.

* refs/heads/t/rm_asn:
  Remove unused E_PUBLIC_KEY.
  crypt.c: Combine load_key() and get_private_key().
  crypto: Remove support for ASN public keys.
  crypto: Simplify asymetric key handling.
  crypto: Rename check_key_file() -> check_private_key_file().
  gcrypt.c: Always initialize result pointer in get_private_key().

NEWS.md
crypt.c
crypt.h
crypt_backend.h
crypt_common.c
error.h
gcrypt.c
user_list.c

diff --git a/NEWS.md b/NEWS.md
index d441db4..771e12d 100644 (file)
--- a/NEWS.md
+++ b/NEWS.md
@@ -7,6 +7,10 @@ NEWS
 - Support for Mac OS X has been removed.
 - On Linux systems, glibc-2.17 or newer is required to build the
   source tree.
+- Support for RSA public keys in ASN format (as generated by openssl
+  genrsa) has been removed. These keys have been deprecated since
+  2011, so users should have long switched to keys generated with
+  ssh-keygen(1).
 
 Downloads:
 [tarball](./releases/paraslash-git.tar.bz2),
diff --git a/crypt.c b/crypt.c
index 8116fb6..29a1c95 100644 (file)
--- a/crypt.c
+++ b/crypt.c
@@ -61,41 +61,24 @@ void init_random_seed_or_die(void)
        srandom(seed);
 }
 
-static EVP_PKEY *load_key(const char *file, int private)
+static int get_private_key(const char *path, RSA **rsa)
 {
-       BIO *key;
-       EVP_PKEY *pkey = NULL;
-       int ret = check_key_file(file, private);
-
-       if (ret < 0) {
-               PARA_ERROR_LOG("%s\n", para_strerror(-ret));
-               return NULL;
-       }
-       key = BIO_new(BIO_s_file());
-       if (!key)
-               return NULL;
-       if (BIO_read_filename(key, file) > 0) {
-               if (private == LOAD_PRIVATE_KEY)
-                       pkey = PEM_read_bio_PrivateKey(key, NULL, NULL, NULL);
-               else
-                       pkey = PEM_read_bio_PUBKEY(key, NULL, NULL, NULL);
-       }
-       BIO_free(key);
-       return pkey;
-}
-
-static int get_openssl_key(const char *key_file, RSA **rsa, int private)
-{
-       EVP_PKEY *key = load_key(key_file, private);
-
-       if (!key)
-               return (private == LOAD_PRIVATE_KEY)? -E_PRIVATE_KEY
-                       : -E_PUBLIC_KEY;
-       *rsa = EVP_PKEY_get1_RSA(key);
-       EVP_PKEY_free(key);
-       if (!*rsa)
-               return -E_RSA;
-       return RSA_size(*rsa);
+       EVP_PKEY *pkey;
+       BIO *bio = BIO_new(BIO_s_file());
+
+       *rsa = NULL;
+       if (!bio)
+               return -E_PRIVATE_KEY;
+       if (BIO_read_filename(bio, path) <= 0)
+               goto bio_free;
+       pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
+       if (!pkey)
+               goto bio_free;
+       *rsa = EVP_PKEY_get1_RSA(pkey);
+       EVP_PKEY_free(pkey);
+bio_free:
+       BIO_free(bio);
+       return *rsa? RSA_size(*rsa) : -E_PRIVATE_KEY;
 }
 
 /*
@@ -160,8 +143,7 @@ fail:
        return ret;
 }
 
-int get_asymmetric_key(const char *key_file, int private,
-               struct asymmetric_key **result)
+int get_public_key(const char *key_file, struct asymmetric_key **result)
 {
        struct asymmetric_key *key = NULL;
        void *map = NULL;
@@ -171,21 +153,13 @@ int get_asymmetric_key(const char *key_file, int private,
        char *cp;
 
        key = para_malloc(sizeof(*key));
-       if (private) {
-               ret = get_openssl_key(key_file, &key->rsa, LOAD_PRIVATE_KEY);
-               goto out;
-       }
        ret = mmap_full_file(key_file, O_RDONLY, &map, &map_size, NULL);
        if (ret < 0)
                goto out;
        ret = is_ssh_rsa_key(map, map_size);
        if (!ret) {
-               ret = para_munmap(map, map_size);
-               map = NULL;
-               if (ret < 0)
-                       goto out;
-               ret = get_openssl_key(key_file, &key->rsa, LOAD_PUBLIC_KEY);
-               goto out;
+               para_munmap(map, map_size);
+               return -E_SSH_PARSE;
        }
        cp = map + ret;
        encoded_size = map_size - ret;
@@ -215,7 +189,7 @@ out:
        return ret;
 }
 
-void free_asymmetric_key(struct asymmetric_key *key)
+void free_public_key(struct asymmetric_key *key)
 {
        if (!key)
                return;
@@ -229,11 +203,17 @@ int priv_decrypt(const char *key_file, unsigned char *outbuf,
        struct asymmetric_key *priv;
        int ret;
 
+       ret = check_private_key_file(key_file);
+       if (ret < 0)
+               return ret;
        if (inlen < 0)
                return -E_RSA;
-       ret = get_asymmetric_key(key_file, LOAD_PRIVATE_KEY, &priv);
-       if (ret < 0)
+       priv = para_malloc(sizeof(*priv));
+       ret = get_private_key(key_file, &priv->rsa);
+       if (ret < 0) {
+               free(priv);
                return ret;
+       }
        /*
         * RSA is vulnerable to timing attacks. Generate a random blinding
         * factor to protect against this kind of attack.
@@ -247,7 +227,8 @@ int priv_decrypt(const char *key_file, unsigned char *outbuf,
        if (ret <= 0)
                ret = -E_DECRYPT;
 out:
-       free_asymmetric_key(priv);
+       RSA_free(priv->rsa);
+       free(priv);
        return ret;
 }
 
diff --git a/crypt.h b/crypt.h
index 9be7a23..6f3befd 100644 (file)
--- a/crypt.h
+++ b/crypt.h
@@ -51,22 +51,21 @@ int priv_decrypt(const char *key_file, unsigned char *outbuf,
  * Read an asymmetric key from a file.
  *
  * \param key_file The file containing the key.
- * \param private if non-zero, read the private key, otherwise the public key.
  * \param result The key structure is returned here.
  *
  * \return The size of the key on success, negative on errors.
  */
-int get_asymmetric_key(const char *key_file, int private,
-               struct asymmetric_key **result);
+int get_public_key(const char *key_file, struct asymmetric_key **result);
 
 /**
- * Deallocate an asymmetric key structure.
+ * Deallocate a public key.
  *
  * \param key Pointer to the key structure to free.
  *
- * This must be called for any key obtained by get_asymmetric_key().
+ * This should be called for keys obtained by get_public_key() if the key is no
+ * longer needed.
  */
-void free_asymmetric_key(struct asymmetric_key *key);
+void free_public_key(struct asymmetric_key *key);
 
 
 /**
index f9a69d9..fccdd2e 100644 (file)
@@ -14,4 +14,4 @@
 size_t is_ssh_rsa_key(char *data, size_t size);
 uint32_t read_ssh_u32(const void *vp);
 int check_ssh_key_header(const unsigned char *blob, int blen);
-int check_key_file(const char *file, bool private_key);
+int check_private_key_file(const char *file);
index a05572d..1fd8189 100644 (file)
@@ -103,25 +103,22 @@ int check_ssh_key_header(const unsigned char *blob, int blen)
 }
 
 /**
- * Check existence and permissions of a key file.
+ * Check existence and permissions of a private key file.
  *
  * \param file The path of the key file.
- * \param private_key Whether this is a private key.
  *
- * This checks whether the file exists. If it is a private key, we additionally
- * check that the permissions are restrictive enough. It is considered an error
- * if we own the file and it is readable for others.
+ * This checks whether the file exists and its permissions are restrictive
+ * enough. It is considered an error if we own the file and it is readable for
+ * others.
  *
  * \return Standard.
  */
-int check_key_file(const char *file, bool private_key)
+int check_private_key_file(const char *file)
 {
        struct stat st;
 
        if (stat(file, &st) != 0)
                return -ERRNO_TO_PARA_ERROR(errno);
-       if (!private_key)
-               return 0;
        if ((st.st_uid == getuid()) && (st.st_mode & 077) != 0)
                return -E_KEY_PERM;
        return 1;
diff --git a/error.h b/error.h
index dde0122..f2b15a4 100644 (file)
--- a/error.h
+++ b/error.h
        PARA_ERROR(PLAY_SYNTAX, "para_play: syntax error"), \
        PARA_ERROR(PREBUFFER_SUCCESS, "prebuffering complete"), \
        PARA_ERROR(PRIVATE_KEY, "can not read private key"), \
-       PARA_ERROR(PUBLIC_KEY, "can not read public key"), \
        PARA_ERROR(QUEUE, "packet queue overrun"), \
        PARA_ERROR(READ_PATTERN, "did not read expected pattern"), \
        PARA_ERROR(RECV_EOF, "end of file"), \
index 7c19aeb..1ae7cfb 100644 (file)
--- a/gcrypt.c
+++ b/gcrypt.c
@@ -301,64 +301,6 @@ static inline int get_long_form_num_length_bytes(unsigned char c)
        return c & 0x7f;
 }
 
-static int find_pubkey_bignum_offset(const unsigned char *data, int len)
-{
-       const unsigned char *p = data, *end = data + len;
-
-       /* the whole thing starts with one sequence */
-       if (*p != ASN1_TYPE_SEQUENCE)
-               return -E_ASN1_PARSE;
-       p++;
-       if (p >= end)
-               return -E_ASN1_PARSE;
-       if (is_short_form(*p))
-               p++;
-       else
-               p += 1 + get_long_form_num_length_bytes(*p);
-       if (p >= end)
-               return -E_ASN1_PARSE;
-       /* another sequence containing the object id, skip it */
-       if (*p != ASN1_TYPE_SEQUENCE)
-               return -E_ASN1_PARSE;
-       p++;
-       if (p >= end)
-               return -E_ASN1_PARSE;
-       if (!is_short_form(*p))
-               return -E_ASN1_PARSE;
-       p += 1 + get_short_form_length(*p);
-       if (p >= end)
-               return -E_ASN1_PARSE;
-       /* all numbers are wrapped in a bit string object that follows */
-       if (*p != ASN1_TYPE_BIT_STRING)
-               return -E_ASN1_PARSE;
-       p++;
-       if (p >= end)
-               return -E_ASN1_PARSE;
-       if (is_short_form(*p))
-               p++;
-       else
-               p += 1 + get_long_form_num_length_bytes(*p);
-       p++; /* skip number of unused bits in the bit string */
-       if (p >= end)
-               return -E_ASN1_PARSE;
-
-       /* next, we have a sequence of two integers (n and e) */
-       if (*p != ASN1_TYPE_SEQUENCE)
-               return -E_ASN1_PARSE;
-       p++;
-       if (p >= end)
-               return -E_ASN1_PARSE;
-       if (is_short_form(*p))
-               p++;
-       else
-               p += 1 + get_long_form_num_length_bytes(*p);
-       if (p >= end)
-               return -E_ASN1_PARSE;
-       if (*p != ASN1_TYPE_INTEGER)
-               return -E_ASN1_PARSE;
-       return p - data;
-}
-
 /*
  * Returns: Number of bytes scanned. This may differ from the value returned via
  * bn_bytes because the latter does not include the ASN.1 prefix and a leading
@@ -460,6 +402,7 @@ static int get_private_key(const char *key_file, struct asymmetric_key **result)
        gcry_sexp_t sexp;
        struct asymmetric_key *key;
 
+       *result = NULL;
        ret = decode_key(key_file, PRIVATE_KEY_HEADER, PRIVATE_KEY_FOOTER,
                &blob);
        if (ret < 0)
@@ -538,65 +481,6 @@ free_blob:
        return ret;
 }
 
-/** Public keys start with this header. */
-#define PUBLIC_KEY_HEADER "-----BEGIN PUBLIC KEY-----"
-/** Public keys end with this footer. */
-#define PUBLIC_KEY_FOOTER "-----END PUBLIC KEY-----"
-
-static int get_asn_public_key(const char *key_file, struct asymmetric_key **result)
-{
-       gcry_mpi_t n = NULL, e = NULL;
-       unsigned char *blob, *cp, *end;
-       int blob_size, ret, n_size;
-       gcry_error_t gret;
-       size_t erroff;
-       gcry_sexp_t sexp;
-       struct asymmetric_key *key;
-
-       ret = decode_key(key_file, PUBLIC_KEY_HEADER, PUBLIC_KEY_FOOTER,
-               &blob);
-       if (ret < 0)
-               return ret;
-       blob_size = ret;
-       end = blob + blob_size;
-       ret = find_pubkey_bignum_offset(blob, blob_size);
-       if (ret < 0)
-               goto free_blob;
-       PARA_DEBUG_LOG("decoding public RSA params at offset %d\n", ret);
-       cp = blob + ret;
-
-       ret = read_bignum(cp, end, &n, &n_size);
-       if (ret < 0)
-               goto free_blob;
-       cp += ret;
-
-       ret = read_bignum(cp, end, &e, NULL);
-       if (ret < 0)
-               goto release_n;
-
-       gret = gcry_sexp_build(&sexp, &erroff, RSA_PUBKEY_SEXP, n, e);
-       if (gret) {
-               PARA_ERROR_LOG("offset %zu: %s\n", erroff,
-                       gcry_strerror(gcry_err_code(gret)));
-               ret = -E_SEXP_BUILD;
-               goto release_e;
-       }
-       key = para_malloc(sizeof(*key));
-       key->sexp = sexp;
-       key->num_bytes = n_size;
-       *result = key;
-       ret = n_size;
-       PARA_INFO_LOG("successfully read %d bit asn public key\n", n_size * 8);
-
-release_e:
-       gcry_mpi_release(e);
-release_n:
-       gcry_mpi_release(n);
-free_blob:
-       free(blob);
-       return ret;
-}
-
 static int get_ssh_public_key(unsigned char *data, int size, gcry_sexp_t *result)
 {
        int ret;
@@ -658,8 +542,7 @@ free_blob:
        return ret;
 }
 
-int get_asymmetric_key(const char *key_file, int private,
-               struct asymmetric_key **result)
+int get_public_key(const char *key_file, struct asymmetric_key **result)
 {
        int ret, ret2;
        void *map;
@@ -668,17 +551,13 @@ int get_asymmetric_key(const char *key_file, int private,
        gcry_sexp_t sexp;
        struct asymmetric_key *key;
 
-       if (private)
-               return get_private_key(key_file, result);
        ret = mmap_full_file(key_file, O_RDONLY, &map, &map_size, NULL);
        if (ret < 0)
                return ret;
        ret = is_ssh_rsa_key(map, map_size);
        if (!ret) {
-               ret = para_munmap(map, map_size);
-               if (ret < 0)
-                       return ret;
-               return get_asn_public_key(key_file, result);
+               para_munmap(map, map_size);
+               return -E_SSH_PARSE;
        }
        start = map + ret;
        end = map + map_size;
@@ -699,7 +578,7 @@ unmap:
        return ret;
 }
 
-void free_asymmetric_key(struct asymmetric_key *key)
+void free_public_key(struct asymmetric_key *key)
 {
        if (!key)
                return;
@@ -778,7 +657,7 @@ int priv_decrypt(const char *key_file, unsigned char *outbuf,
        gcry_sexp_t in, out, priv_key;
        size_t nbytes;
 
-       ret = check_key_file(key_file, true);
+       ret = check_private_key_file(key_file);
        if (ret < 0)
                return ret;
        PARA_INFO_LOG("decrypting %d byte input\n", inlen);
@@ -831,7 +710,8 @@ in_mpi_release:
 key_release:
        gcry_sexp_release(priv_key);
 free_key:
-       free_asymmetric_key(priv);
+       gcry_sexp_release(priv->sexp);
+       free(priv);
        return ret;
 }
 
index 9c75124..23fe086 100644 (file)
@@ -48,7 +48,7 @@ static void populate_user_list(char *user_list_file)
                if (strcmp(w, "user"))
                        continue;
                PARA_DEBUG_LOG("found entry for user %s\n", n);
-               ret = get_asymmetric_key(k, LOAD_PUBLIC_KEY, &pubkey);
+               ret = get_public_key(k, &pubkey);
                if (ret < 0) {
                        PARA_NOTICE_LOG("skipping entry for user %s: %s\n", n,
                                para_strerror(-ret));
@@ -63,7 +63,7 @@ static void populate_user_list(char *user_list_file)
                if (ret <= CHALLENGE_SIZE + 2 * SESSION_KEY_LEN + 41) {
                        PARA_WARNING_LOG("public key %s too short (%d)\n",
                                k, ret);
-                       free_asymmetric_key(pubkey);
+                       free_public_key(pubkey);
                        continue;
                }
                u = para_malloc(sizeof(*u));
@@ -114,7 +114,7 @@ void init_user_list(char *user_list_file)
                list_for_each_entry_safe(u, tmp, &user_list, node) {
                        list_del(&u->node);
                        free(u->name);
-                       free_asymmetric_key(u->pubkey);
+                       free_public_key(u->pubkey);
                        free(u);
                }
        } else