[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: (Client side) RSA signatures with SHA2 (RFC 8332 and RFC 8308)
[Thread Prev] | [Thread Next]
- Subject: Re: (Client side) RSA signatures with SHA2 (RFC 8332 and RFC 8308)
- From: Jakub Jelen <jjelen@xxxxxxxxxx>
- Reply-to: libssh@xxxxxxxxxx
- Date: Fri, 31 Aug 2018 10:49:03 +0200
- To: libssh@xxxxxxxxxx, Andreas Schneider <asn@xxxxxxxxxxxxxx>
Hello, based on our discussion yesterday, I rewrote the patches not to define new key types, and hide the signature hash inside of signature structure. This makes the changes a bit bigger, but they will hopefully be more acceptable. The current patch set is available here and in attachment. https://gitlab.com/jjelen/libssh-mirror/tree/rsa-sha2-v2 Regards, Jakub On Wed, 2018-06-27 at 15:23 +0200, Jakub Jelen wrote: > Hello, > The attached are patches to implement extension negotiation for SSH > (RFC 8308) and a new RSA signatures with SHA2 (RFC 8332), which are > negotiated using this mechanism and already used for few years in > OpenSSH. > > While debugging the initial implementation, I noticed failures in > torture_connect_double test, which showed up the disconnect was not > properly resetting all the session structures and values to be ready > for the new connection. This worked before, because just the connect > did not use the negotiated crypto before [first patch]. > > Then there are few more minor issues that I hit during the work > throughout the code. > > From the RFC 8332 (SHA2 extension), Section 2 [1], it is not > completely > clear whether the the new algorithms are supposed to reference only > the > signatures in the code, or they can reference also the public/private > keys themselves. The distinction is somehow fuzzy also in the OpenSSH > code if I remember well, so this is one thing to consideration. > > Another thing to consider is possibility to configure/enable/disable > these algorithms if needed. This is done in OpenSSH with > PubkeyAcceptedKeyTypes option, which is not implemented in libssh at > this moment, but should not be too hard to handle (I can do that, if > you would consider it make sense). > > The host keys are tested in a new test torture_hostkey, which tries > all > the supported mechanisms and verifies the server key signature is > properly validated. > > The second part is about using this extension for public key > authentication, which adds the signature counterparts for the verify > added in the previous part. This requires addition to the PKI api to > specify the algorithm to use in addition to the key itself. The > attached patch provides the verification that signatures provided by > this interface are verifiable. > > The last part is about ability to request the same signatures from > compatible ssh-agent using the flags described in the ssh-agent > protocol. > > What is missing is a server side implementation: The server > recognizing > the extension, sending its supported algorithms, sending the host > keys > signatures using the new algorithms and verifying the new signatures > during authentication (should work, but needs to be verified). I did > not go into that, because of the lack of the test suite for the > server. > Are there any plans for introducing tests for server? > > I tested against current OpenSSH 7.7p1 in Fedora and with all of the > openssl, libgcrypt and mbedtls and all the tests are passing. > > [1] https://tools.ietf.org/html/rfc8332#section-2 > -- Jakub Jelen Software Engineer Security Technologies Red Hat, Inc.
From 687fc064d41b1f7221a920e220301f1e06336fd0 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Wed, 29 Aug 2018 20:15:47 +0200 Subject: [PATCH 01/22] pkd: Generate host keys in old format This is required to work against OpenSSH 7.8, which is now writing keys in new openssh format by default Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- tests/pkd/pkd_keyutil.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/pkd/pkd_keyutil.c b/tests/pkd/pkd_keyutil.c index 14856d3c..f0e0c710 100644 --- a/tests/pkd/pkd_keyutil.c +++ b/tests/pkd/pkd_keyutil.c @@ -21,7 +21,7 @@ void setup_rsa_key() { int rc = 0; if (access(LIBSSH_RSA_TESTKEY, F_OK) != 0) { - rc = system_checked(OPENSSH_KEYGEN " -t rsa -q -N \"\" -f " + rc = system_checked(OPENSSH_KEYGEN " -m PEM -t rsa -q -N \"\" -f " LIBSSH_RSA_TESTKEY); } assert_int_equal(rc, 0); @@ -40,7 +40,7 @@ void setup_ed25519_key() { void setup_dsa_key() { int rc = 0; if (access(LIBSSH_DSA_TESTKEY, F_OK) != 0) { - rc = system_checked(OPENSSH_KEYGEN " -t dsa -q -N \"\" -f " + rc = system_checked(OPENSSH_KEYGEN " -m PEM -t dsa -q -N \"\" -f " LIBSSH_DSA_TESTKEY); } assert_int_equal(rc, 0); @@ -51,17 +51,17 @@ void setup_ecdsa_keys() { int rc = 0; if (access(LIBSSH_ECDSA_256_TESTKEY, F_OK) != 0) { - rc = system_checked(OPENSSH_KEYGEN " -t ecdsa -b 256 -q -N \"\" -f " + rc = system_checked(OPENSSH_KEYGEN " -m PEM -t ecdsa -b 256 -q -N \"\" -f " LIBSSH_ECDSA_256_TESTKEY); assert_int_equal(rc, 0); } if (access(LIBSSH_ECDSA_384_TESTKEY, F_OK) != 0) { - rc = system_checked(OPENSSH_KEYGEN " -t ecdsa -b 384 -q -N \"\" -f " + rc = system_checked(OPENSSH_KEYGEN " -m PEM -t ecdsa -b 384 -q -N \"\" -f " LIBSSH_ECDSA_384_TESTKEY); assert_int_equal(rc, 0); } if (access(LIBSSH_ECDSA_521_TESTKEY, F_OK) != 0) { - rc = system_checked(OPENSSH_KEYGEN " -t ecdsa -b 521 -q -N \"\" -f " + rc = system_checked(OPENSSH_KEYGEN " -m PEM -t ecdsa -b 521 -q -N \"\" -f " LIBSSH_ECDSA_521_TESTKEY); assert_int_equal(rc, 0); } -- 2.17.1 From f0e95acc78257539d8c87761b4fd63d302dd0627 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Wed, 29 Aug 2018 20:14:47 +0200 Subject: [PATCH 02/22] pkd: Produce more useful logs Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- tests/pkd/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pkd/CMakeLists.txt b/tests/pkd/CMakeLists.txt index 9abb25f3..b2a22f90 100644 --- a/tests/pkd/CMakeLists.txt +++ b/tests/pkd/CMakeLists.txt @@ -38,7 +38,7 @@ target_link_libraries(pkd_hello ${pkd_libs}) # specified with `-i` and may be helpful for chasing down bugs that # are not 100% reproducible. # -add_test(pkd_hello_i1 ${CMAKE_CURRENT_BINARY_DIR}/pkd_hello -i1 -w /tmp/pkd_socket_wrapper_XXXXXX) +add_test(pkd_hello_i1 ${CMAKE_CURRENT_BINARY_DIR}/pkd_hello -e -o -i1 -w /tmp/pkd_socket_wrapper_XXXXXX) # # Configure environment for cwrap socket wrapper. -- 2.17.1 From f980528ec74d84a02d5cf2b0b590893d46f21ae3 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Mon, 6 Aug 2018 11:47:32 +0200 Subject: [PATCH 03/22] kex: Signalize support for the extension negotiation in client (RFC 8308) Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- src/kex.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/kex.c b/src/kex.c index f7c7880d..229294b3 100644 --- a/src/kex.c +++ b/src/kex.c @@ -101,6 +101,9 @@ #define KEY_EXCHANGE CURVE25519 ECDH "diffie-hellman-group14-sha1,diffie-hellman-group1-sha1" #define KEX_METHODS_SIZE 10 +/* RFC 8308 */ +#define KEX_EXTENSION_CLIENT "ext-info-c" + /* NOTE: This is a fixed API and the index is defined by ssh_kex_types_e */ static const char *default_methods[] = { KEY_EXCHANGE, @@ -642,11 +645,15 @@ static char *ssh_client_select_hostkeys(ssh_session session) * @brief sets the key exchange parameters to be sent to the server, * in function of the options and available methods. */ -int ssh_set_client_kex(ssh_session session){ +int ssh_set_client_kex(ssh_session session) +{ struct ssh_kex_struct *client= &session->next_crypto->client_kex; const char *wanted; + char *kex = NULL; + char *kex_tmp = NULL; int ok; int i; + size_t kex_len, len; ok = ssh_get_random(client->cookie, 16, 0); if (!ok) { @@ -673,6 +680,23 @@ int ssh_set_client_kex(ssh_session session){ } } + /* Here we append ext-info-c to the list of kex algorithms */ + kex = client->methods[SSH_KEX]; + len = strlen(kex); + if (len + strlen(KEX_EXTENSION_CLIENT) + 2 < len) { + /* Overflow */ + return SSH_ERROR; + } + kex_len = len + strlen(KEX_EXTENSION_CLIENT) + 2; /* comma, NULL */ + kex_tmp = realloc(kex, kex_len); + if (kex_tmp == NULL) { + free(kex); + ssh_set_error_oom(session); + return SSH_ERROR; + } + snprintf(kex_tmp + len, kex_len - len, ",%s", KEX_EXTENSION_CLIENT); + client->methods[SSH_KEX] = kex_tmp; + return SSH_OK; } @@ -682,8 +706,16 @@ int ssh_set_client_kex(ssh_session session){ int ssh_kex_select_methods (ssh_session session){ struct ssh_kex_struct *server = &session->next_crypto->server_kex; struct ssh_kex_struct *client = &session->next_crypto->client_kex; + char *ext_start = NULL; int i; + /* Here we should drop the ext-info-c from the list so we avoid matching. + * it. We added it to the end, so we can just truncate the string here */ + ext_start = strstr(client->methods[SSH_KEX], ","KEX_EXTENSION_CLIENT); + if (ext_start != NULL) { + ext_start = '\0'; + } + for (i = 0; i < KEX_METHODS_SIZE; i++) { session->next_crypto->kex_methods[i]=ssh_find_matching(server->methods[i],client->methods[i]); if(session->next_crypto->kex_methods[i] == NULL && i < SSH_LANG_C_S){ -- 2.17.1 From 4adff84a0a74691d24deeec57cb8d00d54f32a6f Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Mon, 6 Aug 2018 11:51:26 +0200 Subject: [PATCH 04/22] client: Handle the MSG_EXT_INFO packet signalling supported extensions RFC 8308: The extension negotiation in Secure Shell (SSH) Protocol RFC 8332: Use of RSA Keys with SHA-256 and SHA-512 in the Secure Shell (SSH) Protocol Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/packet.h | 1 + include/libssh/session.h | 8 +++++++ include/libssh/ssh2.h | 1 + src/packet.c | 5 ++-- src/packet_cb.c | 52 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 2 deletions(-) diff --git a/include/libssh/packet.h b/include/libssh/packet.h index 1a9283d8..a3bcb9a8 100644 --- a/include/libssh/packet.h +++ b/include/libssh/packet.h @@ -51,6 +51,7 @@ SSH_PACKET_CALLBACK(ssh_packet_ignore_callback); SSH_PACKET_CALLBACK(ssh_packet_dh_reply); SSH_PACKET_CALLBACK(ssh_packet_newkeys); SSH_PACKET_CALLBACK(ssh_packet_service_accept); +SSH_PACKET_CALLBACK(ssh_packet_ext_info); #ifdef WITH_SERVER SSH_PACKET_CALLBACK(ssh_packet_kexdh_init); diff --git a/include/libssh/session.h b/include/libssh/session.h index 6cb79628..00717652 100644 --- a/include/libssh/session.h +++ b/include/libssh/session.h @@ -86,6 +86,11 @@ enum ssh_pending_call_e { #define SSH_OPT_FLAG_KBDINT_AUTH 0x4 #define SSH_OPT_FLAG_GSSAPI_AUTH 0x8 +/* extensions flags */ +/* server-sig-algs extension */ +#define SSH_EXT_SIG_RSA_SHA256 0x01 +#define SSH_EXT_SIG_RSA_SHA512 0x02 + /* members that are common to ssh_session and ssh_bind */ struct ssh_common_struct { struct error_struct error; @@ -114,6 +119,9 @@ struct ssh_session_struct { /* session flags (SSH_SESSION_FLAG_*) */ int flags; + /* Extensions negotiated using RFC 8308 */ + uint32_t extensions; + ssh_string banner; /* that's the issue banner from the server */ char *discon_msg; /* disconnect message from diff --git a/include/libssh/ssh2.h b/include/libssh/ssh2.h index 8b39b9a6..35214330 100644 --- a/include/libssh/ssh2.h +++ b/include/libssh/ssh2.h @@ -7,6 +7,7 @@ #define SSH2_MSG_DEBUG 4 #define SSH2_MSG_SERVICE_REQUEST 5 #define SSH2_MSG_SERVICE_ACCEPT 6 +#define SSH2_MSG_EXT_INFO 7 #define SSH2_MSG_KEXINIT 20 #define SSH2_MSG_NEWKEYS 21 diff --git a/src/packet.c b/src/packet.c index 16f96149..5b9252f5 100644 --- a/src/packet.c +++ b/src/packet.c @@ -59,8 +59,9 @@ static ssh_packet_callback default_packet_handlers[]= { NULL, #endif ssh_packet_service_accept, // SSH2_MSG_SERVICE_ACCEPT 6 - NULL, NULL, NULL, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, NULL, NULL, // 7-19 + ssh_packet_ext_info, // SSH2_MSG_EXT_INFO 7 + NULL, NULL, NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, NULL, NULL, // 8-19 ssh_packet_kexinit, // SSH2_MSG_KEXINIT 20 ssh_packet_newkeys, // SSH2_MSG_NEWKEYS 21 NULL, NULL, NULL, NULL, NULL, NULL, NULL, diff --git a/src/packet_cb.c b/src/packet_cb.c index ecf327ef..c3a1997f 100644 --- a/src/packet_cb.c +++ b/src/packet_cb.c @@ -270,3 +270,55 @@ SSH_PACKET_CALLBACK(ssh_packet_service_accept){ return SSH_PACKET_USED; } + +/** + * @internal + * @brief handles a SSH2_MSG_EXT_INFO packet defined in RFC 8308 + * + */ +SSH_PACKET_CALLBACK(ssh_packet_ext_info) +{ + int rc; + uint32_t nr_extensions = 0; + uint32_t i; + (void)type; + (void)user; + + SSH_LOG(SSH_LOG_PACKET, "Received SSH_MSG_EXT_INFO"); + + rc = ssh_buffer_get_u32(packet, &nr_extensions); + if (rc == 0) { + SSH_LOG(SSH_LOG_PACKET, "Failed to read number of extensions"); + return SSH_PACKET_USED; + } + nr_extensions = ntohl(nr_extensions); + SSH_LOG(SSH_LOG_PACKET, "Follows %u extensions", nr_extensions); + + for (i = 0; i < nr_extensions; i++) { + char *name = NULL; + char *value = NULL; + int cmp; + + rc = ssh_buffer_unpack(packet, "ss", &name, &value); + if (rc != SSH_OK) { + SSH_LOG(SSH_LOG_PACKET, "Error reading extension name-value pair"); + return SSH_PACKET_USED; + } + + cmp = strcmp(name, "server-sig-algs"); + if (cmp == 0) { + /* TODO check for NULL bytes */ + SSH_LOG(SSH_LOG_PACKET, "Extension: %s=<%s>", name, value); + if (ssh_match_group(value, "rsa-sha2-512")) { + session->extensions |= SSH_EXT_SIG_RSA_SHA512; + } + if (ssh_match_group(value, "rsa-sha2-256")) { + session->extensions |= SSH_EXT_SIG_RSA_SHA256; + } + } + free(name); + free(value); + } + + return SSH_PACKET_USED; +} -- 2.17.1 From db4420638c18afab9b5ceebc6a7695dc0b763053 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 12:17:29 +0200 Subject: [PATCH 05/22] pki: Support RSA verification using different hash algorithms This changes the private API by adding one more argument to function pki_signature_from_blob() Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/pki.h | 1 + include/libssh/pki_priv.h | 5 +- include/libssh/wrapper.h | 7 ++ src/pki.c | 108 ++++++++++++++++++++++++-- src/pki_crypto.c | 30 ++++++- src/pki_gcrypt.c | 29 ++++++- src/pki_mbedcrypto.c | 31 +++++++- tests/unittests/torture_pki_ed25519.c | 4 +- 8 files changed, 196 insertions(+), 19 deletions(-) diff --git a/include/libssh/pki.h b/include/libssh/pki.h index 4a4ce612..558680d7 100644 --- a/include/libssh/pki.h +++ b/include/libssh/pki.h @@ -69,6 +69,7 @@ struct ssh_key_struct { struct ssh_signature_struct { enum ssh_keytypes_e type; + enum ssh_digest_e hash_type; const char *type_c; #ifdef HAVE_LIBGCRYPT gcry_sexp_t dsa_sig; diff --git a/include/libssh/pki_priv.h b/include/libssh/pki_priv.h index af041504..b10e3b95 100644 --- a/include/libssh/pki_priv.h +++ b/include/libssh/pki_priv.h @@ -45,6 +45,8 @@ int bcrypt_pbkdf(const char *pass, int pki_key_ecdsa_nid_from_name(const char *name); const char *pki_key_ecdsa_nid_to_name(int nid); +const char *ssh_key_signature_to_char(enum ssh_keytypes_e type, + enum ssh_digest_e hash_type); /* SSH Key Functions */ ssh_key pki_key_dup(const ssh_key key, int demote); @@ -85,7 +87,8 @@ ssh_string pki_publickey_to_blob(const ssh_key key); ssh_string pki_signature_to_blob(const ssh_signature sign); ssh_signature pki_signature_from_blob(const ssh_key pubkey, const ssh_string sig_blob, - enum ssh_keytypes_e type); + enum ssh_keytypes_e type, + enum ssh_digest_e hash_type); int pki_signature_verify(ssh_session session, const ssh_signature sig, const ssh_key key, diff --git a/include/libssh/wrapper.h b/include/libssh/wrapper.h index 62a974fd..23d98afc 100644 --- a/include/libssh/wrapper.h +++ b/include/libssh/wrapper.h @@ -27,6 +27,13 @@ #include "libssh/libgcrypt.h" #include "libssh/libmbedcrypto.h" +enum ssh_digest_e { + SSH_DIGEST_AUTO=0, + SSH_DIGEST_SHA1=1, + SSH_DIGEST_SHA256, + SSH_DIGEST_SHA512 +}; + enum ssh_mac_e { SSH_MAC_SHA1=1, SSH_MAC_SHA256, diff --git a/src/pki.c b/src/pki.c index b8731730..f0557708 100644 --- a/src/pki.c +++ b/src/pki.c @@ -193,6 +193,36 @@ enum ssh_keytypes_e ssh_key_type(const ssh_key key){ return key->type; } +/** + * @brief Convert a signature type to a string. + * + * @param[in] type The algorithm type to convert. + * + * @return A string for the keytype or NULL if unknown. + */ +const char * +ssh_key_signature_to_char(enum ssh_keytypes_e type, + enum ssh_digest_e hash_type) +{ + if (type != SSH_KEYTYPE_RSA) { + return ssh_key_type_to_char(type); + } + + switch (hash_type) { + case SSH_DIGEST_SHA256: + return "rsa-sha2-256"; + case SSH_DIGEST_SHA512: + return "rsa-sha2-512"; + case SSH_DIGEST_SHA1: + return "ssh-rsa"; + default: + return NULL; + } + + /* We should never reach this */ + return NULL; +} + /** * @brief Convert a key type to a string. * @@ -223,6 +253,45 @@ const char *ssh_key_type_to_char(enum ssh_keytypes_e type) { return NULL; } +static enum ssh_digest_e ssh_key_hash_from_name(const char *name) +{ + if (name == NULL) { + /* TODO we should rather fail */ + return SSH_DIGEST_AUTO; + } + + if (strcmp(name, "ssh-rsa") == 0) { + return SSH_DIGEST_SHA1; + } else if (strcmp(name, "rsa-sha2-256") == 0) { + return SSH_DIGEST_SHA256; + } else if (strcmp(name, "rsa-sha2-512") == 0) { + return SSH_DIGEST_SHA512; + } + + /* we do not care for others now */ + return SSH_DIGEST_AUTO; +} +/** + * @brief Convert a ssh key algorithm name to a ssh key algorithm type. + * + * @param[in] name The name to convert. + * + * @return The enum ssh key algorithm type. + */ +static enum ssh_keytypes_e ssh_key_type_from_signature_name(const char *name) { + if (name == NULL) { + return SSH_KEYTYPE_UNKNOWN; + } + + if ((strcmp(name, "rsa-sha2-256") == 0) || + (strcmp(name, "rsa-sha2-512") == 0)) { + return SSH_KEYTYPE_RSA; + } + + /* Otherwise the key type matches the signature type */ + return ssh_key_type_from_name(name); +} + /** * @brief Convert a ssh key name to a ssh key type. * @@ -1507,8 +1576,10 @@ int ssh_pki_import_signature_blob(const ssh_string sig_blob, { ssh_signature sig; enum ssh_keytypes_e type; + enum ssh_digest_e hash_type; ssh_string str; ssh_buffer buf; + const char *alg = NULL; int rc; if (sig_blob == NULL || psig == NULL) { @@ -1534,7 +1605,9 @@ int ssh_pki_import_signature_blob(const ssh_string sig_blob, return SSH_ERROR; } - type = ssh_key_type_from_name(ssh_string_get_char(str)); + alg = ssh_string_get_char(str); + type = ssh_key_type_from_signature_name(alg); + hash_type = ssh_key_hash_from_name(alg); ssh_string_free(str); str = ssh_buffer_get_ssh_string(buf); @@ -1543,7 +1616,7 @@ int ssh_pki_import_signature_blob(const ssh_string sig_blob, return SSH_ERROR; } - sig = pki_signature_from_blob(pubkey, str, type); + sig = pki_signature_from_blob(pubkey, str, type, hash_type); ssh_string_free(str); if (sig == NULL) { return SSH_ERROR; @@ -1593,22 +1666,45 @@ int ssh_pki_signature_verify_blob(ssh_session session, } else if (key->type == SSH_KEYTYPE_ED25519) { rc = pki_signature_verify(session, sig, key, digest, dlen); } else { - unsigned char hash[SHA_DIGEST_LEN] = {0}; + unsigned char hash[SHA512_DIGEST_LEN] = {0}; + uint32_t hlen = 0; + + if (sig->type != SSH_KEYTYPE_RSA && sig->hash_type != SSH_DIGEST_AUTO) { + SSH_LOG(SSH_LOG_TRACE, "Only RSA keys support non-SHA1 hashes."); + return SSH_ERROR; + } - sha1(digest, dlen, hash); + switch (sig->hash_type) { + case SSH_DIGEST_SHA256: + sha256(digest, dlen, hash); + hlen = SHA256_DIGEST_LEN; + break; + case SSH_DIGEST_SHA512: + sha512(digest, dlen, hash); + hlen = SHA512_DIGEST_LEN; + break; + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + sha1(digest, dlen, hash); + hlen = SHA_DIGEST_LEN; + break; + default: + SSH_LOG(SSH_LOG_TRACE, "Unknown sig->hash_type: %d", sig->hash_type); + return SSH_ERROR; + } #ifdef DEBUG_CRYPTO ssh_print_hexa(key->type == SSH_KEYTYPE_DSS ? "Hash to be verified with DSA" : "Hash to be verified with RSA", hash, - SHA_DIGEST_LEN); + hlen); #endif rc = pki_signature_verify(session, sig, key, hash, - SHA_DIGEST_LEN); + hlen); } ssh_signature_free(sig); diff --git a/src/pki_crypto.c b/src/pki_crypto.c index 7494b162..b41dcb3f 100644 --- a/src/pki_crypto.c +++ b/src/pki_crypto.c @@ -1409,7 +1409,8 @@ errout: ssh_signature pki_signature_from_blob(const ssh_key pubkey, const ssh_string sig_blob, - enum ssh_keytypes_e type) + enum ssh_keytypes_e type, + enum ssh_digest_e hash_type) { ssh_signature sig; ssh_string r; @@ -1424,7 +1425,8 @@ ssh_signature pki_signature_from_blob(const ssh_key pubkey, } sig->type = type; - sig->type_c = ssh_key_type_to_char(type); + sig->hash_type = hash_type; + sig->type_c = ssh_key_signature_to_char(type, hash_type); len = ssh_string_len(sig_blob); @@ -1598,6 +1600,7 @@ int pki_signature_verify(ssh_session session, size_t hlen) { int rc; + int nid; switch(key->type) { case SSH_KEYTYPE_DSS: @@ -1615,13 +1618,33 @@ int pki_signature_verify(ssh_session session, break; case SSH_KEYTYPE_RSA: case SSH_KEYTYPE_RSA1: - rc = RSA_verify(NID_sha1, + switch (sig->hash_type) { + case SSH_DIGEST_AUTO: + case SSH_DIGEST_SHA1: + nid = NID_sha1; + break; + case SSH_DIGEST_SHA256: + nid = NID_sha256; + break; + case SSH_DIGEST_SHA512: + nid = NID_sha512; + break; + default: + SSH_LOG(SSH_LOG_TRACE, "Unknown hash type %d", sig->hash_type); + ssh_set_error(session, + SSH_FATAL, + "Unexpected hash type %d during RSA verify", + sig->hash_type); + return SSH_ERROR; + } + rc = RSA_verify(nid, hash, hlen, ssh_string_data(sig->rsa_sig), ssh_string_len(sig->rsa_sig), key->rsa); if (rc <= 0) { + SSH_LOG(SSH_LOG_TRACE, "RSA verify failed"); ssh_set_error(session, SSH_FATAL, "RSA error: %s", @@ -1655,6 +1678,7 @@ int pki_signature_verify(ssh_session session, #endif case SSH_KEYTYPE_UNKNOWN: default: + SSH_LOG(SSH_LOG_TRACE, "Unknown key type"); ssh_set_error(session, SSH_FATAL, "Unknown public key type"); return SSH_ERROR; } diff --git a/src/pki_gcrypt.c b/src/pki_gcrypt.c index 9f53321f..9a8e8622 100644 --- a/src/pki_gcrypt.c +++ b/src/pki_gcrypt.c @@ -1773,7 +1773,8 @@ ssh_string pki_signature_to_blob(const ssh_signature sig) ssh_signature pki_signature_from_blob(const ssh_key pubkey, const ssh_string sig_blob, - enum ssh_keytypes_e type) + enum ssh_keytypes_e type, + enum ssh_digest_e hash_type) { ssh_signature sig; gcry_error_t err; @@ -1787,6 +1788,8 @@ ssh_signature pki_signature_from_blob(const ssh_key pubkey, } sig->type = type; + sig->hash_type = hash_type; + sig->type_c = ssh_key_signature_to_char(type, hash_type); len = ssh_string_len(sig_blob); @@ -1952,6 +1955,7 @@ int pki_signature_verify(ssh_session session, size_t hlen) { unsigned char ghash[hlen + 1]; + const char *hash_type = NULL; gcry_sexp_t sexp; gcry_error_t err; @@ -1986,10 +1990,29 @@ int pki_signature_verify(ssh_session session, } break; case SSH_KEYTYPE_RSA: + switch (sig->hash_type) { + case SSH_DIGEST_SHA256: + hash_type = "sha256"; + break; + case SSH_DIGEST_SHA512: + hash_type = "sha512"; + break; + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + hash_type = "sha1"; + break; + default: + SSH_LOG(SSH_LOG_TRACE, "Unknown sig type %d", sig->hash_type); + ssh_set_error(session, + SSH_FATAL, + "Unexpected signature type %d during RSA verify", + sig->hash_type); + return SSH_ERROR; + } err = gcry_sexp_build(&sexp, NULL, - "(data(flags pkcs1)(hash sha1 %b))", - hlen, hash); + "(data(flags pkcs1)(hash %s %b))", + hash_type, hlen, hash); if (err) { ssh_set_error(session, SSH_FATAL, diff --git a/src/pki_mbedcrypto.c b/src/pki_mbedcrypto.c index 6dbaa56f..57465009 100644 --- a/src/pki_mbedcrypto.c +++ b/src/pki_mbedcrypto.c @@ -825,8 +825,10 @@ errout: ssh_signature_free(sig); return NULL; } -ssh_signature pki_signature_from_blob(const ssh_key pubkey, const ssh_string - sig_blob, enum ssh_keytypes_e type) +ssh_signature pki_signature_from_blob(const ssh_key pubkey, + const ssh_string sig_blob, + enum ssh_keytypes_e type, + enum ssh_digest_e hash_type) { ssh_signature sig = NULL; int rc; @@ -837,7 +839,8 @@ ssh_signature pki_signature_from_blob(const ssh_key pubkey, const ssh_string } sig->type = type; - sig->type_c = ssh_key_type_to_char(type); + sig->hash_type = hash_type; + sig->type_c = ssh_key_signature_to_char(type, hash_type); switch(type) { case SSH_KEYTYPE_RSA: @@ -930,10 +933,30 @@ int pki_signature_verify(ssh_session session, const ssh_signature sig, const ssh_key key, const unsigned char *hash, size_t hlen) { int rc; + mbedtls_md_type_t md = 0; switch (key->type) { case SSH_KEYTYPE_RSA: - rc = mbedtls_pk_verify(key->rsa, MBEDTLS_MD_SHA1, hash, hlen, + switch (sig->hash_type) { + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + md = MBEDTLS_MD_SHA1; + break; + case SSH_DIGEST_SHA256: + md = MBEDTLS_MD_SHA256; + break; + case SSH_DIGEST_SHA512: + md = MBEDTLS_MD_SHA512; + break; + default: + SSH_LOG(SSH_LOG_TRACE, "Unknown sig type %d", sig->hash_type); + ssh_set_error(session, + SSH_FATAL, + "Unexpected signature hash type %d during RSA verify", + sig->hash_type); + return SSH_ERROR; + } + rc = mbedtls_pk_verify(key->rsa, md, hash, hlen, ssh_string_data(sig->rsa_sig), ssh_string_len(sig->rsa_sig)); if (rc != 0) { diff --git a/tests/unittests/torture_pki_ed25519.c b/tests/unittests/torture_pki_ed25519.c index 5c6f8703..98c96a59 100644 --- a/tests/unittests/torture_pki_ed25519.c +++ b/tests/unittests/torture_pki_ed25519.c @@ -374,7 +374,7 @@ static void torture_pki_ed25519_verify(void **state){ assert_true(rc == SSH_OK); ssh_string_fill(blob, ref_signature, ED25519_SIG_LEN); - sig = pki_signature_from_blob(pubkey, blob, SSH_KEYTYPE_ED25519); + sig = pki_signature_from_blob(pubkey, blob, SSH_KEYTYPE_ED25519, SSH_DIGEST_AUTO); assert_true(sig != NULL); rc = pki_ed25519_verify(pubkey, sig, HASH, sizeof(HASH)); @@ -411,7 +411,7 @@ static void torture_pki_ed25519_verify_bad(void **state){ for (i=0; i < ED25519_SIG_LEN; ++i){ ssh_string_fill(blob, ref_signature, ED25519_SIG_LEN); ((uint8_t *)ssh_string_data(blob))[i] ^= 0xff; - sig = pki_signature_from_blob(pubkey, blob, SSH_KEYTYPE_ED25519); + sig = pki_signature_from_blob(pubkey, blob, SSH_KEYTYPE_ED25519, SSH_DIGEST_AUTO); assert_true(sig != NULL); rc = pki_ed25519_verify(pubkey, sig, HASH, sizeof(HASH)); -- 2.17.1 From 8584f8401140cf12b86c54ec9088849656272c42 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 15:04:13 +0200 Subject: [PATCH 06/22] kex: Offer SHA2 extension signature algorithms by default Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- src/kex.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/kex.c b/src/kex.c index 229294b3..effe8422 100644 --- a/src/kex.c +++ b/src/kex.c @@ -86,12 +86,12 @@ #ifdef HAVE_ECDH #define ECDH "ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521," -#define HOSTKEYS "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-rsa,ssh-dss" +#define HOSTKEYS "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-rsa,rsa-sha2-512,rsa-sha2-256,ssh-dss" #else #ifdef HAVE_DSA -#define HOSTKEYS "ssh-ed25519,ssh-rsa,ssh-dss" +#define HOSTKEYS "ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256,ssh-dss" #else -#define HOSTKEYS "ssh-ed25519,ssh-rsa" +#define HOSTKEYS "ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256" #endif #define ECDH "" #endif -- 2.17.1 From f5f7218b0e73e9423c72dabd2223595944ae215d Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 15:04:45 +0200 Subject: [PATCH 07/22] pki: RSA signatures with SHA2 hash algorithms (RFC 8332) * This change introduces a new API to request signature using one key and different hash algorithms. This is used only with RSA keys, that used to have SHA1 hardcoded, but the new algorithsms allow to use the SHA2 hashes, if the extension is negotiated. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/pki_priv.h | 9 ++-- src/pki.c | 87 +++++++++++++++++++++++++++++++++++---- src/pki_crypto.c | 66 +++++++++++++++++++++++++---- src/pki_gcrypt.c | 35 ++++++++++++++-- src/pki_mbedcrypto.c | 43 +++++++++++++++---- 5 files changed, 209 insertions(+), 31 deletions(-) diff --git a/include/libssh/pki_priv.h b/include/libssh/pki_priv.h index b10e3b95..38c7aa4b 100644 --- a/include/libssh/pki_priv.h +++ b/include/libssh/pki_priv.h @@ -96,9 +96,12 @@ int pki_signature_verify(ssh_session session, size_t hlen); /* SSH Signing Functions */ -ssh_signature pki_do_sign(const ssh_key privkey, - const unsigned char *hash, - size_t hlen); +#define pki_do_sign(key, hash, hlen) \ + pki_do_sign_hash(key, hash, hlen, SSH_DIGEST_AUTO) +ssh_signature pki_do_sign_hash(const ssh_key privkey, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type); ssh_signature pki_do_sign_sessionid(const ssh_key key, const unsigned char *hash, size_t hlen); diff --git a/src/pki.c b/src/pki.c index f0557708..5106aec5 100644 --- a/src/pki.c +++ b/src/pki.c @@ -271,6 +271,44 @@ static enum ssh_digest_e ssh_key_hash_from_name(const char *name) /* we do not care for others now */ return SSH_DIGEST_AUTO; } +/** + * @brief Convert a key type to a hash type. This is usually unambiguous + * for all the key types, unless the SHA2 extension (RFC 8332) is + * negotiated during key exchange. + * + * @param[in] session SSH Session. + * + * @param[in] type The type to convert. + * + * @return A hash type to be used. + */ +static enum ssh_keytypes_e ssh_key_type_to_hash(ssh_session session, + enum ssh_keytypes_e type) +{ + /* TODO this should also reflect supported key types specified in + * configuration (ssh_config PubkeyAcceptedKeyTypes) */ + switch (type) { + case SSH_KEYTYPE_RSA: + if (session->extensions & SSH_EXT_SIG_RSA_SHA512) { + return SSH_DIGEST_SHA512; + } + + if (session->extensions & SSH_EXT_SIG_RSA_SHA256) { + return SSH_DIGEST_SHA256; + } + + /* Default algorithm for RSA is SHA1 */ + return SSH_DIGEST_SHA1; + + default: + /* Other key types use the default value (not used) */ + return SSH_DIGEST_AUTO; + } + + /* We should never reach this */ + return SSH_DIGEST_AUTO; +} + /** * @brief Convert a ssh key algorithm name to a ssh key algorithm type. * @@ -1783,24 +1821,55 @@ ssh_string ssh_pki_do_sign(ssh_session session, ssh_buffer_get_len(buf)); ssh_buffer_free(buf); } else { - unsigned char hash[SHA_DIGEST_LEN] = {0}; - SHACTX ctx; + unsigned char hash[SHA512_DIGEST_LEN] = {0}; + uint32_t hlen = 0; + enum ssh_digest_e hash_type; + ssh_buffer buf; - ctx = sha1_init(); - if (ctx == NULL) { + buf = ssh_buffer_new(); + if (buf == NULL) { ssh_string_free(session_id); return NULL; } - sha1_update(ctx, session_id, ssh_string_len(session_id) + 4); - sha1_update(ctx, ssh_buffer_get(sigbuf), ssh_buffer_get_len(sigbuf)); - sha1_final(hash, ctx); + ssh_buffer_set_secure(buf); + rc = ssh_buffer_pack(buf, + "SP", + session_id, + ssh_buffer_get_len(sigbuf), ssh_buffer_get(sigbuf)); + if (rc != SSH_OK) { + ssh_string_free(session_id); + ssh_buffer_free(buf); + return NULL; + } + + hash_type = ssh_key_type_to_hash(session, privkey->type); + switch (hash_type) { + case SSH_DIGEST_SHA256: + sha256(ssh_buffer_get(buf), ssh_buffer_get_len(buf), hash); + hlen = SHA256_DIGEST_LEN; + break; + case SSH_DIGEST_SHA512: + sha512(ssh_buffer_get(buf), ssh_buffer_get_len(buf), hash); + hlen = SHA512_DIGEST_LEN; + break; + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + sha1(ssh_buffer_get(buf), ssh_buffer_get_len(buf), hash); + hlen = SHA_DIGEST_LEN; + break; + default: + SSH_LOG(SSH_LOG_TRACE, "Unknown hash algorithm for type: %d", + sig->type); + ssh_string_free(session_id); + return NULL; + } #ifdef DEBUG_CRYPTO - ssh_print_hexa("Hash being signed", hash, SHA_DIGEST_LEN); + ssh_print_hexa("Hash being signed", hash, hlen); #endif - sig = pki_do_sign(privkey, hash, SHA_DIGEST_LEN); + sig = pki_do_sign_hash(privkey, hash, hlen, hash_type); } ssh_string_free(session_id); if (sig == NULL) { diff --git a/src/pki_crypto.c b/src/pki_crypto.c index b41dcb3f..524cc0ff 100644 --- a/src/pki_crypto.c +++ b/src/pki_crypto.c @@ -1181,23 +1181,43 @@ fail: * * @param[in] privkey The private rsa key to use for signing. * + * @param[in] hash_type The hash algorithm to use. + * * @return A newly allocated rsa sig blob or NULL on error. */ -static ssh_string _RSA_do_sign(const unsigned char *digest, - int dlen, - RSA *privkey) +static ssh_string _RSA_do_sign_hash(const unsigned char *digest, + int dlen, + RSA *privkey, + enum ssh_digest_e hash_type) { ssh_string sig_blob; unsigned char *sig; unsigned int slen; int ok; + int nid = 0; + + switch (hash_type) { + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + nid = NID_sha1; + break; + case SSH_DIGEST_SHA256: + nid = NID_sha256; + break; + case SSH_DIGEST_SHA512: + nid = NID_sha512; + break; + default: + SSH_LOG(SSH_LOG_WARN, "Incomplatible hash type"); + return NULL; + } sig = malloc(RSA_size(privkey)); if (sig == NULL) { return NULL; } - ok = RSA_sign(NID_sha1, digest, dlen, sig, &slen, privkey); + ok = RSA_sign(nid, digest, dlen, sig, &slen, privkey); if (!ok) { SAFE_FREE(sig); return NULL; @@ -1216,6 +1236,26 @@ static ssh_string _RSA_do_sign(const unsigned char *digest, return sig_blob; } +/** + * @internal + * + * @brief Compute a digital signature. + * + * @param[in] digest The message digest. + * + * @param[in] dlen The length of the digest. + * + * @param[in] privkey The private rsa key to use for signing. + * + * @return A newly allocated rsa sig blob or NULL on error. + */ +static ssh_string _RSA_do_sign(const unsigned char *digest, + int dlen, + RSA *privkey) +{ + return _RSA_do_sign_hash(digest, dlen, privkey, SSH_DIGEST_AUTO); +} + static ssh_string pki_dsa_signature_to_blob(const ssh_signature sig) { char buffer[40] = { 0 }; @@ -1686,18 +1726,27 @@ int pki_signature_verify(ssh_session session, return SSH_OK; } -ssh_signature pki_do_sign(const ssh_key privkey, - const unsigned char *hash, - size_t hlen) { +ssh_signature pki_do_sign_hash(const ssh_key privkey, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type) +{ ssh_signature sig; int rc; + /* Only RSA supports different signature algorithm types now */ + if (privkey->type != SSH_KEYTYPE_RSA && hash_type != SSH_DIGEST_AUTO) { + SSH_LOG(SSH_LOG_WARN, "Incompatible signature algorithm passed"); + return NULL; + } + sig = ssh_signature_new(); if (sig == NULL) { return NULL; } sig->type = privkey->type; + sig->hash_type = hash_type; sig->type_c = privkey->type_c; switch(privkey->type) { @@ -1720,7 +1769,8 @@ ssh_signature pki_do_sign(const ssh_key privkey, break; case SSH_KEYTYPE_RSA: case SSH_KEYTYPE_RSA1: - sig->rsa_sig = _RSA_do_sign(hash, hlen, privkey->rsa); + sig->type_c = ssh_key_signature_to_char(privkey->type, hash_type); + sig->rsa_sig = _RSA_do_sign_hash(hash, hlen, privkey->rsa, hash_type); if (sig->rsa_sig == NULL) { ssh_signature_free(sig); return NULL; diff --git a/src/pki_gcrypt.c b/src/pki_gcrypt.c index 9a8e8622..c0395e9d 100644 --- a/src/pki_gcrypt.c +++ b/src/pki_gcrypt.c @@ -2079,19 +2079,29 @@ int pki_signature_verify(ssh_session session, return SSH_OK; } -ssh_signature pki_do_sign(const ssh_key privkey, - const unsigned char *hash, - size_t hlen) { +ssh_signature pki_do_sign_hash(const ssh_key privkey, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type) +{ unsigned char ghash[hlen + 1]; + const char *hash_c = NULL; ssh_signature sig; gcry_sexp_t sexp; gcry_error_t err; + /* Only RSA supports different signature algorithm types now */ + if (privkey->type != SSH_KEYTYPE_RSA && hash_type != SSH_DIGEST_AUTO) { + SSH_LOG(SSH_LOG_WARN, "Incompatible signature algorithm passed"); + return NULL; + } + sig = ssh_signature_new(); if (sig == NULL) { return NULL; } sig->type = privkey->type; + sig->hash_type = hash_type; sig->type_c = privkey->type_c; switch (privkey->type) { case SSH_KEYTYPE_DSS: @@ -2117,9 +2127,26 @@ ssh_signature pki_do_sign(const ssh_key privkey, } break; case SSH_KEYTYPE_RSA: + sig->type_c = ssh_key_signature_to_char(privkey->type, hash_type); + switch (hash_type) { + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + hash_c = "sha1"; + break; + case SSH_DIGEST_SHA256: + hash_c = "sha256"; + break; + case SSH_DIGEST_SHA512: + hash_c = "sha512"; + break; + default: + SSH_LOG(SSH_LOG_WARN, "Incomplatible key algorithm"); + return NULL; + } err = gcry_sexp_build(&sexp, NULL, - "(data(flags pkcs1)(hash sha1 %b))", + "(data(flags pkcs1)(hash %s %b))", + hash_c, hlen, hash); if (err) { diff --git a/src/pki_mbedcrypto.c b/src/pki_mbedcrypto.c index 57465009..68a80b42 100644 --- a/src/pki_mbedcrypto.c +++ b/src/pki_mbedcrypto.c @@ -993,21 +993,40 @@ int pki_signature_verify(ssh_session session, const ssh_signature sig, const return SSH_OK; } -static ssh_string rsa_do_sign(const unsigned char *digest, int dlen, - mbedtls_pk_context *privkey) +static ssh_string rsa_do_sign_hash(const unsigned char *digest, + int dlen, + mbedtls_pk_context *privkey, + enum ssh_digest_e hash_type) { ssh_string sig_blob = NULL; + mbedtls_md_type_t md = 0; unsigned char *sig = NULL; size_t slen; int ok; + switch (hash_type) { + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + md = MBEDTLS_MD_SHA1; + break; + case SSH_DIGEST_SHA256: + md = MBEDTLS_MD_SHA256; + break; + case SSH_DIGEST_SHA512: + md = MBEDTLS_MD_SHA512; + break; + default: + SSH_LOG(SSH_LOG_WARN, "Incomplatible key algorithm"); + return NULL; + } + sig = malloc(mbedtls_pk_get_bitlen(privkey) / 8); if (sig == NULL) { return NULL; } ok = mbedtls_pk_sign(privkey, - MBEDTLS_MD_SHA1, + md, digest, dlen, sig, @@ -1034,23 +1053,33 @@ static ssh_string rsa_do_sign(const unsigned char *digest, int dlen, } -ssh_signature pki_do_sign(const ssh_key privkey, const unsigned char *hash, - size_t hlen) +ssh_signature pki_do_sign_hash(const ssh_key privkey, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type) { ssh_signature sig = NULL; int rc; + /* Only RSA supports different signature algorithm types now */ + if (privkey->type != SSH_KEYTYPE_RSA && hash_type != SSH_DIGEST_AUTO) { + SSH_LOG(SSH_LOG_WARN, "Incompatible signature algorithm passed"); + return NULL; + } + sig = ssh_signature_new(); if (sig == NULL) { return NULL; } sig->type = privkey->type; + sig->hash_type = hash_type; sig->type_c = privkey->type_c; switch(privkey->type) { case SSH_KEYTYPE_RSA: - sig->rsa_sig = rsa_do_sign(hash, hlen, privkey->rsa); + sig->type_c = ssh_key_signature_to_char(privkey->type, hash_type); + sig->rsa_sig = rsa_do_sign_hash(hash, hlen, privkey->rsa, hash_type); if (sig->rsa_sig == NULL) { ssh_signature_free(sig); return NULL; @@ -1113,7 +1142,7 @@ ssh_signature pki_do_sign_sessionid(const ssh_key key, const unsigned char switch (key->type) { case SSH_KEYTYPE_RSA: - sig->rsa_sig = rsa_do_sign(hash, hlen, key->rsa); + sig->rsa_sig = rsa_do_sign_hash(hash, hlen, key->rsa, SSH_DIGEST_AUTO); if (sig->rsa_sig == NULL) { ssh_signature_free(sig); return NULL; -- 2.17.1 From 9209dd176a2d4e9c51c7e30d050251fc8e11171f Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Mon, 6 Aug 2018 13:40:32 +0200 Subject: [PATCH 08/22] auth: Support SHA2 extension for pubkey authentication (RFC 8332) Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/pki.h | 4 ++++ src/auth.c | 14 +++++++++----- src/pki.c | 24 ++++++++++++++++++++++-- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/include/libssh/pki.h b/include/libssh/pki.h index 558680d7..0a3823f3 100644 --- a/include/libssh/pki.h +++ b/include/libssh/pki.h @@ -96,6 +96,10 @@ typedef struct ssh_signature_struct *ssh_signature; ssh_key ssh_key_dup(const ssh_key key); void ssh_key_clean (ssh_key key); +const char * +ssh_key_get_signature_algorithm(ssh_session session, + enum ssh_keytypes_e type); + /* SSH Signature Functions */ ssh_signature ssh_signature_new(void); void ssh_signature_free(ssh_signature sign); diff --git a/src/auth.c b/src/auth.c index 9cd80c43..a491cf69 100644 --- a/src/auth.c +++ b/src/auth.c @@ -470,6 +470,7 @@ int ssh_userauth_try_publickey(ssh_session session, const ssh_key pubkey) { ssh_string pubkey_s = NULL; + const char *sig_type_c = NULL; int rc; if (session == NULL) { @@ -506,6 +507,7 @@ int ssh_userauth_try_publickey(ssh_session session, if (rc < 0) { goto fail; } + sig_type_c = ssh_key_get_signature_algorithm(session, pubkey->type); /* request */ rc = ssh_buffer_pack(session->out_buffer, "bsssbsS", @@ -514,7 +516,7 @@ int ssh_userauth_try_publickey(ssh_session session, "ssh-connection", "publickey", 0, /* private key ? */ - pubkey->type_c, /* algo */ + sig_type_c, /* algo */ pubkey_s /* public key */ ); if (rc < 0) { @@ -575,7 +577,7 @@ int ssh_userauth_publickey(ssh_session session, { ssh_string str = NULL; int rc; - const char *type_c; + const char *sig_type_c; enum ssh_keytypes_e key_type; if (session == NULL) { @@ -608,7 +610,7 @@ int ssh_userauth_publickey(ssh_session session, /* Cert auth requires presenting the cert type name (*-cert@xxxxxxxxxxx) */ key_type = privkey->cert != NULL ? privkey->cert_type : privkey->type; - type_c = ssh_key_type_to_char(key_type); + sig_type_c = ssh_key_get_signature_algorithm(session, key_type); /* get public key or cert */ rc = ssh_pki_export_pubkey_blob(privkey, &str); @@ -623,7 +625,7 @@ int ssh_userauth_publickey(ssh_session session, "ssh-connection", "publickey", 1, /* private key */ - type_c, /* algo */ + sig_type_c, /* algo */ str /* public key or cert */ ); if (rc < 0) { @@ -673,6 +675,7 @@ static int ssh_userauth_agent_publickey(ssh_session session, ssh_key pubkey) { ssh_string str = NULL; + const char *sig_type_c = NULL; int rc; switch(session->pending_call_state) { @@ -700,6 +703,7 @@ static int ssh_userauth_agent_publickey(ssh_session session, if (rc < 0) { goto fail; } + sig_type_c = ssh_key_get_signature_algorithm(session, pubkey->type); /* request */ rc = ssh_buffer_pack(session->out_buffer, "bsssbsS", @@ -708,7 +712,7 @@ static int ssh_userauth_agent_publickey(ssh_session session, "ssh-connection", "publickey", 1, /* private key */ - pubkey->type_c, /* algo */ + sig_type_c, /* algo */ str /* public key */ ); if (rc < 0) { diff --git a/src/pki.c b/src/pki.c index 5106aec5..d894762c 100644 --- a/src/pki.c +++ b/src/pki.c @@ -282,8 +282,8 @@ static enum ssh_digest_e ssh_key_hash_from_name(const char *name) * * @return A hash type to be used. */ -static enum ssh_keytypes_e ssh_key_type_to_hash(ssh_session session, - enum ssh_keytypes_e type) +static enum ssh_digest_e ssh_key_type_to_hash(ssh_session session, + enum ssh_keytypes_e type) { /* TODO this should also reflect supported key types specified in * configuration (ssh_config PubkeyAcceptedKeyTypes) */ @@ -309,6 +309,26 @@ static enum ssh_keytypes_e ssh_key_type_to_hash(ssh_session session, return SSH_DIGEST_AUTO; } +/** + * @brief Gets signature algorithm name to be used with the given + * key type. + * + * @param[in] session SSH session. + * @param[in] type The algorithm type to convert. + * + * @return A string for the keytype or NULL if unknown. + */ +const char * +ssh_key_get_signature_algorithm(ssh_session session, + enum ssh_keytypes_e type) +{ + enum ssh_digest_e hash_type; + + hash_type = ssh_key_type_to_hash(session, type); + + return ssh_key_signature_to_char(type, hash_type); +} + /** * @brief Convert a ssh key algorithm name to a ssh key algorithm type. * -- 2.17.1 From 03453320530b8da965a1f786855b8250cc90f08a Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Wed, 27 Jun 2018 15:19:02 +0200 Subject: [PATCH 09/22] tests: SHA2 extension signatures This introduces a new test case for RSA unit tests, verifying that libraries are able to provide and verify the RSA signatures with SHA2 hash algorithms. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- tests/unittests/torture_pki_rsa.c | 50 +++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/unittests/torture_pki_rsa.c b/tests/unittests/torture_pki_rsa.c index 0bdf132f..c0c5aaaf 100644 --- a/tests/unittests/torture_pki_rsa.c +++ b/tests/unittests/torture_pki_rsa.c @@ -15,6 +15,9 @@ #define LIBSSH_RSA_TESTKEY_PASSPHRASE "libssh_testkey_passphrase.id_rsa" const unsigned char RSA_HASH[] = "12345678901234567890"; +const unsigned char SHA256_HASH[] = "12345678901234567890123456789012"; +const unsigned char SHA512_HASH[] = "1234567890123456789012345678901234567890" + "123456789012345678901234"; static int setup_rsa_key(void **state) { @@ -393,6 +396,52 @@ static void torture_pki_rsa_generate_key(void **state) ssh_free(session); } +static void torture_pki_rsa_sha2(void **state) +{ + int rc; + ssh_key key = NULL; + ssh_signature sign; + ssh_session session=ssh_new(); + (void) state; + + /* Setup */ + rc = ssh_pki_generate(SSH_KEYTYPE_RSA, 2048, &key); + assert_true(rc == SSH_OK); + assert_true(key != NULL); + + /* Sign using automatic digest */ + sign = pki_do_sign_hash(key, RSA_HASH, 20, SSH_DIGEST_AUTO); + assert_true(sign != NULL); + rc = pki_signature_verify(session, sign, key, RSA_HASH, 20); + assert_true(rc == SSH_OK); + ssh_signature_free(sign); + + /* Sign using old SHA1 digest */ + sign = pki_do_sign_hash(key, RSA_HASH, 20, SSH_DIGEST_SHA1); + assert_true(sign != NULL); + rc = pki_signature_verify(session, sign, key, RSA_HASH, 20); + assert_true(rc == SSH_OK); + ssh_signature_free(sign); + + /* Sign using new SHA256 digest */ + sign = pki_do_sign_hash(key, SHA256_HASH, 32, SSH_DIGEST_SHA256); + assert_true(sign != NULL); + rc = pki_signature_verify(session, sign, key, SHA256_HASH, 32); + assert_true(rc == SSH_OK); + ssh_signature_free(sign); + + /* Sign using rsa-sha2-512 algorithm */ + sign = pki_do_sign_hash(key, SHA512_HASH, 64, SSH_DIGEST_SHA512); + assert_true(sign != NULL); + rc = pki_signature_verify(session, sign, key, SHA512_HASH, 64); + assert_true(rc == SSH_OK); + ssh_signature_free(sign); + + /* Cleanup */ + ssh_key_free(key); + ssh_free(session); +} + #ifdef HAVE_LIBCRYPTO static void torture_pki_rsa_write_privkey(void **state) { @@ -557,6 +606,7 @@ int torture_run_tests(void) { setup_rsa_key, teardown), #endif /* HAVE_LIBCRYPTO */ + cmocka_unit_test(torture_pki_rsa_sha2), }; ssh_init(); -- 2.17.1 From 7febed76128959c9f39c3e7919022e227b57b9c4 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 26 Jun 2018 12:22:31 +0200 Subject: [PATCH 10/22] SHA2 extension in the ssh-agent interface The new constants for flags are defined in draft-miller-ssh-agent-02 are active if the SHA2 extension is negotiated with the server. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/agent.h | 3 +++ src/agent.c | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/include/libssh/agent.h b/include/libssh/agent.h index 8f9ef941..0142f575 100644 --- a/include/libssh/agent.h +++ b/include/libssh/agent.h @@ -66,6 +66,9 @@ #define SSH_COM_AGENT2_FAILURE 102 #define SSH_AGENT_OLD_SIGNATURE 0x01 +/* Signature flags from draft-miller-ssh-agent-02 */ +#define SSH_AGENT_RSA_SHA2_256 0x02 +#define SSH_AGENT_RSA_SHA2_512 0x04 struct ssh_agent_struct { struct ssh_socket_struct *sock; diff --git a/src/agent.c b/src/agent.c index bcde62aa..15a62556 100644 --- a/src/agent.c +++ b/src/agent.c @@ -548,6 +548,14 @@ ssh_string ssh_agent_sign_data(ssh_session session, return NULL; } + /* Add Flags: SHA2 extension (RFC 8332) if negotiated */ + if (pubkey->type == SSH_KEYTYPE_RSA) { + if (session->extensions & SSH_EXT_SIG_RSA_SHA512) { + flags |= SSH_AGENT_RSA_SHA2_512; + } else if (session->extensions & SSH_EXT_SIG_RSA_SHA256) { + flags |= SSH_AGENT_RSA_SHA2_256; + } + } if (ssh_buffer_add_u32(request, htonl(flags)) < 0) { ssh_buffer_free(request); return NULL; -- 2.17.1 From 8ff73244fbf11d1a5c03b01072703eed35747244 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Mon, 2 Jul 2018 10:50:28 +0200 Subject: [PATCH 11/22] kex: The public key algorithms are no longer only host keys only XXX makes the testsuite pass Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- src/kex.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/kex.c b/src/kex.c index effe8422..7dcf5397 100644 --- a/src/kex.c +++ b/src/kex.c @@ -86,12 +86,12 @@ #ifdef HAVE_ECDH #define ECDH "ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521," -#define HOSTKEYS "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-rsa,rsa-sha2-512,rsa-sha2-256,ssh-dss" +#define PUBLIC_KEY_ALGORITHMS "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-rsa,rsa-sha2-512,rsa-sha2-256,ssh-dss" #else #ifdef HAVE_DSA -#define HOSTKEYS "ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256,ssh-dss" +#define PUBLIC_KEY_ALGORITHMS "ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256,ssh-dss" #else -#define HOSTKEYS "ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256" +#define PUBLIC_KEY_ALGORITHMS "ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256" #endif #define ECDH "" #endif @@ -107,7 +107,7 @@ /* NOTE: This is a fixed API and the index is defined by ssh_kex_types_e */ static const char *default_methods[] = { KEY_EXCHANGE, - HOSTKEYS, + PUBLIC_KEY_ALGORITHMS, AES BLOWFISH DES, AES BLOWFISH DES, "hmac-sha2-256,hmac-sha2-512,hmac-sha1", @@ -122,7 +122,7 @@ static const char *default_methods[] = { /* NOTE: This is a fixed API and the index is defined by ssh_kex_types_e */ static const char *supported_methods[] = { KEY_EXCHANGE, - HOSTKEYS, + PUBLIC_KEY_ALGORITHMS, CHACHA20 AES BLOWFISH DES_SUPPORTED, CHACHA20 AES BLOWFISH DES_SUPPORTED, "hmac-sha2-256,hmac-sha2-512,hmac-sha1", -- 2.17.1 From 721aff8389b9d8da2e2fd49af3cdc3d2467ba2e4 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 11:25:27 +0200 Subject: [PATCH 12/22] options: The new option SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES This option allows to specify acceptable public key algorithms and reflects the PubkeyAcceptedTypes configuration option from OpenSSH. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/libssh.h | 1 + include/libssh/session.h | 1 + src/options.c | 31 +++++++++++++++++++++++++++++++ src/session.c | 1 + 4 files changed, 34 insertions(+) diff --git a/include/libssh/libssh.h b/include/libssh/libssh.h index 01e0a138..1f5cdf87 100644 --- a/include/libssh/libssh.h +++ b/include/libssh/libssh.h @@ -404,6 +404,7 @@ enum ssh_options_e { SSH_OPTIONS_GSSAPI_AUTH, SSH_OPTIONS_GLOBAL_KNOWNHOSTS, SSH_OPTIONS_NODELAY, + SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, }; enum { diff --git a/include/libssh/session.h b/include/libssh/session.h index 00717652..107d4aec 100644 --- a/include/libssh/session.h +++ b/include/libssh/session.h @@ -204,6 +204,7 @@ struct ssh_session_struct { char *knownhosts; char *global_knownhosts; char *wanted_methods[10]; + char *pubkey_accepted_types; char *ProxyCommand; char *custombanner; unsigned long timeout; /* seconds */ diff --git a/src/options.c b/src/options.c index 0e428e65..bc456b87 100644 --- a/src/options.c +++ b/src/options.c @@ -147,6 +147,14 @@ int ssh_options_copy(ssh_session src, ssh_session *dest) { return -1; } } + + if (src->opts.pubkey_accepted_types) { + new->opts.pubkey_accepted_types = strdup(src->opts.pubkey_accepted_types); + if (new->opts.pubkey_accepted_types == NULL) { + ssh_free(new); + return -1; + } + } new->opts.fd = src->opts.fd; new->opts.port = src->opts.port; new->opts.timeout = src->opts.timeout; @@ -343,6 +351,11 @@ int ssh_options_set_algo(ssh_session session, * comma-separated list). ex: * "ssh-rsa,ssh-dss,ecdh-sha2-nistp256" * + * - SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES: + * Set the preferred public key algorithms to be used for + * authentication (const char *, comma-separated list). ex: + * "ssh-rsa,rsa-sha2-256,ssh-dss,ecdh-sha2-nistp256" + * * - SSH_OPTIONS_COMPRESSION_C_S: * Set the compression to use for client to server * communication (const char *, "yes", "no" or a specific @@ -743,6 +756,24 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type, return -1; } break; + case SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES: + v = value; + if (v == NULL || v[0] == '\0') { + ssh_set_error_invalid(session); + return -1; + } else { + p = ssh_keep_known_algos(SSH_HOSTKEYS, v); + if (p == NULL) { + ssh_set_error(session, SSH_REQUEST_DENIED, + "Setting method: no known public key algorithm (%s)", + v); + return -1; + } + + SAFE_FREE(session->opts.pubkey_accepted_types); + session->opts.pubkey_accepted_types = p; + } + break; case SSH_OPTIONS_HMAC_C_S: v = value; if (v == NULL || v[0] == '\0') { diff --git a/src/session.c b/src/session.c index a1959d48..28255221 100644 --- a/src/session.c +++ b/src/session.c @@ -282,6 +282,7 @@ void ssh_free(ssh_session session) { SAFE_FREE(session->opts.ProxyCommand); SAFE_FREE(session->opts.gss_server_identity); SAFE_FREE(session->opts.gss_client_identity); + SAFE_FREE(session->opts.pubkey_accepted_types); for (i = 0; i < 10; i++) { if (session->opts.wanted_methods[i]) { -- 2.17.1 From 62678612e9a856719ee141e222df67cbb4aae1e0 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 11:29:19 +0200 Subject: [PATCH 13/22] config: Accept the PubkeyAcceptedTypes configuration option Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- src/config.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/config.c b/src/config.c index 49f221e6..6435f860 100644 --- a/src/config.c +++ b/src/config.c @@ -72,6 +72,7 @@ enum ssh_config_opcode_e { SOC_KBDINTERACTIVEAUTHENTICATION, SOC_PASSWORDAUTHENTICATION, SOC_PUBKEYAUTHENTICATION, + SOC_PUBKEYACCEPTEDTYPES, SOC_END /* Keep this one last in the list */ }; @@ -144,7 +145,7 @@ static struct ssh_config_keyword_table_s ssh_config_keyword_table[] = { { "preferredauthentications", SOC_UNSUPPORTED}, { "proxyjump", SOC_UNSUPPORTED}, { "proxyusefdpass", SOC_UNSUPPORTED}, - { "pubkeyacceptedtypes", SOC_UNSUPPORTED}, + { "pubkeyacceptedtypes", SOC_PUBKEYACCEPTEDTYPES}, { "rekeylimit", SOC_UNSUPPORTED}, { "remotecommand", SOC_UNSUPPORTED}, { "revokedhostkeys", SOC_UNSUPPORTED}, @@ -592,6 +593,12 @@ static int ssh_config_parse_line(ssh_session session, const char *line, ssh_options_set(session, SSH_OPTIONS_HOSTKEYS, p); } break; + case SOC_PUBKEYACCEPTEDTYPES: + p = ssh_config_get_str_tok(&s, NULL); + if (p && *parsing) { + ssh_options_set(session, SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, p); + } + break; case SOC_KEXALGORITHMS: p = ssh_config_get_str_tok(&s, NULL); if (p && *parsing) { -- 2.17.1 From 7d5ed41232dc9d0f431ac9e6005b589c29b03502 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 11:31:01 +0200 Subject: [PATCH 14/22] tests: Cover PubkeyAcceptedTypes configuration option Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- tests/unittests/torture_config.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c index b21efad0..aa4bfd2d 100644 --- a/tests/unittests/torture_config.c +++ b/tests/unittests/torture_config.c @@ -24,6 +24,7 @@ extern LIBSSH_THREAD int ssh_log_level; #define ID_FILE "/etc/xxx" #define KEXALGORITHMS "ecdh-sha2-nistp521,diffie-hellman-group14-sha1" #define HOSTKEYALGORITHMS "ssh-ed25519,ecdsa-sha2-nistp521,ssh-rsa" +#define PUBKEYACCEPTEDTYPES "rsa-sha2-512,ssh-rsa,ecdsa-sha2-nistp521" #define MACS "hmac-sha1,hmac-sha2-256" #define USER_KNOWN_HOSTS "%d/my_known_hosts" #define GLOBAL_KNOWN_HOSTS "/etc/ssh/my_ssh_known_hosts" @@ -51,6 +52,7 @@ static int setup_config_files(void **state) "\n\nIdentityFile "ID_FILE"\n" "\n\nKexAlgorithms "KEXALGORITHMS"\n" "\n\nHostKeyAlgorithms "HOSTKEYALGORITHMS"\n" + "\n\nPubkeyAcceptedTypes "PUBKEYACCEPTEDTYPES"\n" "\n\nMACs "MACS"\n"); /* Multiple Port settings -> parsing returns early. */ @@ -161,6 +163,8 @@ static void torture_config_from_file(void **state) { assert_string_equal(session->opts.wanted_methods[SSH_HOSTKEYS], HOSTKEYALGORITHMS); + assert_string_equal(session->opts.pubkey_accepted_types, PUBKEYACCEPTEDTYPES); + assert_string_equal(session->opts.wanted_methods[SSH_MAC_C_S], MACS); assert_string_equal(session->opts.wanted_methods[SSH_MAC_S_C], MACS); } -- 2.17.1 From 82d5a768610be4db4966d4f90306fe3fede97ec3 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 11:32:36 +0200 Subject: [PATCH 15/22] pki: Allow filtering accepted public key types based on the configuration This effectively allows to disable using the SHA2 extension, disable other old public key mechanisms out of the box (hello DSA) or force the new SHA2-based key algorithm types if needed. This exposes the default_methods array from kex.c. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/pki.h | 1 + src/kex.c | 2 +- src/pki.c | 30 ++++++++++++++++++++++++++---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/include/libssh/pki.h b/include/libssh/pki.h index 0a3823f3..621378ad 100644 --- a/include/libssh/pki.h +++ b/include/libssh/pki.h @@ -138,4 +138,5 @@ ssh_string ssh_srv_pki_do_sign_sessionid(ssh_session session, ssh_public_key ssh_pki_convert_key_to_publickey(const ssh_key key); ssh_private_key ssh_pki_convert_key_to_privatekey(const ssh_key key); +int ssh_key_algorithm_allowed(ssh_session session, const char *type); #endif /* PKI_H_ */ diff --git a/src/kex.c b/src/kex.c index 7dcf5397..165a953f 100644 --- a/src/kex.c +++ b/src/kex.c @@ -105,7 +105,7 @@ #define KEX_EXTENSION_CLIENT "ext-info-c" /* NOTE: This is a fixed API and the index is defined by ssh_kex_types_e */ -static const char *default_methods[] = { +const char *default_methods[] = { KEY_EXCHANGE, PUBLIC_KEY_ALGORITHMS, AES BLOWFISH DES, diff --git a/src/pki.c b/src/pki.c index d894762c..4299714d 100644 --- a/src/pki.c +++ b/src/pki.c @@ -80,6 +80,8 @@ enum ssh_keytypes_e pki_privatekey_type_from_string(const char *privkey) { return SSH_KEYTYPE_UNKNOWN; } +extern const char *default_methods[]; + /** * @brief returns the ECDSA key name ("ecdsa-sha2-nistp256" for example) * @@ -271,6 +273,26 @@ static enum ssh_digest_e ssh_key_hash_from_name(const char *name) /* we do not care for others now */ return SSH_DIGEST_AUTO; } +/** + * @brief Checks the given key against the configured allowed + * public key algorithm types + * + * @param[in] session The SSH session + * @parma[in] type The key algorithm to check + * @returns 1 if the key algorithm is allowed 0 otherwise + */ +int ssh_key_algorithm_allowed(ssh_session session, const char *type) +{ + const char *allowed_list; + + allowed_list = session->opts.pubkey_accepted_types; + if (allowed_list == NULL) + allowed_list = default_methods[SSH_HOSTKEYS]; + + SSH_LOG(SSH_LOG_DEBUG, "Checking %s with list <%s>", type, allowed_list); + return ssh_match_group(allowed_list, type); +} + /** * @brief Convert a key type to a hash type. This is usually unambiguous * for all the key types, unless the SHA2 extension (RFC 8332) is @@ -285,15 +307,15 @@ static enum ssh_digest_e ssh_key_hash_from_name(const char *name) static enum ssh_digest_e ssh_key_type_to_hash(ssh_session session, enum ssh_keytypes_e type) { - /* TODO this should also reflect supported key types specified in - * configuration (ssh_config PubkeyAcceptedKeyTypes) */ switch (type) { case SSH_KEYTYPE_RSA: - if (session->extensions & SSH_EXT_SIG_RSA_SHA512) { + if (ssh_key_algorithm_allowed(session, "rsa-sha2-512") && + (session->extensions & SSH_EXT_SIG_RSA_SHA512)) { return SSH_DIGEST_SHA512; } - if (session->extensions & SSH_EXT_SIG_RSA_SHA256) { + if (ssh_key_algorithm_allowed(session, "rsa-sha2-256") && + (session->extensions & SSH_EXT_SIG_RSA_SHA256)) { return SSH_DIGEST_SHA256; } -- 2.17.1 From c54bb14c048b7cd011d922c89c21967e0d1bb22a Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 7 Aug 2018 11:34:30 +0200 Subject: [PATCH 16/22] tests: PUBLICKEY_ACCEPTED_TYPES are effective Verify the PUBLICKEY_ACCEPTED_TYPES option is handled correctly and affects the signature algorithm selection based on the extensions and can be used to limit list of offered mechanisms to the server. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/pki_priv.h | 2 ++ src/pki.c | 2 +- tests/unittests/torture_options.c | 50 +++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/include/libssh/pki_priv.h b/include/libssh/pki_priv.h index 38c7aa4b..623ca5a1 100644 --- a/include/libssh/pki_priv.h +++ b/include/libssh/pki_priv.h @@ -47,6 +47,8 @@ int pki_key_ecdsa_nid_from_name(const char *name); const char *pki_key_ecdsa_nid_to_name(int nid); const char *ssh_key_signature_to_char(enum ssh_keytypes_e type, enum ssh_digest_e hash_type); +enum ssh_digest_e ssh_key_type_to_hash(ssh_session session, + enum ssh_keytypes_e type); /* SSH Key Functions */ ssh_key pki_key_dup(const ssh_key key, int demote); diff --git a/src/pki.c b/src/pki.c index 4299714d..99c29156 100644 --- a/src/pki.c +++ b/src/pki.c @@ -304,7 +304,7 @@ int ssh_key_algorithm_allowed(ssh_session session, const char *type) * * @return A hash type to be used. */ -static enum ssh_digest_e ssh_key_type_to_hash(ssh_session session, +enum ssh_digest_e ssh_key_type_to_hash(ssh_session session, enum ssh_keytypes_e type) { switch (type) { diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c index 412df102..b531b542 100644 --- a/tests/unittests/torture_options.c +++ b/tests/unittests/torture_options.c @@ -11,6 +11,7 @@ #include "torture_key.h" #include <libssh/session.h> #include <libssh/misc.h> +#include <libssh/pki_priv.h> static int setup(void **state) { @@ -115,6 +116,54 @@ static void torture_options_set_hostkey(void **state) { assert_false(rc == 0); } +static void torture_options_set_pubkey_accepted_types(void **state) { + ssh_session session = *state; + int rc; + enum ssh_digest_e type; + + /* Test known public key algorithms */ + rc = ssh_options_set(session, + SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "ssh-ed25519,ecdsa-sha2-nistp384,ssh-rsa"); + assert_true(rc == 0); + assert_string_equal(session->opts.pubkey_accepted_types, + "ssh-ed25519,ecdsa-sha2-nistp384,ssh-rsa"); + + /* Test one unknown public key algorithms */ + rc = ssh_options_set(session, + SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "ssh-ed25519,unknown-crap@xxxxxxxxxxx,ssh-rsa"); + assert_true(rc == 0); + assert_string_equal(session->opts.pubkey_accepted_types, + "ssh-ed25519,ssh-rsa"); + + /* Test all unknown public key algorithms */ + rc = ssh_options_set(session, + SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "unknown-crap@xxxxxxxxxxx,more-crap@xxxxxxxxxxx"); + assert_false(rc == 0); + + /* Test that the option affects the algorithm selection for RSA keys */ + /* simulate the SHA2 extension was negotiated */ + session->extensions = SSH_EXT_SIG_RSA_SHA256; + + /* previous configuration did not list the SHA2 extension algoritms, so + * it should not be used */ + type = ssh_key_type_to_hash(session, SSH_KEYTYPE_RSA); + assert_int_equal(type, SSH_DIGEST_SHA1); + + /* now, lets allow the signature from SHA2 extension and expect + * it to be used */ + rc = ssh_options_set(session, + SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "rsa-sha2-256,ssh-rsa"); + assert_true(rc == 0); + assert_string_equal(session->opts.pubkey_accepted_types, + "rsa-sha2-256,ssh-rsa"); + type = ssh_key_type_to_hash(session, SSH_KEYTYPE_RSA); + assert_int_equal(type, SSH_DIGEST_SHA256); +} + static void torture_options_set_macs(void **state) { ssh_session session = *state; int rc; @@ -401,6 +450,7 @@ int torture_run_tests(void) { cmocka_unit_test_setup_teardown(torture_options_set_ciphers, setup, teardown), cmocka_unit_test_setup_teardown(torture_options_set_key_exchange, setup, teardown), cmocka_unit_test_setup_teardown(torture_options_set_hostkey, setup, teardown), + cmocka_unit_test_setup_teardown(torture_options_set_pubkey_accepted_types, setup, teardown), cmocka_unit_test_setup_teardown(torture_options_set_macs, setup, teardown), cmocka_unit_test_setup_teardown(torture_options_config_host, setup, teardown) }; -- 2.17.1 From 2eef6bedf36ec5aaebd88c019ce6695a656b78ef Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Mon, 2 Jul 2018 16:44:47 +0200 Subject: [PATCH 17/22] auth: Prevent authentication with non-allowed key algorithms Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- src/auth.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/src/auth.c b/src/auth.c index a491cf69..c1bb2cc5 100644 --- a/src/auth.c +++ b/src/auth.c @@ -495,6 +495,17 @@ int ssh_userauth_try_publickey(ssh_session session, return SSH_ERROR; } + sig_type_c = ssh_key_get_signature_algorithm(session, pubkey->type); + + /* Check if the given public key algorithm is allowed */ + if (!ssh_key_algorithm_allowed(session, sig_type_c)) { + ssh_set_error(session, SSH_REQUEST_DENIED, + "The key algorithm '%s' is not allowed to be used by" + " PUBLICKEY_ACCEPTED_TYPES configuration option", + sig_type_c); + return SSH_AUTH_DENIED; + } + rc = ssh_userauth_request_service(session); if (rc == SSH_AGAIN) { return SSH_AUTH_AGAIN; @@ -507,7 +518,6 @@ int ssh_userauth_try_publickey(ssh_session session, if (rc < 0) { goto fail; } - sig_type_c = ssh_key_get_signature_algorithm(session, pubkey->type); /* request */ rc = ssh_buffer_pack(session->out_buffer, "bsssbsS", @@ -601,6 +611,19 @@ int ssh_userauth_publickey(ssh_session session, return SSH_AUTH_ERROR; } + /* Cert auth requires presenting the cert type name (*-cert@xxxxxxxxxxx) */ + key_type = privkey->cert != NULL ? privkey->cert_type : privkey->type; + sig_type_c = ssh_key_get_signature_algorithm(session, key_type); + + /* Check if the given public key algorithm is allowed */ + if (!ssh_key_algorithm_allowed(session, sig_type_c)) { + ssh_set_error(session, SSH_REQUEST_DENIED, + "The key algorithm '%s' is not allowed to be used by" + " PUBLICKEY_ACCEPTED_TYPES configuration option", + sig_type_c); + return SSH_AUTH_DENIED; + } + rc = ssh_userauth_request_service(session); if (rc == SSH_AGAIN) { return SSH_AUTH_AGAIN; @@ -608,10 +631,6 @@ int ssh_userauth_publickey(ssh_session session, return SSH_AUTH_ERROR; } - /* Cert auth requires presenting the cert type name (*-cert@xxxxxxxxxxx) */ - key_type = privkey->cert != NULL ? privkey->cert_type : privkey->type; - sig_type_c = ssh_key_get_signature_algorithm(session, key_type); - /* get public key or cert */ rc = ssh_pki_export_pubkey_blob(privkey, &str); if (rc < 0) { @@ -697,7 +716,6 @@ static int ssh_userauth_agent_publickey(ssh_session session, return SSH_AUTH_ERROR; } - /* public key */ rc = ssh_pki_export_pubkey_blob(pubkey, &str); if (rc < 0) { @@ -705,6 +723,15 @@ static int ssh_userauth_agent_publickey(ssh_session session, } sig_type_c = ssh_key_get_signature_algorithm(session, pubkey->type); + /* Check if the given public key algorithm is allowed */ + if (!ssh_key_algorithm_allowed(session, sig_type_c)) { + ssh_set_error(session, SSH_REQUEST_DENIED, + "The key algorithm '%s' is not allowed to be used by" + " PUBLICKEY_ACCEPTED_TYPES configuration option", + sig_type_c); + return SSH_AUTH_DENIED; + } + /* request */ rc = ssh_buffer_pack(session->out_buffer, "bsssbsS", SSH2_MSG_USERAUTH_REQUEST, -- 2.17.1 From fbadd02113f62cfbaf48ab0d5a71816e2f9415a2 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Mon, 2 Jul 2018 16:49:04 +0200 Subject: [PATCH 18/22] tests: Verify the public key algorithms can be limited by configuration option SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES configuration option can limit what keys can or can not be used for public key authentication. This is useful for disabling obsolete algorithms while not completely removing the support for them or allows to configure what public key algorithms will be used with the SHA2 RSA extension. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- tests/client/torture_auth.c | 87 +++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tests/client/torture_auth.c b/tests/client/torture_auth.c index 7c436711..e1b096d6 100644 --- a/tests/client/torture_auth.c +++ b/tests/client/torture_auth.c @@ -547,6 +547,87 @@ static void torture_auth_agent_cert_nonblocking(void **state) { torture_auth_agent_nonblocking(state); } +static void torture_auth_pubkey_types(void **state) { + struct torture_state *s = *state; + ssh_session session = s->ssh.session; + int rc; + + rc = ssh_options_set(session, SSH_OPTIONS_USER, TORTURE_SSH_USER_ALICE); + assert_int_equal(rc, SSH_OK); + + rc = ssh_connect(session); + assert_int_equal(rc, SSH_OK); + + rc = ssh_userauth_none(session,NULL); + /* This request should return a SSH_REQUEST_DENIED error */ + if (rc == SSH_ERROR) { + assert_true(ssh_get_error_code(session) == SSH_REQUEST_DENIED); + } + rc = ssh_userauth_list(session, NULL); + assert_true(rc & SSH_AUTH_METHOD_PUBLICKEY); + + /* Disable RSA key types for authentication */ + rc = ssh_options_set(session, SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "ssh-dss"); + assert_int_equal(rc, SSH_OK); + + rc = ssh_userauth_publickey_auto(session, NULL, NULL); + assert_int_equal(rc, SSH_AUTH_DENIED); + + /* Now enable it and retry */ + rc = ssh_options_set(session, SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "rsa-sha2-512,ssh-rsa"); + assert_int_equal(rc, SSH_OK); + + rc = ssh_userauth_publickey_auto(session, NULL, NULL); + assert_int_equal(rc, SSH_AUTH_SUCCESS); +} + +static void torture_auth_pubkey_types_nonblocking(void **state) { + struct torture_state *s = *state; + ssh_session session = s->ssh.session; + int rc; + + rc = ssh_options_set(session, SSH_OPTIONS_USER, TORTURE_SSH_USER_ALICE); + assert_int_equal(rc, SSH_OK); + + rc = ssh_connect(session); + assert_int_equal(rc, SSH_OK); + + ssh_set_blocking(session,0); + do { + rc = ssh_userauth_none(session, NULL); + } while (rc == SSH_AUTH_AGAIN); + + /* This request should return a SSH_REQUEST_DENIED error */ + if (rc == SSH_ERROR) { + assert_int_equal(ssh_get_error_code(session), SSH_REQUEST_DENIED); + } + + rc = ssh_userauth_list(session, NULL); + assert_true(rc & SSH_AUTH_METHOD_PUBLICKEY); + + /* Disable RSA key types for authentication */ + rc = ssh_options_set(session, SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "ssh-dss"); + assert_int_equal(rc, SSH_OK); + + do { + rc = ssh_userauth_publickey_auto(session, NULL, NULL); + } while (rc == SSH_AUTH_AGAIN); + assert_int_equal(rc, SSH_AUTH_DENIED); + + /* Now enable it and retry */ + rc = ssh_options_set(session, SSH_OPTIONS_PUBLICKEY_ACCEPTED_TYPES, + "rsa-sha2-512,ssh-rsa"); + assert_int_equal(rc, SSH_OK); + + do { + rc = ssh_userauth_publickey_auto(session, NULL, NULL); + } while (rc == SSH_AUTH_AGAIN); + assert_int_equal(rc, SSH_AUTH_SUCCESS); +} + int torture_run_tests(void) { int rc; @@ -590,6 +671,12 @@ int torture_run_tests(void) { cmocka_unit_test_setup_teardown(torture_auth_agent_cert_nonblocking, agent_cert_setup, agent_teardown), + cmocka_unit_test_setup_teardown(torture_auth_pubkey_types, + pubkey_setup, + session_teardown), + cmocka_unit_test_setup_teardown(torture_auth_pubkey_types_nonblocking, + pubkey_setup, + session_teardown), }; ssh_init(); -- 2.17.1 From 82ce9ea132e524ae01dc28248b2ce898078b344d Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 3 Jul 2018 15:53:44 +0200 Subject: [PATCH 19/22] messages: Create correct digest for pki signatures This does not affect old signatures, where the public key algorithm matches the public key type. This is a problem when using SHA2 extension for the RSA keys, where the new signature algorithsm are introduced in addition to the exitsing ssh-rsa which was ignored throughout the code. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- src/messages.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/messages.c b/src/messages.c index 6fe87f78..8a469d41 100644 --- a/src/messages.c +++ b/src/messages.c @@ -645,7 +645,8 @@ error: */ static ssh_buffer ssh_msg_userauth_build_digest(ssh_session session, ssh_message msg, - const char *service) + const char *service, + ssh_string algo) { struct ssh_crypto_struct *crypto = session->current_crypto ? session->current_crypto : @@ -673,7 +674,7 @@ static ssh_buffer ssh_msg_userauth_build_digest(ssh_session session, service, "publickey", /* method */ 1, /* has to be signed (true) */ - msg->auth_request.pubkey->type_c, /* pubkey algorithm */ + ssh_string_get_char(algo), /* pubkey algorithm */ str); /* public key as a blob */ ssh_string_free(str); @@ -785,13 +786,13 @@ SSH_PACKET_CALLBACK(ssh_packet_userauth_request){ if (rc != SSH_OK) { goto error; } - ssh_string_free(algo); - algo = NULL; rc = ssh_pki_import_pubkey_blob(pubkey_blob, &msg->auth_request.pubkey); ssh_string_free(pubkey_blob); pubkey_blob = NULL; if (rc < 0) { + ssh_string_free(algo); + algo = NULL; goto error; } msg->auth_request.signature_state = SSH_PUBLICKEY_STATE_NONE; @@ -804,10 +805,14 @@ SSH_PACKET_CALLBACK(ssh_packet_userauth_request){ if(sig_blob == NULL) { SSH_LOG(SSH_LOG_PACKET, "Invalid signature packet from peer"); msg->auth_request.signature_state = SSH_PUBLICKEY_STATE_ERROR; + ssh_string_free(algo); + algo = NULL; goto error; } - digest = ssh_msg_userauth_build_digest(session, msg, service); + digest = ssh_msg_userauth_build_digest(session, msg, service, algo); + ssh_string_free(algo); + algo = NULL; if (digest == NULL) { ssh_string_free(sig_blob); SSH_LOG(SSH_LOG_PACKET, "Failed to get digest"); -- 2.17.1 From cd81b84f350d032b78dc00dce2de04f6da84dba7 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 3 Jul 2018 16:16:25 +0200 Subject: [PATCH 20/22] server: Support for extension negotiation This includes intercepting the ext-info-c string from the client kex proposal, configuring the server to allow using this extension and sending the SSH_MSG_EXT_INFO packet back to the client after the new keys are in use. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/session.h | 1 + src/kex.c | 16 ++++++++++++++++ src/server.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/include/libssh/session.h b/include/libssh/session.h index 107d4aec..2f1c4269 100644 --- a/include/libssh/session.h +++ b/include/libssh/session.h @@ -90,6 +90,7 @@ enum ssh_pending_call_e { /* server-sig-algs extension */ #define SSH_EXT_SIG_RSA_SHA256 0x01 #define SSH_EXT_SIG_RSA_SHA512 0x02 +#define SSH_EXT_ALL SSH_EXT_SIG_RSA_SHA256 | SSH_EXT_SIG_RSA_SHA512 /* members that are common to ssh_session and ssh_bind */ struct ssh_common_struct { diff --git a/src/kex.c b/src/kex.c index 165a953f..663cadcf 100644 --- a/src/kex.c +++ b/src/kex.c @@ -510,6 +510,22 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit){ goto error; } + /* + * If client sent a ext-info-c message in the kex list, it supports + * RFC 8308 extension negotiation. + */ + rc = ssh_match_group(session->next_crypto->client_kex.methods[SSH_KEX], + KEX_EXTENSION_CLIENT); + if (rc == 1) { + /* + * Enable all the supported extensions and when the time comes + * (after NEWKEYS) send them to the client. + */ + SSH_LOG(SSH_LOG_DEBUG, "The client supports extension " + "negotiation: enabling all extensions"); + session->extensions = SSH_EXT_ALL; + } + /* * Remember whether 'first_kex_packet_follows' was set and the client * guess was wrong: in this case the next SSH_MSG_KEXDH_INIT message diff --git a/src/server.c b/src/server.c index 8984e6a0..f315bd14 100644 --- a/src/server.c +++ b/src/server.c @@ -67,6 +67,7 @@ static int dh_handshake_server(ssh_session session); +extern char *default_methods[]; /** * @addtogroup libssh_server @@ -194,6 +195,35 @@ static int ssh_server_kexdh_init(ssh_session session, ssh_buffer packet){ return SSH_OK; } +static int ssh_server_send_extensions(ssh_session session) { + int rc; + + SSH_LOG(SSH_LOG_PACKET, "Sending SSH_MSG_EXT_INFO"); + /* + * We can list here all the default hostkey methods, since + * they already contain the SHA2 extension algorithms + */ + rc = ssh_buffer_pack(session->out_buffer, + "bdss", + SSH2_MSG_EXT_INFO, + 1, /* nr. of extensions */ + "server-sig-algs", + default_methods[SSH_HOSTKEYS]); + if (rc != SSH_OK) { + goto error; + } + + if (ssh_packet_send(session) == SSH_ERROR) { + goto error; + } + + return 0; +error: + ssh_buffer_reinit(session->out_buffer); + + return -1; +} + SSH_PACKET_CALLBACK(ssh_packet_kexdh_init){ int rc = SSH_ERROR; (void)type; @@ -486,6 +516,15 @@ static void ssh_server_connection_callback(ssh_session session){ session->session_state=SSH_SESSION_STATE_AUTHENTICATING; if (session->flags & SSH_SESSION_FLAG_AUTHENTICATED) session->session_state = SSH_SESSION_STATE_AUTHENTICATED; + + /* + * If the client supports extension negotiation, we will send + * our supported extensions now. This is the first message after + * sending NEWKEYS message and after turning on crypto. + */ + if (session->extensions) { + ssh_server_send_extensions(session); + } } break; case SSH_SESSION_STATE_AUTHENTICATING: -- 2.17.1 From 56ffa13ec3894ab5ae2d993bf3bda52558bb0767 Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Tue, 3 Jul 2018 16:54:35 +0200 Subject: [PATCH 21/22] server: We should list SHA2 variants in offered hostkeys The SHA2 variants should be preferred. Also the buffer needs to be extended to fit all possible public key algorithms. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- src/server.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/server.c b/src/server.c index f315bd14..de5ad7ef 100644 --- a/src/server.c +++ b/src/server.c @@ -88,7 +88,7 @@ static int server_set_kex(ssh_session session) { struct ssh_kex_struct *server = &session->next_crypto->server_kex; int i, j, rc; const char *wanted; - char hostkeys[64] = {0}; + char hostkeys[128] = {0}; enum ssh_keytypes_e keytype; size_t len; int ok; @@ -124,6 +124,11 @@ static int server_set_kex(ssh_session session) { } #endif if (session->srv.rsa_key != NULL) { + /* We support also the SHA2 variants */ + len = strlen(hostkeys); + snprintf(hostkeys + len, sizeof(hostkeys) - len, + ",rsa-sha2-512,rsa-sha2-256"); + len = strlen(hostkeys); keytype = ssh_key_type(session->srv.rsa_key); -- 2.17.1 From a96d50c0b0dbed0b29ae1ce23bb0a25771b98f6e Mon Sep 17 00:00:00 2001 From: Jakub Jelen <jjelen@xxxxxxxxxx> Date: Mon, 6 Aug 2018 14:32:28 +0200 Subject: [PATCH 22/22] pki: Support RSA SHA2 signatures of sessionid for server This involves mostly creation of host keys proofs but needs to follow the same procedure as the client authentication signatures. At the same time, the SHA2 extension is enabled in the pkd so we are able to atomicaly provide correct signatures and pass tests. Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx> --- include/libssh/pki.h | 1 + include/libssh/pki_priv.h | 9 ++++++--- src/pki.c | 33 ++++++++++++++++++++++++--------- src/pki_crypto.c | 37 +++++++++++++------------------------ src/pki_gcrypt.c | 33 +++++++++++++++++++++++++++++---- src/pki_mbedcrypto.c | 16 +++++++++++++--- src/wrapper.c | 2 +- tests/pkd/pkd_client.h | 4 ++-- 8 files changed, 89 insertions(+), 46 deletions(-) diff --git a/include/libssh/pki.h b/include/libssh/pki.h index 621378ad..b682f273 100644 --- a/include/libssh/pki.h +++ b/include/libssh/pki.h @@ -99,6 +99,7 @@ void ssh_key_clean (ssh_key key); const char * ssh_key_get_signature_algorithm(ssh_session session, enum ssh_keytypes_e type); +enum ssh_keytypes_e ssh_key_type_from_signature_name(const char *name); /* SSH Signature Functions */ ssh_signature ssh_signature_new(void); diff --git a/include/libssh/pki_priv.h b/include/libssh/pki_priv.h index 623ca5a1..fe7e92a8 100644 --- a/include/libssh/pki_priv.h +++ b/include/libssh/pki_priv.h @@ -104,9 +104,12 @@ ssh_signature pki_do_sign_hash(const ssh_key privkey, const unsigned char *hash, size_t hlen, enum ssh_digest_e hash_type); -ssh_signature pki_do_sign_sessionid(const ssh_key key, - const unsigned char *hash, - size_t hlen); +#define pki_do_sign_sessionid(key, hash, hlen) \ + pki_do_sign_sessionid_hash(key, hash, hlen, SSH_DIGEST_AUTO) +ssh_signature pki_do_sign_sessionid_hash(const ssh_key key, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type); int pki_ed25519_sign(const ssh_key privkey, ssh_signature sig, const unsigned char *hash, size_t hlen); int pki_ed25519_verify(const ssh_key pubkey, ssh_signature sig, diff --git a/src/pki.c b/src/pki.c index 99c29156..f7ce60c9 100644 --- a/src/pki.c +++ b/src/pki.c @@ -358,7 +358,7 @@ ssh_key_get_signature_algorithm(ssh_session session, * * @return The enum ssh key algorithm type. */ -static enum ssh_keytypes_e ssh_key_type_from_signature_name(const char *name) { +enum ssh_keytypes_e ssh_key_type_from_signature_name(const char *name) { if (name == NULL) { return SSH_KEYTYPE_UNKNOWN; } @@ -2034,21 +2034,36 @@ ssh_string ssh_srv_pki_do_sign_sessionid(ssh_session session, sig = NULL; } } else { - unsigned char hash[SHA_DIGEST_LEN] = {0}; - SHACTX ctx; + unsigned char hash[SHA512_DIGEST_LEN] = {0}; + uint32_t hlen = 0; + enum ssh_digest_e hash_type; - ctx = sha1_init(); - if (ctx == NULL) { + hash_type = ssh_key_type_to_hash(session, privkey->type); + switch (hash_type) { + case SSH_DIGEST_SHA256: + sha256(crypto->secret_hash, crypto->digest_len, hash); + hlen = SHA256_DIGEST_LEN; + break; + case SSH_DIGEST_SHA512: + sha512(crypto->secret_hash, crypto->digest_len, hash); + hlen = SHA512_DIGEST_LEN; + break; + case SSH_DIGEST_SHA1: + case SSH_DIGEST_AUTO: + sha1(crypto->secret_hash, crypto->digest_len, hash); + hlen = SHA_DIGEST_LEN; + break; + default: + SSH_LOG(SSH_LOG_TRACE, "Unknown sig->type: %d", sig->type); return NULL; } - sha1_update(ctx, crypto->secret_hash, crypto->digest_len); - sha1_final(hash, ctx); + #ifdef DEBUG_CRYPTO - ssh_print_hexa("Hash being signed", hash, SHA_DIGEST_LEN); + ssh_print_hexa("Hash being signed", hash, hlen); #endif - sig = pki_do_sign_sessionid(privkey, hash, SHA_DIGEST_LEN); + sig = pki_do_sign_sessionid_hash(privkey, hash, hlen, hash_type); if (sig == NULL) { return NULL; } diff --git a/src/pki_crypto.c b/src/pki_crypto.c index 524cc0ff..8e1e16ad 100644 --- a/src/pki_crypto.c +++ b/src/pki_crypto.c @@ -1236,26 +1236,6 @@ static ssh_string _RSA_do_sign_hash(const unsigned char *digest, return sig_blob; } -/** - * @internal - * - * @brief Compute a digital signature. - * - * @param[in] digest The message digest. - * - * @param[in] dlen The length of the digest. - * - * @param[in] privkey The private rsa key to use for signing. - * - * @return A newly allocated rsa sig blob or NULL on error. - */ -static ssh_string _RSA_do_sign(const unsigned char *digest, - int dlen, - RSA *privkey) -{ - return _RSA_do_sign_hash(digest, dlen, privkey, SSH_DIGEST_AUTO); -} - static ssh_string pki_dsa_signature_to_blob(const ssh_signature sig) { char buffer[40] = { 0 }; @@ -1813,16 +1793,24 @@ ssh_signature pki_do_sign_hash(const ssh_key privkey, } #ifdef WITH_SERVER -ssh_signature pki_do_sign_sessionid(const ssh_key key, - const unsigned char *hash, - size_t hlen) +ssh_signature pki_do_sign_sessionid_hash(const ssh_key key, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type) { ssh_signature sig; + /* Only RSA supports different signature algorithm types now */ + if (key->type != SSH_KEYTYPE_RSA && hash_type != SSH_DIGEST_AUTO) { + SSH_LOG(SSH_LOG_WARN, "Incompatible signature algorithm passed"); + return NULL; + } + sig = ssh_signature_new(); if (sig == NULL) { return NULL; } + sig->type = key->type; sig->type_c = key->type_c; @@ -1836,7 +1824,8 @@ ssh_signature pki_do_sign_sessionid(const ssh_key key, break; case SSH_KEYTYPE_RSA: case SSH_KEYTYPE_RSA1: - sig->rsa_sig = _RSA_do_sign(hash, hlen, key->rsa); + sig->type_c = ssh_key_signature_to_char(key->type, hash_type); + sig->rsa_sig = _RSA_do_sign_hash(hash, hlen, key->rsa, hash_type); if (sig->rsa_sig == NULL) { ssh_signature_free(sig); return NULL; diff --git a/src/pki_gcrypt.c b/src/pki_gcrypt.c index c0395e9d..a988d805 100644 --- a/src/pki_gcrypt.c +++ b/src/pki_gcrypt.c @@ -2199,19 +2199,28 @@ ssh_signature pki_do_sign_hash(const ssh_key privkey, } #ifdef WITH_SERVER -ssh_signature pki_do_sign_sessionid(const ssh_key key, - const unsigned char *hash, - size_t hlen) +ssh_signature pki_do_sign_sessionid_hash(const ssh_key key, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type) { unsigned char ghash[hlen + 1]; + const char *hash_c = NULL; ssh_signature sig; gcry_sexp_t sexp; gcry_error_t err; + /* Only RSA supports different signature algorithm types now */ + if (key->type != SSH_KEYTYPE_RSA && hash_type != SSH_DIGEST_AUTO) { + SSH_LOG(SSH_LOG_WARN, "Incompatible signature algorithm passed"); + return NULL; + } + sig = ssh_signature_new(); if (sig == NULL) { return NULL; } + sig->type = key->type; sig->type_c = key->type_c; @@ -2238,9 +2247,25 @@ ssh_signature pki_do_sign_sessionid(const ssh_key key, } break; case SSH_KEYTYPE_RSA: + sig->type_c = ssh_key_signature_to_char(key->type, hash_type); + switch (hash_type) { + case SSH_DIGEST_SHA1: + hash_c = "sha1"; + break; + case SSH_DIGEST_SHA256: + hash_c = "sha256"; + break; + case SSH_DIGEST_SHA512: + hash_c = "sha512"; + break; + default: + SSH_LOG(SSH_LOG_WARN, "Incomplatible key algorithm"); + return NULL; + } err = gcry_sexp_build(&sexp, NULL, - "(data(flags pkcs1)(hash sha1 %b))", + "(data(flags pkcs1)(hash %s %b))", + hash_c, hlen, hash); if (err) { diff --git a/src/pki_mbedcrypto.c b/src/pki_mbedcrypto.c index 68a80b42..fceacd80 100644 --- a/src/pki_mbedcrypto.c +++ b/src/pki_mbedcrypto.c @@ -1127,22 +1127,32 @@ ssh_signature pki_do_sign_hash(const ssh_key privkey, } #ifdef WITH_SERVER -ssh_signature pki_do_sign_sessionid(const ssh_key key, const unsigned char - *hash, size_t hlen) +ssh_signature pki_do_sign_sessionid_hash(const ssh_key key, + const unsigned char *hash, + size_t hlen, + enum ssh_digest_e hash_type) { ssh_signature sig = NULL; int rc; + /* Only RSA supports different signature algorithm types now */ + if (key->type != SSH_KEYTYPE_RSA && hash_type != SSH_DIGEST_AUTO) { + SSH_LOG(SSH_LOG_WARN, "Incompatible signature algorithm passed"); + return NULL; + } + sig = ssh_signature_new(); if (sig == NULL) { return NULL; } + sig->type = key->type; sig->type_c = key->type_c; switch (key->type) { case SSH_KEYTYPE_RSA: - sig->rsa_sig = rsa_do_sign_hash(hash, hlen, key->rsa, SSH_DIGEST_AUTO); + sig->type_c = ssh_key_signature_to_char(key->type, hash_type); + sig->rsa_sig = rsa_do_sign_hash(hash, hlen, key->rsa, hash_type); if (sig->rsa_sig == NULL) { ssh_signature_free(sig); return NULL; diff --git a/src/wrapper.c b/src/wrapper.c index 48749489..68707989 100644 --- a/src/wrapper.c +++ b/src/wrapper.c @@ -509,7 +509,7 @@ int crypt_set_algorithms_server(ssh_session session){ } method = session->next_crypto->kex_methods[SSH_HOSTKEYS]; - session->srv.hostkey = ssh_key_type_from_name(method); + session->srv.hostkey = ssh_key_type_from_signature_name(method); return SSH_OK; } diff --git a/tests/pkd/pkd_client.h b/tests/pkd/pkd_client.h index da40a7c5..4d01a607 100644 --- a/tests/pkd/pkd_client.h +++ b/tests/pkd/pkd_client.h @@ -15,8 +15,8 @@ #define OPENSSH_BINARY "ssh" #define OPENSSH_KEYGEN "ssh-keygen" -#define OPENSSH_HOSTKEY_ALGOS_DEFAULT "ssh-ed25519,ssh-rsa" -#define OPENSSH_PKACCEPTED_DEFAULT "ssh-ed25519,ssh-rsa" +#define OPENSSH_HOSTKEY_ALGOS_DEFAULT "ssh-ed25519,rsa-sha2-512,rsa-sha2-256,ssh-rsa" +#define OPENSSH_PKACCEPTED_DEFAULT "ssh-ed25519,rsa-sha2-512,rsa-sha2-256,ssh-rsa" #if HAVE_ECC #define OPENSSH_HOSTKEY_ALGOS_ECDSA ",ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521" -- 2.17.1
Re: (Client side) RSA signatures with SHA2 (RFC 8332 and RFC 8308) | Andreas Schneider <asn@xxxxxxxxxxxxxx> |