From: Andre Noll Date: Sun, 9 Apr 2017 11:47:15 +0000 (+0200) Subject: Merge branch 'refs/heads/t/rm_asn' X-Git-Tag: v0.6.0~14 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=3b3b41a8819bcb9c10772057de03878188ae6f8f;hp=c1cec595405b31188d589062cc1117afe8daa64f Merge branch 'refs/heads/t/rm_asn' 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(). --- diff --git a/NEWS.md b/NEWS.md index d441db42..771e12df 100644 --- 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 8116fb6e..29a1c955 100644 --- 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 9be7a23e..6f3befdb 100644 --- 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); /** diff --git a/crypt_backend.h b/crypt_backend.h index f9a69d94..fccdd2ef 100644 --- a/crypt_backend.h +++ b/crypt_backend.h @@ -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); diff --git a/crypt_common.c b/crypt_common.c index a05572df..1fd8189c 100644 --- a/crypt_common.c +++ b/crypt_common.c @@ -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 dde0122a..f2b15a4e 100644 --- a/error.h +++ b/error.h @@ -186,7 +186,6 @@ 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"), \ diff --git a/gcrypt.c b/gcrypt.c index 7c19aeb0..1ae7cfb0 100644 --- 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; } diff --git a/user_list.c b/user_list.c index 9c751244..23fe086f 100644 --- a/user_list.c +++ b/user_list.c @@ -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