aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDirkjan Bussink <d.bussink@gmail.com>2020-09-29 15:14:09 +0200
committerJakub Jelen <jjelen@redhat.com>2021-08-17 15:46:53 +0200
commit36e56dcd9392b342bb98691a64afe1947b0eec98 (patch)
treef76e6aeabb90b17f754b55ffe45933eaffb8539d
parentf834e10a478c68cb18b6f90d7a890d8f952e4339 (diff)
downloadlibssh-36e56dcd9392b342bb98691a64afe1947b0eec98.tar.gz
libssh-36e56dcd9392b342bb98691a64afe1947b0eec98.tar.xz
libssh-36e56dcd9392b342bb98691a64afe1947b0eec98.zip
Fix handshake bug with AEAD ciphers and no HMAC overlap
There's currently a bug in libssh that a handshake doesn't complete if there is no overlap between HMAC methods, but when an AEAD cipher is used. In case of an AEAD cipher such as chacha20-poly1305 or aes256-gcm, the HMAC algorithm that is being picked is not relevant. But the problem here is that the HMAC still needs to have an overlap in the handshake, even if it is not used afterwards. This was found with a very strict server side configuration with libssh where only AEAD ciphers and EtM HMAC modes are accepted. The client tested against was dropbear. Dropbear does have support for chacha20-poly1305 and AES GCM modes, but no support for EtM HMAC modes. This meant that the libssh server in this case rejected the dropbear client, even though it is perfectly able to serve it since dropbear supports AEAD algorithms. The fix implemented here updates the HMAC phase of the handshake to handle this case. If it detects an AEAD cipher is used, it uses the HMAC abbreviations for the method instead. This is the same name that is used in other places as well. It matches the client to server and server to client values, but it does depend on the order of things in the ssh_kex_types_e enum, which I'm assuming here is ok since it's explicit. I've looked at how to add a test for this, but I couldn't really find a suitable place for it. I would love some tips if this is easily possible, or if it's easier for someone else to contribute, that's of course welcome too. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com> (cherry picked from commit 42741b18832aa8acab51c53505efa263e8193537)
-rw-r--r--src/kex.c57
1 files changed, 41 insertions, 16 deletions
diff --git a/src/kex.c b/src/kex.c
index 80b6e8ad..c2ca77f1 100644
--- a/src/kex.c
+++ b/src/kex.c
@@ -735,13 +735,29 @@ int ssh_set_client_kex(ssh_session session)
return SSH_OK;
}
+static const char *ssh_find_aead_hmac(const char *cipher)
+{
+ if (cipher == NULL) {
+ return NULL;
+ } else if (strcmp(cipher, "chacha20-poly1305@openssh.com") == 0) {
+ return "aead-poly1305";
+ } else if (strcmp(cipher, "aes256-gcm@openssh.com") == 0) {
+ return "aead-gcm";
+ } else if (strcmp(cipher, "aes128-gcm@openssh.com") == 0) {
+ return "aead-gcm";
+ }
+ return NULL;
+}
+
/** @brief Select the different methods on basis of client's and
* server's kex messages, and watches out if a match is possible.
*/
-int ssh_kex_select_methods (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;
+ const char *aead_hmac = NULL;
int i;
/* Here we should drop the ext-info-c from the list so we avoid matching.
@@ -753,7 +769,15 @@ int ssh_kex_select_methods (ssh_session session){
for (i = 0; i < SSH_KEX_METHODS; 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){
+
+ if (i == SSH_MAC_C_S || i == SSH_MAC_S_C) {
+ aead_hmac = ssh_find_aead_hmac(session->next_crypto->kex_methods[i-2]);
+ if (aead_hmac) {
+ free(session->next_crypto->kex_methods[i]);
+ session->next_crypto->kex_methods[i] = strdup(aead_hmac);
+ }
+ }
+ if (session->next_crypto->kex_methods[i] == NULL && i < SSH_LANG_C_S){
ssh_set_error(session,SSH_FATAL,"kex error : no match for method %s: server [%s], client [%s]",
ssh_kex_descriptions[i],server->methods[i],client->methods[i]);
return SSH_ERROR;
@@ -762,31 +786,31 @@ int ssh_kex_select_methods (ssh_session session){
session->next_crypto->kex_methods[i] = strdup("");
}
}
- if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group1-sha1") == 0){
+ if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group1-sha1") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP1_SHA1;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha1") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha1") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP14_SHA1;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha256") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha256") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP14_SHA256;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group16-sha512") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group16-sha512") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP16_SHA512;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group18-sha512") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group18-sha512") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP18_SHA512;
#ifdef WITH_GEX
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha1") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha1") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GEX_SHA1;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha256") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha256") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GEX_SHA256;
#endif /* WITH_GEX */
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp256") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp256") == 0){
session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP256;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp384") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp384") == 0){
session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP384;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp521") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp521") == 0){
session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP521;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256@libssh.org") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256@libssh.org") == 0){
session->next_crypto->kex_type=SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG;
- } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256") == 0){
+ } else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256") == 0){
session->next_crypto->kex_type=SSH_KEX_CURVE25519_SHA256;
}
SSH_LOG(SSH_LOG_INFO, "Negotiated %s,%s,%s,%s,%s,%s,%s,%s,%s,%s",
@@ -806,7 +830,8 @@ int ssh_kex_select_methods (ssh_session session){
/* this function only sends the predefined set of kex methods */
-int ssh_send_kex(ssh_session session, int server_kex) {
+int ssh_send_kex(ssh_session session, int server_kex)
+{
struct ssh_kex_struct *kex = (server_kex ? &session->next_crypto->server_kex :
&session->next_crypto->client_kex);
ssh_string str = NULL;
@@ -1023,7 +1048,7 @@ int ssh_make_sessionid(ssh_session session)
ssh_buffer_get(server_hash),
server_pubkey_blob);
SSH_STRING_FREE(server_pubkey_blob);
- if(rc != SSH_OK){
+ if (rc != SSH_OK){
goto error;
}