Hi, Attached is a fix for https://red.libssh.org/issues/144; more details are in the commit message.
Thanks, -Jon
>From bb5099845894d7afb9e7b7aa2f14d78c79fcc7f2 Mon Sep 17 00:00:00 2001 From: Jon Simons <[email protected]> Date: Tue, 21 Jan 2014 23:36:08 -0800 Subject: [PATCH] pki_crypto: fix DSA signature extraction Fix the DSA portion of 'pki_signature_to_blob': before this change, it is possible to sometimes observe DSA signature validation failure when testing with OpenSSH clients. The problem ended up being the following snippet which did not account for the case when 'ssh_string_len(x)' may be less than 20: r = make_bignum_string(sig->dsa_sig->r); ... memcpy(buffer, ((char *) ssh_string_data(r)) + ssh_string_len(r) - 20, 20); Above consider the case that ssh_string_len(r) is 19; in that case the memcpy unintentionally starts in the wrong place. The same situation can happen for value 's' in this code. To fix, adjust the offsets used for the input and output pointers, taking into account that the lengths of 'r' and 's' can be less than 20. With the fix I am no longer able to reproduce the original failure mode. BUG: https://red.libssh.org/issues/144 --- src/pki_crypto.c | 76 +++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/src/pki_crypto.c b/src/pki_crypto.c index 85471f9..435a893 100644 --- a/src/pki_crypto.c +++ b/src/pki_crypto.c @@ -1094,41 +1094,65 @@ static ssh_string _RSA_do_sign(const unsigned char *digest, return sig_blob; } -ssh_string pki_signature_to_blob(const ssh_signature sig) +static ssh_string pki_dsa_signature_to_blob(const ssh_signature sig) { - char buffer[40] = {0}; + char buffer[40] = { 0 }; ssh_string sig_blob = NULL; + ssh_string r; + int r_len, r_offset_in, r_offset_out; + ssh_string s; + int s_len, s_offset_in, s_offset_out; - switch(sig->type) { - case SSH_KEYTYPE_DSS: - r = make_bignum_string(sig->dsa_sig->r); - if (r == NULL) { - return NULL; - } - s = make_bignum_string(sig->dsa_sig->s); - if (s == NULL) { - ssh_string_free(r); - return NULL; - } + r = make_bignum_string(sig->dsa_sig->r); + if (r == NULL) { + return NULL; + } - memcpy(buffer, - ((char *)ssh_string_data(r)) + ssh_string_len(r) - 20, - 20); - memcpy(buffer + 20, - ((char *)ssh_string_data(s)) + ssh_string_len(s) - 20, - 20); + s = make_bignum_string(sig->dsa_sig->s); + if (s == NULL) { + ssh_string_free(r); + return NULL; + } - ssh_string_free(r); - ssh_string_free(s); + r_len = ssh_string_len(r); + r_offset_in = (r_len > 20) ? (r_len - 20) : 0; + r_offset_out = (r_len < 20) ? (20 - r_len) : 0; - sig_blob = ssh_string_new(40); - if (sig_blob == NULL) { - return NULL; - } + s_len = ssh_string_len(s); + s_offset_in = (s_len > 20) ? (s_len - 20) : 0; + s_offset_out = (s_len < 20) ? (20 - s_len) : 0; + + memcpy(buffer + r_offset_out, + ((char *)ssh_string_data(r)) + r_offset_in, + r_len - r_offset_in); + memcpy(buffer + 20 + s_offset_out, + ((char *)ssh_string_data(s)) + s_offset_in, + s_len - s_offset_in); + + ssh_string_free(r); + ssh_string_free(s); + + sig_blob = ssh_string_new(40); + if (sig_blob == NULL) { + return NULL; + } + + ssh_string_fill(sig_blob, buffer, 40); + + return sig_blob; +} + +ssh_string pki_signature_to_blob(const ssh_signature sig) +{ + ssh_string r; + ssh_string s; + ssh_string sig_blob = NULL; - ssh_string_fill(sig_blob, buffer, 40); + switch(sig->type) { + case SSH_KEYTYPE_DSS: + sig_blob = pki_dsa_signature_to_blob(sig); break; case SSH_KEYTYPE_RSA: case SSH_KEYTYPE_RSA1: -- 1.8.4.21.g992c386
