From 58ae048c92753e869ea7638bf3d30f4f4cd0ca3f Mon Sep 17 00:00:00 2001 From: Zenju Date: Tue, 11 May 2021 23:09:57 +0200 Subject: [PATCH] Fix detailed _libssh2_error being overwritten (#473) Files: openssl.c, pem.c, userauth.c Notes: * Fix detailed _libssh2_error being overwritten by generic errors * Unified error handling Credit: Zenju --- src/openssl.c | 68 +++++++++++++++++++++++--------------------------- src/pem.c | 31 +++++++++++++++-------- src/userauth.c | 9 +++---- 3 files changed, 55 insertions(+), 53 deletions(-) diff --git a/src/openssl.c b/src/openssl.c index 4d0f53cf..2ea9e1ba 100644 --- a/src/openssl.c +++ b/src/openssl.c @@ -1690,6 +1690,8 @@ gen_publickey_from_ed25519_openssh_priv_data(LIBSSH2_SESSION *session, method_buf = LIBSSH2_ALLOC(session, 11); /* ssh-ed25519. */ if(method_buf == NULL) { + _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for ED25519 key"); goto clean_exit; } @@ -1698,6 +1700,8 @@ gen_publickey_from_ed25519_openssh_priv_data(LIBSSH2_SESSION *session, key_len = LIBSSH2_ED25519_KEY_LEN + 19; key = LIBSSH2_CALLOC(session, key_len); if(key == NULL) { + _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for ED25519 key"); goto clean_exit; } @@ -2418,6 +2422,7 @@ gen_publickey_from_ecdsa_openssh_priv_data(LIBSSH2_SESSION *session, if((rc = _libssh2_ecdsa_curve_name_with_octal_new(&ec_key, point_buf, pointlen, curve_type)) != 0) { + rc = -1; _libssh2_error(session, LIBSSH2_ERROR_PROTO, "ECDSA could not create key"); goto fail; @@ -2426,6 +2431,8 @@ gen_publickey_from_ecdsa_openssh_priv_data(LIBSSH2_SESSION *session, bn_exponent = BN_new(); if(bn_exponent == NULL) { rc = -1; + _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for private key data"); goto fail; } @@ -2452,15 +2459,10 @@ gen_publickey_from_ecdsa_openssh_priv_data(LIBSSH2_SESSION *session, return rc; fail: - if(ec_key != NULL) EC_KEY_free(ec_key); - return _libssh2_error(session, - LIBSSH2_ERROR_ALLOC, - "Unable to allocate memory for private key data"); - - + return rc; } static int @@ -3053,17 +3055,13 @@ _libssh2_pub_priv_openssh_keyfilememory(LIBSSH2_SESSION *session, if(key_ctx != NULL) *key_ctx = NULL; - if(session == NULL) { - _libssh2_error(session, LIBSSH2_ERROR_PROTO, - "Session is required"); - return -1; - } + if(session == NULL) + return _libssh2_error(session, LIBSSH2_ERROR_PROTO, + "Session is required"); - if(key_type != NULL && (strlen(key_type) > 11 || strlen(key_type) < 7)) { - _libssh2_error(session, LIBSSH2_ERROR_PROTO, - "type is invalid"); - return -1; - } + if(key_type != NULL && (strlen(key_type) > 11 || strlen(key_type) < 7)) + return _libssh2_error(session, LIBSSH2_ERROR_PROTO, + "type is invalid"); _libssh2_init_if_needed(); @@ -3071,20 +3069,18 @@ _libssh2_pub_priv_openssh_keyfilememory(LIBSSH2_SESSION *session, privatekeydata, privatekeydata_len, &decrypted); - if(rc) { + if(rc) return rc; - } /* We have a new key file, now try and parse it using supported types */ rc = _libssh2_get_string(decrypted, &buf, NULL); - if(rc != 0 || buf == NULL) { - _libssh2_error(session, LIBSSH2_ERROR_PROTO, - "Public key type in decrypted key data not found"); - return -1; - } + if(rc != 0 || buf == NULL) + return _libssh2_error(session, LIBSSH2_ERROR_PROTO, + "Public key type in decrypted " + "key data not found"); - rc = -1; + rc = LIBSSH2_ERROR_FILE; #if LIBSSH2_ED25519 if(strcmp("ssh-ed25519", (const char *)buf) == 0) { @@ -3138,6 +3134,11 @@ _libssh2_pub_priv_openssh_keyfilememory(LIBSSH2_SESSION *session, } #endif + if(rc == LIBSSH2_ERROR_FILE) + rc = _libssh2_error(session, LIBSSH2_ERROR_FILE, + "Unable to extract public key from private key file: " + "invalid/unrecognized private key file format"); + if(decrypted) _libssh2_string_buf_free(session, decrypted); @@ -3177,10 +3178,10 @@ _libssh2_pub_priv_keyfilememory(LIBSSH2_SESSION *session, "Computing public key from private key."); bp = BIO_new_mem_buf((char *)privatekeydata, privatekeydata_len); - if(!bp) { - return -1; - } - + if(!bp) + return _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory when" + "computing public key"); BIO_reset(bp); pk = PEM_read_bio_PrivateKey(bp, NULL, NULL, (void *)passphrase); BIO_free(bp); @@ -3195,15 +3196,8 @@ _libssh2_pub_priv_keyfilememory(LIBSSH2_SESSION *session, privatekeydata, privatekeydata_len, (unsigned const char *)passphrase); - if(st != 0) { - return _libssh2_error(session, - LIBSSH2_ERROR_FILE, - "Unable to extract public key " - "from private key file: " - "Wrong passphrase or invalid/unrecognized " - "private key file format"); - } - + if(st != 0) + return st; return 0; } diff --git a/src/pem.c b/src/pem.c index 53f58c2e..3416bd52 100644 --- a/src/pem.c +++ b/src/pem.c @@ -176,6 +176,8 @@ _libssh2_pem_parse(LIBSSH2_SESSION * session, linelen = strlen(line); tmp = LIBSSH2_REALLOC(session, b64data, b64datalen + linelen); if(!tmp) { + _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for PEM parsing"); ret = -1; goto out; } @@ -319,6 +321,8 @@ _libssh2_pem_parse_memory(LIBSSH2_SESSION * session, linelen = strlen(line); tmp = LIBSSH2_REALLOC(session, b64data, b64datalen + linelen); if(!tmp) { + _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for PEM parsing"); ret = -1; goto out; } @@ -690,6 +694,8 @@ _libssh2_openssh_pem_parse(LIBSSH2_SESSION * session, linelen = strlen(line); tmp = LIBSSH2_REALLOC(session, b64data, b64datalen + linelen); if(!tmp) { + _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for PEM parsing"); ret = -1; goto out; } @@ -738,17 +744,17 @@ _libssh2_openssh_pem_parse_memory(LIBSSH2_SESSION * session, size_t off = 0; int ret; - if(filedata == NULL || filedata_len <= 0) { - return -1; - } + if(filedata == NULL || filedata_len <= 0) + return _libssh2_error(session, LIBSSH2_ERROR_PROTO, + "Error parsing PEM: filedata missing"); do { *line = '\0'; - if(off >= filedata_len) { - return -1; - } + if(off >= filedata_len) + return _libssh2_error(session, LIBSSH2_ERROR_PROTO, + "Error parsing PEM: offset out of bounds"); if(readline_memory(line, LINE_SIZE, filedata, filedata_len, &off)) { return -1; @@ -766,7 +772,9 @@ _libssh2_openssh_pem_parse_memory(LIBSSH2_SESSION * session, linelen = strlen(line); tmp = LIBSSH2_REALLOC(session, b64data, b64datalen + linelen); if(!tmp) { - ret = -1; + ret = _libssh2_error(session, LIBSSH2_ERROR_ALLOC, + "Unable to allocate memory for " + "PEM parsing"); goto out; } memcpy(tmp + b64datalen, line, linelen); @@ -777,7 +785,8 @@ _libssh2_openssh_pem_parse_memory(LIBSSH2_SESSION * session, *line = '\0'; if(off >= filedata_len) { - ret = -1; + ret = _libssh2_error(session, LIBSSH2_ERROR_PROTO, + "Error parsing PEM: offset out of bounds"); goto out; } @@ -787,9 +796,9 @@ _libssh2_openssh_pem_parse_memory(LIBSSH2_SESSION * session, } } while(strcmp(line, OPENSSH_HEADER_END) != 0); - if(!b64data) { - return -1; - } + if(!b64data) + return _libssh2_error(session, LIBSSH2_ERROR_PROTO, + "Error parsing PEM: base 64 data missing"); ret = _libssh2_openssh_pem_parse_data(session, passphrase, b64data, b64datalen, decrypted_buf); diff --git a/src/userauth.c b/src/userauth.c index df7663fe..5f669594 100644 --- a/src/userauth.c +++ b/src/userauth.c @@ -1447,15 +1447,14 @@ userauth_publickey_frommemory(LIBSSH2_SESSION *session, } else if(privatekeydata_len && privatekeydata) { /* Compute public key from private key. */ - if(_libssh2_pub_priv_keyfilememory(session, + rc = _libssh2_pub_priv_keyfilememory(session, &session->userauth_pblc_method, &session->userauth_pblc_method_len, &pubkeydata, &pubkeydata_len, privatekeydata, privatekeydata_len, - passphrase)) - return _libssh2_error(session, LIBSSH2_ERROR_FILE, - "Unable to extract public key " - "from private key."); + passphrase); + if(rc) + return rc; } else { return _libssh2_error(session, LIBSSH2_ERROR_FILE,