From 73c2bb7a12d124619f664890b6108d836c2dc2f5 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Wed, 15 Jun 2016 21:49:02 +0200 Subject: [PATCH 2/2] cert_to_ssh_key: properly add leading 0 to bignums In the ssh keys a leading 0 is added to the bignums of the RSA modulus and exponent if the leading bit is set to avoid the interpretation as a negative number. --- src/util/cert/nss/cert.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/util/cert/nss/cert.c b/src/util/cert/nss/cert.c index 851378379f4e508824368347aa7a7e079d6cf8e1..2e3fe45743b4175bbc0b2950aca24954041930bd 100644 --- a/src/util/cert/nss/cert.c +++ b/src/util/cert/nss/cert.c @@ -267,6 +267,8 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, parameters.length = sizeof (parameters); SECStatus rv; SECStatus rv_verify; + size_t exponent_prefix_len; + size_t modulus_prefix_len; if (der_blob == NULL || der_size == 0) { return EINVAL; @@ -369,10 +371,21 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, goto done; } + /* Looks like nss drops the leading 00 which afaik is added to make sure + * the bigint is handled as positive number if the leading bit is set. */ + exponent_prefix_len = 0; + if (cert_pub_key->u.rsa.publicExponent.data[0] & 0x80) { + exponent_prefix_len = 1; + } + + modulus_prefix_len = 0; + if (cert_pub_key->u.rsa.modulus.data[0] & 0x80) { + modulus_prefix_len = 1; + } size = SSH_RSA_HEADER_LEN + 3 * sizeof(uint32_t) + cert_pub_key->u.rsa.modulus.len + cert_pub_key->u.rsa.publicExponent.len - + 1; /* see comment about missing 00 below */ + + exponent_prefix_len + modulus_prefix_len; buf = talloc_size(mem_ctx, size); if (buf == NULL) { @@ -386,17 +399,20 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, SAFEALIGN_SET_UINT32(buf, htobe32(SSH_RSA_HEADER_LEN), &c); safealign_memcpy(&buf[c], SSH_RSA_HEADER, SSH_RSA_HEADER_LEN, &c); SAFEALIGN_SET_UINT32(&buf[c], - htobe32(cert_pub_key->u.rsa.publicExponent.len), &c); + htobe32(cert_pub_key->u.rsa.publicExponent.len + + exponent_prefix_len), &c); + if (exponent_prefix_len == 1) { + SAFEALIGN_SETMEM_VALUE(&buf[c], '\0', unsigned char, &c); + } safealign_memcpy(&buf[c], cert_pub_key->u.rsa.publicExponent.data, cert_pub_key->u.rsa.publicExponent.len, &c); - /* Looks like nss drops the leading 00 which afaik is added to make sure - * the bigint is handled as positive number */ - /* TODO: make a better check if 00 must be added or not, e.g. ... & 0x80) - */ SAFEALIGN_SET_UINT32(&buf[c], - htobe32(cert_pub_key->u.rsa.modulus.len + 1 ), &c); - SAFEALIGN_SETMEM_VALUE(&buf[c], '\0', unsigned char, &c); + htobe32(cert_pub_key->u.rsa.modulus.len + + modulus_prefix_len ), &c); + if (modulus_prefix_len == 1) { + SAFEALIGN_SETMEM_VALUE(&buf[c], '\0', unsigned char, &c); + } safealign_memcpy(&buf[c], cert_pub_key->u.rsa.modulus.data, cert_pub_key->u.rsa.modulus.len, &c); -- 2.1.0