From 78e9a85355f6b89997662a6901de3d432b82cc16 Mon Sep 17 00:00:00 2001 From: djm Date: Wed, 14 Jan 2015 15:02:39 +0000 Subject: [PATCH] avoid BIGNUM in KRL code by using a simple bitmap; feedback and ok markus --- usr.bin/ssh/bitmap.c | 387 +++++++++++++++++++++++++++++ usr.bin/ssh/bitmap.h | 56 +++++ usr.bin/ssh/krl.c | 62 +++-- usr.bin/ssh/lib/Makefile | 9 +- usr.bin/ssh/sshbuf-getput-basic.c | 38 ++- usr.bin/ssh/sshbuf-getput-crypto.c | 18 +- usr.bin/ssh/sshbuf.h | 4 +- 7 files changed, 531 insertions(+), 43 deletions(-) create mode 100644 usr.bin/ssh/bitmap.c create mode 100644 usr.bin/ssh/bitmap.h diff --git a/usr.bin/ssh/bitmap.c b/usr.bin/ssh/bitmap.c new file mode 100644 index 00000000000..cd9df8c77e6 --- /dev/null +++ b/usr.bin/ssh/bitmap.c @@ -0,0 +1,387 @@ +/* + * Copyright (c) 2015 Damien Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include +#include +#include + +#include "bitmap.h" + +#define BITMAP_WTYPE u_int +#define BITMAP_MAX (1<<24) +#define BITMAP_BYTES (sizeof(BITMAP_WTYPE)) +#define BITMAP_BITS (sizeof(BITMAP_WTYPE) * 8) +#define BITMAP_WMASK ((BITMAP_WTYPE)BITMAP_BITS - 1) +struct bitmap { + BITMAP_WTYPE *d; + size_t len; /* number of words allocated */ + size_t top; /* index of top word allocated */ +}; + +struct bitmap * +bitmap_new(void) +{ + struct bitmap *ret; + + if ((ret = calloc(1, sizeof(*ret))) == NULL) + return NULL; + if ((ret->d = calloc(1, BITMAP_BYTES)) == NULL) { + free(ret); + return NULL; + } + ret->len = 1; + ret->top = 0; + return ret; +} + +void +bitmap_free(struct bitmap *b) +{ + if (b != NULL && b->d != NULL) { + memset(b->d, 0, b->len); + free(b->d); + } + free(b); +} + +void +bitmap_zero(struct bitmap *b) +{ + memset(b->d, 0, b->len * BITMAP_BYTES); + b->top = 0; +} + +int +bitmap_test_bit(struct bitmap *b, u_int n) +{ + if (b->top >= b->len) + return 0; /* invalid */ + if (b->len == 0 || (n / BITMAP_BITS) > b->top) + return 0; + return (b->d[n / BITMAP_BITS] >> (n & BITMAP_WMASK)) & 1; +} + +static int +reserve(struct bitmap *b, u_int n) +{ + BITMAP_WTYPE *tmp; + size_t nlen; + + if (b->top >= b->len || n > BITMAP_MAX) + return -1; /* invalid */ + nlen = (n / BITMAP_BITS) + 1; + if (b->len < nlen) { + if ((tmp = reallocarray(b->d, nlen, BITMAP_BYTES)) == NULL) + return -1; + b->d = tmp; + memset(b->d + b->len, 0, (nlen - b->len) * BITMAP_BYTES); + b->len = nlen; + } + return 0; +} + +int +bitmap_set_bit(struct bitmap *b, u_int n) +{ + int r; + size_t offset; + + if ((r = reserve(b, n)) != 0) + return r; + offset = n / BITMAP_BITS; + if (offset > b->top) + b->top = offset; + b->d[offset] |= (BITMAP_WTYPE)1 << (n & BITMAP_WMASK); + return 0; +} + +/* Resets b->top to point to the most significant bit set in b->d */ +static void +retop(struct bitmap *b) +{ + if (b->top >= b->len) + return; + while (b->top > 0 && b->d[b->top] == 0) + b->top--; +} + +void +bitmap_clear_bit(struct bitmap *b, u_int n) +{ + size_t offset; + + if (b->top >= b->len || n > BITMAP_MAX) + return; /* invalid */ + offset = n / BITMAP_BITS; + if (offset > b->top) + return; + b->d[offset] &= ~((BITMAP_WTYPE)1 << (n & BITMAP_WMASK)); + /* The top may have changed as a result of the clear */ + retop(b); +} + +size_t +bitmap_nbits(struct bitmap *b) +{ + size_t bits; + BITMAP_WTYPE w; + + retop(b); + if (b->top >= b->len) + return 0; /* invalid */ + if (b->len == 0 || (b->top == 0 && b->d[0] == 0)) + return 0; + /* Find MSB set */ + w = b->d[b->top]; + bits = (b->top + 1) * BITMAP_BITS; + while (!(w & ((BITMAP_WTYPE)1 << (BITMAP_BITS - 1)))) { + w <<= 1; + bits--; + } + return bits; + +} + +size_t +bitmap_nbytes(struct bitmap *b) +{ + return (bitmap_nbits(b) + 7) / 8; +} + +int +bitmap_to_string(struct bitmap *b, void *p, size_t l) +{ + u_char *s = (u_char *)p; + size_t i, j, k, need = bitmap_nbytes(b); + + if (l < need || b->top >= b->len) + return -1; + if (l > need) + l = need; + /* Put the bytes from LSB backwards */ + for (i = k = 0; i < b->top + 1; i++) { + for (j = 0; j < BITMAP_BYTES; j++) { + if (k >= l) + break; + s[need - 1 - k++] = (b->d[i] >> (j * 8)) & 0xff; + } + } + + return 0; +} + +int +bitmap_from_string(struct bitmap *b, const void *p, size_t l) +{ + int r; + size_t i, offset, shift; + u_char *s = (u_char *)p; + + if (l > BITMAP_MAX / 8) + return -1; + if ((r = reserve(b, l * 8)) != 0) + return r; + bitmap_zero(b); + if (l == 0) + return 0; + b->top = offset = ((l + (BITMAP_BYTES - 1)) / BITMAP_BYTES) - 1; + shift = ((l + (BITMAP_BYTES - 1)) % BITMAP_BYTES) * 8; + for (i = 0; i < l; i++) { + b->d[offset] |= (BITMAP_WTYPE)s[i] << shift; + if (shift == 0) { + offset--; + shift = BITMAP_BITS - 8; + } else + shift -= 8; + } + retop(b); + return 0; +} + +#ifdef BITMAP_TEST + +/* main() test against OpenSSL BN */ +#include +#include +#include +#include + +#define LIM 131 +#define FAIL(...) fail(__FILE__, __LINE__, __VA_ARGS__) + +void +bitmap_dump(struct bitmap *b, FILE *f) +{ + size_t i; + + fprintf(f, "bitmap %p len=%zu top=%zu d =", b, b->len, b->top); + for (i = 0; i < b->len; i++) { + fprintf(f, " %0*llx", (int)BITMAP_BITS / 4, + (unsigned long long)b->d[i]); + } + fputc('\n', f); +} + +static void +fail(char *file, int line, char *fmt, ...) +{ + va_list ap; + + fprintf(stderr, "%s:%d ", file, line); + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + fputc('\n', stdout); + /* abort(); */ + exit(1); +} + +static void +dump(const char *s, const u_char *buf, size_t l) +{ + size_t i; + + fprintf(stderr, "%s len %zu = ", s, l); + for (i = 0; i < l; i++) + fprintf(stderr, "%02x ", buf[i]); + fputc('\n', stderr); +} + +int main(void) { + struct bitmap *b = bitmap_new(); + BIGNUM *bn = BN_new(); + size_t len; + int i, j, k, n; + u_char bbuf[1024], bnbuf[1024]; + int r; + + for (i = -1; i < LIM; i++) { + fputc('i', stdout); + fflush(stdout); + for (j = -1; j < LIM; j++) { + for (k = -1; k < LIM; k++) { + bitmap_zero(b); + BN_clear(bn); + /* Test setting bits */ + if (i > 0 && bitmap_set_bit(b, i) != 0) + FAIL("bitmap_set_bit %d fail", i); + if (j > 0 && bitmap_set_bit(b, j) != 0) + FAIL("bitmap_set_bit %d fail", j); + if (k > 0 && bitmap_set_bit(b, k) != 0) + FAIL("bitmap_set_bit %d fail", k); + if ((i > 0 && BN_set_bit(bn, i) != 1) || + (j > 0 && BN_set_bit(bn, j) != 1) || + (k > 0 && BN_set_bit(bn, k) != 1)) + FAIL("BN_set_bit fail"); + for (n = 0; n < LIM; n++) { + if (BN_is_bit_set(bn, n) != + bitmap_test_bit(b, n)) { + FAIL("miss %d/%d/%d %d " + "%d %d", i, j, k, n, + BN_is_bit_set(bn, n), + bitmap_test_bit(b, n)); + } + } + /* Test length calculations */ + if (BN_num_bytes(bn) != (int)bitmap_nbytes(b)) { + FAIL("bytes %d/%d/%d %d != %zu", + i, j, k, BN_num_bytes(bn), + bitmap_nbytes(b)); + } + if (BN_num_bits(bn) != (int)bitmap_nbits(b)) { + FAIL("bits %d/%d/%d %d != %zu", + i, j, k, BN_num_bits(bn), + bitmap_nbits(b)); + } + /* Test serialisation */ + len = bitmap_nbytes(b); + memset(bbuf, 0xfc, sizeof(bbuf)); + if (bitmap_to_string(b, bbuf, + sizeof(bbuf)) != 0) + FAIL("bitmap_to_string %d/%d/%d", + i, j, k); + for (n = len; n < (int)sizeof(bbuf); n++) { + if (bbuf[n] != 0xfc) + FAIL("bad string " + "%d/%d/%d %d 0x%02x", + i, j, k, n, bbuf[n]); + } + if ((r = BN_bn2bin(bn, bnbuf)) < 0) + FAIL("BN_bn2bin %d/%d/%d", + i, j, k); + if ((size_t)r != len) + FAIL("len bad %d/%d/%d", i, j, k); + if (memcmp(bbuf, bnbuf, len) != 0) { + dump("bbuf", bbuf, sizeof(bbuf)); + dump("bnbuf", bnbuf, sizeof(bnbuf)); + FAIL("buf bad %d/%d/%d", i, j, k); + } + /* Test deserialisation */ + bitmap_zero(b); + if (bitmap_from_string(b, bnbuf, len) != 0) + FAIL("bitmap_to_string %d/%d/%d", + i, j, k); + for (n = 0; n < LIM; n++) { + if (BN_is_bit_set(bn, n) != + bitmap_test_bit(b, n)) { + FAIL("miss %d/%d/%d %d " + "%d %d", i, j, k, n, + BN_is_bit_set(bn, n), + bitmap_test_bit(b, n)); + } + } + /* Test clearing bits */ + for (n = 0; n < LIM; n++) { + if (bitmap_set_bit(b, n) != 0) { + bitmap_dump(b, stderr); + FAIL("bitmap_set_bit %d " + "fail", n); + } + if (BN_set_bit(bn, n) != 1) + FAIL("BN_set_bit fail"); + } + if (i > 0) { + bitmap_clear_bit(b, i); + BN_clear_bit(bn, i); + } + if (j > 0) { + bitmap_clear_bit(b, j); + BN_clear_bit(bn, j); + } + if (k > 0) { + bitmap_clear_bit(b, k); + BN_clear_bit(bn, k); + } + for (n = 0; n < LIM; n++) { + if (BN_is_bit_set(bn, n) != + bitmap_test_bit(b, n)) { + bitmap_dump(b, stderr); + FAIL("cmiss %d/%d/%d %d " + "%d %d", i, j, k, n, + BN_is_bit_set(bn, n), + bitmap_test_bit(b, n)); + } + } + } + } + } + fputc('\n', stdout); + bitmap_free(b); + BN_free(bn); + + return 0; +} +#endif diff --git a/usr.bin/ssh/bitmap.h b/usr.bin/ssh/bitmap.h new file mode 100644 index 00000000000..c1bb1741a4f --- /dev/null +++ b/usr.bin/ssh/bitmap.h @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2015 Damien Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#ifndef _BITMAP_H +#define _BITMAP_H + +#include + +/* Simple bit vector routines */ + +struct bitmap; + +/* Allocate a new bitmap. Returns NULL on allocation failure. */ +struct bitmap *bitmap_new(void); + +/* Free a bitmap */ +void bitmap_free(struct bitmap *b); + +/* Zero an existing bitmap */ +void bitmap_zero(struct bitmap *b); + +/* Test whether a bit is set in a bitmap. */ +int bitmap_test_bit(struct bitmap *b, u_int n); + +/* Set a bit in a bitmap. Returns 0 on success or -1 on error */ +int bitmap_set_bit(struct bitmap *b, u_int n); + +/* Clear a bit in a bitmap */ +void bitmap_clear_bit(struct bitmap *b, u_int n); + +/* Return the number of bits in a bitmap (i.e. the position of the MSB) */ +size_t bitmap_nbits(struct bitmap *b); + +/* Return the number of bytes needed to represent a bitmap */ +size_t bitmap_nbytes(struct bitmap *b); + +/* Convert a bitmap to a big endian byte string */ +int bitmap_to_string(struct bitmap *b, void *p, size_t l); + +/* Convert a big endian byte string to a bitmap */ +int bitmap_from_string(struct bitmap *b, const void *p, size_t l); + +#endif /* _BITMAP_H */ diff --git a/usr.bin/ssh/krl.c b/usr.bin/ssh/krl.c index ef8049df4f8..b90ffac39e7 100644 --- a/usr.bin/ssh/krl.c +++ b/usr.bin/ssh/krl.c @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* $OpenBSD: krl.c,v 1.25 2015/01/13 19:04:35 djm Exp $ */ +/* $OpenBSD: krl.c,v 1.26 2015/01/14 15:02:39 djm Exp $ */ #include #include @@ -35,6 +35,7 @@ #include "misc.h" #include "log.h" #include "digest.h" +#include "bitmap.h" #include "krl.h" @@ -517,6 +518,25 @@ choose_next_state(int current_state, u_int64_t contig, int final, return new_state; } +static int +put_bitmap(struct sshbuf *buf, struct bitmap *bitmap) +{ + size_t len; + u_char *blob; + int r; + + len = bitmap_nbytes(bitmap); + if ((blob = malloc(len)) == NULL) + return SSH_ERR_ALLOC_FAIL; + if (bitmap_to_string(bitmap, blob, len) != 0) { + free(blob); + return SSH_ERR_INTERNAL_ERROR; + } + r = sshbuf_put_bignum2_bytes(buf, blob, len); + free(blob); + return r; +} + /* Generate a KRL_SECTION_CERTIFICATES KRL section */ static int revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) @@ -527,7 +547,7 @@ revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) struct revoked_key_id *rki; int next_state, state = 0; struct sshbuf *sect; - BIGNUM *bitmap = NULL; + struct bitmap *bitmap = NULL; if ((sect = sshbuf_new()) == NULL) return SSH_ERR_ALLOC_FAIL; @@ -570,9 +590,9 @@ revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) case KRL_SECTION_CERT_SERIAL_RANGE: break; case KRL_SECTION_CERT_SERIAL_BITMAP: - if ((r = sshbuf_put_bignum2(sect, bitmap)) != 0) + if ((r = put_bitmap(sect, bitmap)) != 0) goto out; - BN_free(bitmap); + bitmap_free(bitmap); bitmap = NULL; break; } @@ -593,7 +613,7 @@ revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) case KRL_SECTION_CERT_SERIAL_RANGE: break; case KRL_SECTION_CERT_SERIAL_BITMAP: - if ((bitmap = BN_new()) == NULL) { + if ((bitmap = bitmap_new()) == NULL) { r = SSH_ERR_ALLOC_FAIL; goto out; } @@ -624,8 +644,8 @@ revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) goto out; } for (i = 0; i < contig; i++) { - if (BN_set_bit(bitmap, - rs->lo + i - bitmap_start) != 1) { + if (bitmap_set_bit(bitmap, + rs->lo + i - bitmap_start) != 0) { r = SSH_ERR_ALLOC_FAIL; goto out; } @@ -643,9 +663,9 @@ revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) case KRL_SECTION_CERT_SERIAL_RANGE: break; case KRL_SECTION_CERT_SERIAL_BITMAP: - if ((r = sshbuf_put_bignum2(sect, bitmap)) != 0) + if ((r = put_bitmap(sect, bitmap)) != 0) goto out; - BN_free(bitmap); + bitmap_free(bitmap); bitmap = NULL; break; } @@ -669,8 +689,7 @@ revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) } r = 0; out: - if (bitmap != NULL) - BN_free(bitmap); + bitmap_free(bitmap); sshbuf_free(sect); return r; } @@ -782,13 +801,13 @@ format_timestamp(u_int64_t timestamp, char *ts, size_t nts) static int parse_revoked_certs(struct sshbuf *buf, struct ssh_krl *krl) { - int r = SSH_ERR_INTERNAL_ERROR, nbits; + int r = SSH_ERR_INTERNAL_ERROR; u_char type; const u_char *blob; - size_t blen; + size_t blen, nbits; struct sshbuf *subsect = NULL; u_int64_t serial, serial_lo, serial_hi; - BIGNUM *bitmap = NULL; + struct bitmap *bitmap = NULL; char *key_id = NULL; struct sshkey *ca_key = NULL; @@ -832,31 +851,32 @@ parse_revoked_certs(struct sshbuf *buf, struct ssh_krl *krl) goto out; break; case KRL_SECTION_CERT_SERIAL_BITMAP: - if ((bitmap = BN_new()) == NULL) { + if ((bitmap = bitmap_new()) == NULL) { r = SSH_ERR_ALLOC_FAIL; goto out; } if ((r = sshbuf_get_u64(subsect, &serial_lo)) != 0 || - (r = sshbuf_get_bignum2(subsect, bitmap)) != 0) + (r = sshbuf_get_bignum2_bytes_direct(subsect, + &blob, &blen)) != 0) goto out; - if ((nbits = BN_num_bits(bitmap)) < 0) { - error("%s: bitmap bits < 0", __func__); + if (bitmap_from_string(bitmap, blob, blen) != 0) { r = SSH_ERR_INVALID_FORMAT; goto out; } + nbits = bitmap_nbits(bitmap); for (serial = 0; serial < (u_int64_t)nbits; serial++) { if (serial > 0 && serial_lo + serial == 0) { error("%s: bitmap wraps u64", __func__); r = SSH_ERR_INVALID_FORMAT; goto out; } - if (!BN_is_bit_set(bitmap, serial)) + if (!bitmap_test_bit(bitmap, serial)) continue; if ((r = ssh_krl_revoke_cert_by_serial(krl, ca_key, serial_lo + serial)) != 0) goto out; } - BN_free(bitmap); + bitmap_free(bitmap); bitmap = NULL; break; case KRL_SECTION_CERT_KEY_ID: @@ -886,7 +906,7 @@ parse_revoked_certs(struct sshbuf *buf, struct ssh_krl *krl) r = 0; out: if (bitmap != NULL) - BN_free(bitmap); + bitmap_free(bitmap); free(key_id); sshkey_free(ca_key); sshbuf_free(subsect); diff --git a/usr.bin/ssh/lib/Makefile b/usr.bin/ssh/lib/Makefile index c45f55f186b..ba2bd301ca6 100644 --- a/usr.bin/ssh/lib/Makefile +++ b/usr.bin/ssh/lib/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.79 2014/06/24 01:13:22 djm Exp $ +# $OpenBSD: Makefile,v 1.80 2015/01/14 15:02:39 djm Exp $ .PATH: ${.CURDIR}/.. .include "${.CURDIR}/../Makefile.inc" @@ -11,7 +11,9 @@ LIB_SRCS= \ sshbuf.c \ sshbuf-getput-basic.c \ sshbuf-misc.c \ - sshkey.c + sshkey.c \ + bitmap.c \ + krl.c .if (${OPENSSL:L} == "yes") LIB_SRCS+= sshbuf-getput-crypto.c digest-openssl.c @@ -34,8 +36,7 @@ SRCS= ${LIB_SRCS} \ .if (${OPENSSL:L} == "yes") SRCS+= bufec.c bufbn.c cipher-3des1.c cipher-bf1.c rsa.c \ ssh-dss.c ssh-rsa.c ssh-ecdsa.c dh.c kexdh.c kexgex.c kexecdh.c \ - kexdhc.c kexgexc.c kexecdhc.c ssh-pkcs11.c \ - krl.c + kexdhc.c kexgexc.c kexecdhc.c ssh-pkcs11.c .else SRCS+= rijndael.c cipher-aesctr.c .endif diff --git a/usr.bin/ssh/sshbuf-getput-basic.c b/usr.bin/ssh/sshbuf-getput-basic.c index bc2e2f9eb7b..304da2d5a58 100644 --- a/usr.bin/ssh/sshbuf-getput-basic.c +++ b/usr.bin/ssh/sshbuf-getput-basic.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshbuf-getput-basic.c,v 1.3 2015/01/12 15:18:07 djm Exp $ */ +/* $OpenBSD: sshbuf-getput-basic.c,v 1.4 2015/01/14 15:02:39 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -422,3 +422,39 @@ sshbuf_put_bignum2_bytes(struct sshbuf *buf, const void *v, size_t len) memcpy(d + 4 + prepend, s, len); return 0; } + +int +sshbuf_get_bignum2_bytes_direct(struct sshbuf *buf, + const u_char **valp, size_t *lenp) +{ + const u_char *d; + size_t len, olen; + int r; + + if ((r = sshbuf_peek_string_direct(buf, &d, &olen)) < 0) + return r; + len = olen; + /* Refuse negative (MSB set) bignums */ + if ((len != 0 && (*d & 0x80) != 0)) + return SSH_ERR_BIGNUM_IS_NEGATIVE; + /* Refuse overlong bignums, allow prepended \0 to avoid MSB set */ + if (len > SSHBUF_MAX_BIGNUM + 1 || + (len == SSHBUF_MAX_BIGNUM + 1 && *d != 0)) + return SSH_ERR_BIGNUM_TOO_LARGE; + /* Trim leading zeros */ + while (len > 0 && *d == 0x00) { + d++; + len--; + } + if (valp != 0) + *valp = d; + if (lenp != NULL) + *lenp = len; + if (sshbuf_consume(buf, olen + 4) != 0) { + /* Shouldn't happen */ + SSHBUF_DBG(("SSH_ERR_INTERNAL_ERROR")); + SSHBUF_ABORT(); + return SSH_ERR_INTERNAL_ERROR; + } + return 0; +} diff --git a/usr.bin/ssh/sshbuf-getput-crypto.c b/usr.bin/ssh/sshbuf-getput-crypto.c index f086cae06eb..aa2a9221d0b 100644 --- a/usr.bin/ssh/sshbuf-getput-crypto.c +++ b/usr.bin/ssh/sshbuf-getput-crypto.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshbuf-getput-crypto.c,v 1.3 2015/01/12 15:18:07 djm Exp $ */ +/* $OpenBSD: sshbuf-getput-crypto.c,v 1.4 2015/01/14 15:02:39 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -34,24 +34,10 @@ sshbuf_get_bignum2(struct sshbuf *buf, BIGNUM *v) size_t len; int r; - if ((r = sshbuf_peek_string_direct(buf, &d, &len)) < 0) + if ((r = sshbuf_get_bignum2_bytes_direct(buf, &d, &len)) != 0) return r; - /* Refuse negative (MSB set) bignums */ - if ((len != 0 && (*d & 0x80) != 0)) - return SSH_ERR_BIGNUM_IS_NEGATIVE; - /* Refuse overlong bignums, allow prepended \0 to avoid MSB set */ - if (len > SSHBUF_MAX_BIGNUM + 1 || - (len == SSHBUF_MAX_BIGNUM + 1 && *d != 0)) - return SSH_ERR_BIGNUM_TOO_LARGE; if (v != NULL && BN_bin2bn(d, len, v) == NULL) return SSH_ERR_ALLOC_FAIL; - /* Consume the string */ - if (sshbuf_get_string_direct(buf, NULL, NULL) != 0) { - /* Shouldn't happen */ - SSHBUF_DBG(("SSH_ERR_INTERNAL_ERROR")); - SSHBUF_ABORT(); - return SSH_ERR_INTERNAL_ERROR; - } return 0; } diff --git a/usr.bin/ssh/sshbuf.h b/usr.bin/ssh/sshbuf.h index 6f58b3ec11d..ce43e0af8c1 100644 --- a/usr.bin/ssh/sshbuf.h +++ b/usr.bin/ssh/sshbuf.h @@ -1,4 +1,4 @@ -/* $OpenBSD: sshbuf.h,v 1.3 2014/06/24 01:13:21 djm Exp $ */ +/* $OpenBSD: sshbuf.h,v 1.4 2015/01/14 15:02:39 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -206,6 +206,8 @@ int sshbuf_peek_string_direct(const struct sshbuf *buf, const u_char **valp, */ int sshbuf_get_bignum2(struct sshbuf *buf, BIGNUM *v); int sshbuf_get_bignum1(struct sshbuf *buf, BIGNUM *v); +int sshbuf_get_bignum2_bytes_direct(struct sshbuf *buf, + const u_char **valp, size_t *lenp); int sshbuf_put_bignum2(struct sshbuf *buf, const BIGNUM *v); int sshbuf_put_bignum1(struct sshbuf *buf, const BIGNUM *v); int sshbuf_put_bignum2_bytes(struct sshbuf *buf, const void *v, size_t len); -- 2.20.1