From b305993cea6325e7e428ce43f203d129c90ab131 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Thu, 4 Oct 2012 15:25:36 +1000 Subject: [PATCH] deprecate the buffer_init() sshbuf entrypoint. Allows making struct sshbuf opaque --- ssh/sshbuf-getput-crypto.c | 39 +++++++++++++++++++++++---------- ssh/sshbuf.c | 40 ++++++++++++++++++---------------- ssh/sshbuf.h | 29 +++++------------------- unittests/sshbuf/test_sshbuf.c | 2 +- 4 files changed, 56 insertions(+), 54 deletions(-) diff --git a/ssh/sshbuf-getput-crypto.c b/ssh/sshbuf-getput-crypto.c index aae77c6..64d06d5 100644 --- a/ssh/sshbuf-getput-crypto.c +++ b/ssh/sshbuf-getput-crypto.c @@ -79,6 +79,20 @@ sshbuf_get_bignum1(struct sshbuf *buf, BIGNUM *v) return 0; } +static int +get_ec(const u_char *d, size_t len, EC_POINT *v, const EC_GROUP *g) +{ + /* Refuse overlong bignums */ + if (len == 0 || len > SSHBUF_MAX_ECPOINT) + return SSH_ERR_ECPOINT_TOO_LARGE; + /* Only handle uncompressed points */ + if (*d != POINT_CONVERSION_UNCOMPRESSED) + return SSH_ERR_INVALID_FORMAT; + if (v != NULL && EC_POINT_oct2point(g, v, d, len, NULL) != 1) + return SSH_ERR_INVALID_FORMAT; /* XXX assumption */ + return 0; +} + int sshbuf_get_ec(struct sshbuf *buf, EC_POINT *v, const EC_GROUP *g) { @@ -88,14 +102,8 @@ sshbuf_get_ec(struct sshbuf *buf, EC_POINT *v, const EC_GROUP *g) if ((r = sshbuf_peek_string_direct(buf, &d, &len)) < 0) return r; - /* Refuse overlong bignums */ - if (len == 0 || len > SSHBUF_MAX_ECPOINT) - return SSH_ERR_ECPOINT_TOO_LARGE; - /* Only handle uncompressed points */ - if (*d != POINT_CONVERSION_UNCOMPRESSED) - return SSH_ERR_INVALID_FORMAT; - if (v != NULL && EC_POINT_oct2point(g, v, d, len, NULL) != 1) - return SSH_ERR_INVALID_FORMAT; /* XXX assumption */ + if ((r = get_ec(d, len, v, g)) != 0) + return r; /* Skip string */ if (sshbuf_get_string_direct(buf, NULL, NULL) != 0) { /* Shouldn't happen */ @@ -111,20 +119,29 @@ sshbuf_get_eckey(struct sshbuf *buf, EC_KEY *v) { EC_POINT *pt = EC_POINT_new(EC_KEY_get0_group(v)); int r; - size_t ooff = buf->off; /* NB. uses buffer internals to rewind on err */ + const u_char *d; + size_t len; if (pt == NULL) { SSHBUF_DBG(("SSH_ERR_ALLOC_FAIL")); return SSH_ERR_ALLOC_FAIL; } - if ((r = sshbuf_get_ec(buf, pt, EC_KEY_get0_group(v))) < 0) + if ((r = sshbuf_peek_string_direct(buf, &d, &len)) < 0) + return r; + if ((r = get_ec(d, len, pt, EC_KEY_get0_group(v))) != 0) return r; if (EC_KEY_set_public_key(v, pt) != 1) { - buf->off = ooff; EC_POINT_free(pt); return SSH_ERR_ALLOC_FAIL; /* XXX assumption */ } EC_POINT_free(pt); + /* Skip 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/ssh/sshbuf.c b/ssh/sshbuf.c index 81e3150..7f34110 100644 --- a/ssh/sshbuf.c +++ b/ssh/sshbuf.c @@ -25,6 +25,20 @@ #define SSHBUF_INTERNAL #include "sshbuf.h" +/* + * NB. do not depend on the internals of this. It will be made opaque + * one day. + */ +struct sshbuf { + u_char *d; /* Data */ + const u_char *cd; /* Const data */ + size_t off; /* First available byte is buf->d + buf->off */ + size_t size; /* Last byte is buf->d + buf->size - 1 */ + size_t max_size; /* Maximum size of buffer */ + size_t alloc; /* Total bytes allocated to buf->d */ + int readonly; /* Refers to external, const data */ +}; + static inline int sshbuf_check_sanity(const struct sshbuf *buf) { @@ -32,7 +46,6 @@ sshbuf_check_sanity(const struct sshbuf *buf) if (__predict_false(buf == NULL || (!buf->readonly && buf->d != buf->cd) || buf->cd == NULL || - (!buf->freeme && buf->readonly) || buf->max_size > SSHBUF_SIZE_MAX || buf->alloc > buf->max_size || buf->size > buf->alloc || @@ -69,7 +82,6 @@ sshbuf_new(void) return NULL; ret->alloc = SSHBUF_SIZE_INIT; ret->max_size = SSHBUF_SIZE_MAX; - ret->freeme = 1; ret->readonly = 0; if ((ret->cd = ret->d = calloc(1, ret->alloc)) == NULL) { free(ret); @@ -87,7 +99,6 @@ sshbuf_from(const void *blob, size_t len) (ret = calloc(sizeof(*ret), 1)) == NULL) return NULL; ret->alloc = ret->size = ret->max_size = len; - ret->freeme = 1; ret->readonly = 1; ret->cd = blob; ret->d = NULL; @@ -106,22 +117,9 @@ sshbuf_fromb(const struct sshbuf *buf) return sshbuf_from(sshbuf_ptr(buf), sshbuf_len(buf)); } -void -sshbuf_init(struct sshbuf *ret) -{ - bzero(ret, sizeof(*ret)); - ret->alloc = SSHBUF_SIZE_INIT; - ret->max_size = SSHBUF_SIZE_MAX; - ret->readonly = 0; - if ((ret->cd = ret->d = calloc(1, ret->alloc)) == NULL) - ret->alloc = 0; -} - void sshbuf_free(struct sshbuf *buf) { - int freeme; - if (buf == NULL) return; /* @@ -136,10 +134,8 @@ sshbuf_free(struct sshbuf *buf) bzero(buf->d, buf->alloc); free(buf->d); } - freeme = buf->freeme; bzero(buf, sizeof(buf)); - if (freeme) - free(buf); + free(buf); } void @@ -169,6 +165,12 @@ sshbuf_max_size(const struct sshbuf *buf) return buf->max_size; } +size_t +sshbuf_alloc(const struct sshbuf *buf) +{ + return buf->alloc; +} + int sshbuf_set_max_size(struct sshbuf *buf, size_t max_size) { diff --git a/ssh/sshbuf.h b/ssh/sshbuf.h index 7ab4a47..72d66b9 100644 --- a/ssh/sshbuf.h +++ b/ssh/sshbuf.h @@ -32,29 +32,7 @@ #define SSHBUF_MAX_BIGNUM (8192 / 8) /* Max bignum *bytes* */ #define SSHBUF_MAX_ECPOINT ((528 * 2 / 8) + 1) /* Max EC point *bytes* */ -/* - * NB. do not depend on the internals of this. It will be made opaque - * one day. - */ -struct sshbuf { - u_char *d; /* Data */ - const u_char *cd; /* Const data */ - size_t off; /* First available byte is buf->d + buf->off */ - size_t size; /* Last byte is buf->d + buf->size - 1 */ - size_t max_size; /* Maximum size of buffer */ - size_t alloc; /* Total bytes allocated to buf->d */ - int freeme; /* Kludge to support sshbuf_init */ - int readonly; /* Refers to external, const data */ -}; - -#ifndef SSHBUF_NO_DEPREACTED -/* - * NB. Please do not use sshbuf_init() in new code. Please use sshbuf_new() - * instead. sshbuf_init() is deprectated and will go away soon (it is - * only included to allow compat with buffer_* in OpenSSH) - */ -void sshbuf_init(struct sshbuf *buf); -#endif +struct sshbuf; /* * Create a new sshbuf buffer. @@ -272,6 +250,11 @@ int sshbuf_b64tod(struct sshbuf *buf, const char *b64); /* Internal definitions follow. Exposed for regress tests */ #ifdef SSHBUF_INTERNAL +/* + * Return the allocation size of buf + */ +size_t sshbuf_alloc(const struct sshbuf *buf); + # define SSHBUF_SIZE_INIT 256 /* Initial allocation */ # define SSHBUF_SIZE_INC 256 /* Preferred increment length */ # define SSHBUF_PACK_MIN 8192 /* Minimim packable offset */ diff --git a/unittests/sshbuf/test_sshbuf.c b/unittests/sshbuf/test_sshbuf.c index 3dad6a3..909068b 100644 --- a/unittests/sshbuf/test_sshbuf.c +++ b/unittests/sshbuf/test_sshbuf.c @@ -213,7 +213,7 @@ sshbuf_tests(void) ASSERT_MEM_FILLED_EQ(cdp, 0xd7, 1000); ASSERT_MEM_FILLED_EQ(cdp + 1000, 0x7d, 223); ASSERT_MEM_FILLED_EQ(cdp + 1223, 0xff, 1); - ASSERT_SIZE_T_EQ(p1->alloc % SSHBUF_SIZE_INC, 0); + ASSERT_SIZE_T_EQ(sshbuf_alloc(p1) % SSHBUF_SIZE_INC, 0); sshbuf_free(p1); TEST_DONE();