[openssl-commits] [openssl] OpenSSL_1_1_0-stable update
Andy Polyakov
appro at openssl.org
Tue Apr 25 21:54:57 UTC 2017
The branch OpenSSL_1_1_0-stable has been updated
via 6fc37bee4a5f81d8f00e6ad45865b6b697163f06 (commit)
via a84627454ba887bee7b6563a5101c2ce065ae386 (commit)
via c2e8be7543952675059c12a828d438646562f94d (commit)
via d2cbb39524e0c970b9808dc1ea372cfaa6fef685 (commit)
from bb22c4057f31346b3d8f43929c759f692a7e7ef9 (commit)
- Log -----------------------------------------------------------------
commit 6fc37bee4a5f81d8f00e6ad45865b6b697163f06
Author: Andy Polyakov <appro at openssl.org>
Date: Sat Apr 15 15:53:50 2017 +0200
asn1/a_int.c: clean up asn1_get_int64.
Trouble was that integer negation wasn't producing *formally* correct
result in platform-neutral sense. Formally correct thing to do is
-(int64_t)u, but this triggers undefined behaviour for one value that
would still be representable in ASN.1. The trigger was masked with
(int64_t)(0-u), but this is formally inappropriate for values other
than the problematic one. [Also reorder branches to favour most-likely
paths and harmonize asn1_string_set_int64 with asn1_get_int64].]
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3231)
(cherry picked from commit 786b6a45fbecc068d0fb8b05252a9228e0661c63)
commit a84627454ba887bee7b6563a5101c2ce065ae386
Author: Andy Polyakov <appro at openssl.org>
Date: Wed Apr 12 00:05:26 2017 +0200
asn1/a_int.c: don't write result if returning error.
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3192)
(cherry picked from commit b997adb3a518b065240e70acf38ec5f77a937f53)
commit c2e8be7543952675059c12a828d438646562f94d
Author: Andy Polyakov <appro at openssl.org>
Date: Wed Apr 12 00:03:35 2017 +0200
asn1/a_int.c: simplify asn1_put_uint64.
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3192)
(cherry picked from commit 6d4321fc242829490e1e7a36358eb12874c9b9e0)
commit d2cbb39524e0c970b9808dc1ea372cfaa6fef685
Author: Andy Polyakov <appro at openssl.org>
Date: Tue Apr 11 23:15:55 2017 +0200
asn1/a_int.c: remove code duplicate and optimize branches,
i.e. reduce amount of branches and favour likely ones.
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3192)
(cherry picked from commit a3ea6bf0ef703b38a656245931979c7e53c410b7)
-----------------------------------------------------------------------
Summary of changes:
crypto/asn1/a_int.c | 234 ++++++++++++++++++++++------------------------------
1 file changed, 100 insertions(+), 134 deletions(-)
diff --git a/crypto/asn1/a_int.c b/crypto/asn1/a_int.c
index 4981ddb..e154343 100644
--- a/crypto/asn1/a_int.c
+++ b/crypto/asn1/a_int.c
@@ -66,71 +66,74 @@ int ASN1_INTEGER_cmp(const ASN1_INTEGER *x, const ASN1_INTEGER *y)
* followed by optional zeros isn't padded.
*/
+/*
+ * If |pad| is zero, the operation is effectively reduced to memcpy,
+ * and if |pad| is 0xff, then it performs two's complement, ~dst + 1.
+ * Note that in latter case sequence of zeros yields itself, and so
+ * does 0x80 followed by any number of zeros. These properties are
+ * used elsewhere below...
+ */
+static void twos_complement(unsigned char *dst, const unsigned char *src,
+ size_t len, unsigned char pad)
+{
+ unsigned int carry = pad & 1;
+
+ /* Begin at the end of the encoding */
+ dst += len;
+ src += len;
+ /* two's complement value: ~value + 1 */
+ while (len-- != 0) {
+ *(--dst) = (unsigned char)(carry += *(--src) ^ pad);
+ carry >>= 8;
+ }
+}
+
static size_t i2c_ibuf(const unsigned char *b, size_t blen, int neg,
unsigned char **pp)
{
- int pad = 0;
+ unsigned int pad = 0;
size_t ret, i;
unsigned char *p, pb = 0;
- const unsigned char *n;
- if (b == NULL || blen == 0)
- ret = 1;
- else {
+ if (b != NULL && blen) {
ret = blen;
i = b[0];
- if (ret == 1 && i == 0)
- neg = 0;
if (!neg && (i > 127)) {
pad = 1;
pb = 0;
} else if (neg) {
+ pb = 0xFF;
if (i > 128) {
pad = 1;
- pb = 0xFF;
} else if (i == 128) {
/*
- * Special case: if any other bytes non zero we pad:
- * otherwise we don't.
+ * Special case [of minimal negative for given length]:
+ * if any other bytes non zero we pad, otherwise we don't.
*/
- for (i = 1; i < blen; i++)
- if (b[i]) {
- pad = 1;
- pb = 0xFF;
- break;
- }
+ for (pad = 0, i = 1; i < blen; i++)
+ pad |= b[i];
+ pb = pad != 0 ? 0xffU : 0;
+ pad = pb & 1;
}
}
ret += pad;
+ } else {
+ ret = 1;
+ blen = 0; /* reduce '(b == NULL || blen == 0)' to '(blen == 0)' */
}
- if (pp == NULL)
+
+ if (pp == NULL || (p = *pp) == NULL)
return ret;
- p = *pp;
- if (pad)
- *(p++) = pb;
- if (b == NULL || blen == 0)
- *p = 0;
- else if (!neg)
- memcpy(p, b, blen);
- else {
- /* Begin at the end of the encoding */
- n = b + blen;
- p += blen;
- i = blen;
- /* Copy zeros to destination as long as source is zero */
- while (!n[-1] && i > 1) {
- *(--p) = 0;
- n--;
- i--;
- }
- /* Complement and increment next octet */
- *(--p) = ((*(--n)) ^ 0xff) + 1;
- i--;
- /* Complement any octets left */
- for (; i > 0; i--)
- *(--p) = *(--n) ^ 0xff;
- }
+ /*
+ * This magically handles all corner cases, such as '(b == NULL ||
+ * blen == 0)', non-negative value, "negative" zero, 0x80 followed
+ * by any number of zeros...
+ */
+ *p = pb;
+ p += pad; /* yes, p[0] can be written twice, but it's little
+ * price to pay for eliminated branches */
+ twos_complement(p, b, blen, pb);
*pp += ret;
return ret;
@@ -145,7 +148,6 @@ static size_t i2c_ibuf(const unsigned char *b, size_t blen, int neg,
static size_t c2i_ibuf(unsigned char *b, int *pneg,
const unsigned char *p, size_t plen)
{
- size_t i;
int neg, pad;
/* Zero content length is illegal */
if (plen == 0) {
@@ -157,7 +159,7 @@ static size_t c2i_ibuf(unsigned char *b, int *pneg,
*pneg = neg;
/* Handle common case where length is 1 octet separately */
if (plen == 1) {
- if (b) {
+ if (b != NULL) {
if (neg)
b[0] = (p[0] ^ 0xFF) + 1;
else
@@ -174,46 +176,14 @@ static size_t c2i_ibuf(unsigned char *b, int *pneg,
ASN1err(ASN1_F_C2I_IBUF, ASN1_R_ILLEGAL_PADDING);
return 0;
}
- /* If positive just copy across */
- if (neg == 0) {
- if (b)
- memcpy(b, p + pad, plen - pad);
- return plen - pad;
- }
-
- if (neg && pad) {
- /* check is any following octets are non zero */
- for (i = 1; i < plen; i++) {
- if (p[i] != 0)
- break;
- }
- /* if all bytes are zero handle as special case */
- if (i == plen) {
- if (b) {
- b[0] = 1;
- memset(b + 1, 0, plen - 1);
- }
- return plen;
- }
- }
+ /* skip over pad */
+ p += pad;
plen -= pad;
- /* Must be negative: calculate twos complement */
- if (b) {
- const unsigned char *from = p + plen - 1 + pad;
- unsigned char *to = b + plen;
- i = plen;
- while (*from == 0 && i) {
- *--to = 0;
- i--;
- from--;
- }
- *--to = (*from-- ^ 0xff) + 1;
- OPENSSL_assert(i != 0);
- i--;
- for (; i > 0; i--)
- *--to = *from-- ^ 0xff;
- }
+
+ if (b != NULL)
+ twos_complement(b, p, plen, neg ? 0xffU : 0);
+
return plen;
}
@@ -226,56 +196,43 @@ int i2c_ASN1_INTEGER(ASN1_INTEGER *a, unsigned char **pp)
static int asn1_get_uint64(uint64_t *pr, const unsigned char *b, size_t blen)
{
size_t i;
+ uint64_t r;
+
if (blen > sizeof(*pr)) {
ASN1err(ASN1_F_ASN1_GET_UINT64, ASN1_R_TOO_LARGE);
return 0;
}
- *pr = 0;
if (b == NULL)
return 0;
- for (i = 0; i < blen; i++) {
- *pr <<= 8;
- *pr |= b[i];
+ for (r = 0, i = 0; i < blen; i++) {
+ r <<= 8;
+ r |= b[i];
}
+ *pr = r;
return 1;
}
-static size_t asn1_put_uint64(unsigned char *b, uint64_t r)
+/*
+ * Write uint64_t to big endian buffer and return offset to first
+ * written octet. In other words it returns offset in range from 0
+ * to 7, with 0 denoting 8 written octets and 7 - one.
+ */
+static size_t asn1_put_uint64(unsigned char b[sizeof(uint64_t)], uint64_t r)
{
- if (r >= 0x100) {
- unsigned char *p;
- uint64_t rtmp = r;
- size_t i = 0;
-
- /* Work out how many bytes we need */
- while (rtmp) {
- rtmp >>= 8;
- i++;
- }
-
- /* Copy from end to beginning */
- p = b + i - 1;
+ size_t off = sizeof(uint64_t);
- do {
- *p-- = r & 0xFF;
- r >>= 8;
- } while (p >= b);
-
- return i;
- }
-
- b[0] = (unsigned char)r;
- return 1;
+ do {
+ b[--off] = (unsigned char)r;
+ } while (r >>= 8);
+ return off;
}
/*
- * Absolute value of INT64_MIN: we can't just use -INT64_MIN as it produces
+ * Absolute value of INT64_MIN: we can't just use -INT64_MIN as gcc produces
* overflow warnings.
*/
-
-#define ABS_INT64_MIN \
- ((uint64_t)INT64_MAX + (uint64_t)(-(INT64_MIN + INT64_MAX)))
+#define ABS_INT64_MIN ((uint64_t)INT64_MAX + (-(INT64_MIN + INT64_MAX)))
/* signed version of asn1_get_uint64 */
static int asn1_get_int64(int64_t *pr, const unsigned char *b, size_t blen,
@@ -285,17 +242,25 @@ static int asn1_get_int64(int64_t *pr, const unsigned char *b, size_t blen,
if (asn1_get_uint64(&r, b, blen) == 0)
return 0;
if (neg) {
- if (r > ABS_INT64_MIN) {
+ if (r <= INT64_MAX) {
+ /* Most significant bit is guaranteed to be clear, negation
+ * is guaranteed to be meaningful in platform-neutral sense. */
+ *pr = -(int64_t)r;
+ } else if (r == ABS_INT64_MIN) {
+ /* This never happens if INT64_MAX == ABS_INT64_MIN, e.g.
+ * on ones'-complement system. */
+ *pr = (int64_t)(0 - r);
+ } else {
ASN1err(ASN1_F_ASN1_GET_INT64, ASN1_R_TOO_SMALL);
return 0;
}
- *pr = 0 - (uint64_t)r;
} else {
- if (r > INT64_MAX) {
+ if (r <= INT64_MAX) {
+ *pr = (int64_t)r;
+ } else {
ASN1err(ASN1_F_ASN1_GET_INT64, ASN1_R_TOO_LARGE);
return 0;
}
- *pr = (int64_t)r;
}
return 1;
}
@@ -356,18 +321,22 @@ static int asn1_string_get_int64(int64_t *pr, const ASN1_STRING *a, int itype)
static int asn1_string_set_int64(ASN1_STRING *a, int64_t r, int itype)
{
unsigned char tbuf[sizeof(r)];
- size_t l;
+ size_t off;
+
a->type = itype;
if (r < 0) {
- l = asn1_put_uint64(tbuf, -r);
+ /* Most obvious '-r' triggers undefined behaviour for most
+ * common INT64_MIN. Even though below '0 - (uint64_t)r' can
+ * appear two's-complement centric, it does produce correct/
+ * expected result even on one's-complement. This is because
+ * cast to unsigned has to change bit pattern... */
+ off = asn1_put_uint64(tbuf, 0 - (uint64_t)r);
a->type |= V_ASN1_NEG;
} else {
- l = asn1_put_uint64(tbuf, r);
+ off = asn1_put_uint64(tbuf, r);
a->type &= ~V_ASN1_NEG;
}
- if (l == 0)
- return 0;
- return ASN1_STRING_set(a, tbuf, l);
+ return ASN1_STRING_set(a, tbuf + off, sizeof(tbuf) - off);
}
static int asn1_string_get_uint64(uint64_t *pr, const ASN1_STRING *a,
@@ -391,12 +360,11 @@ static int asn1_string_get_uint64(uint64_t *pr, const ASN1_STRING *a,
static int asn1_string_set_uint64(ASN1_STRING *a, uint64_t r, int itype)
{
unsigned char tbuf[sizeof(r)];
- size_t l;
+ size_t off;
+
a->type = itype;
- l = asn1_put_uint64(tbuf, r);
- if (l == 0)
- return 0;
- return ASN1_STRING_set(a, tbuf, l);
+ off = asn1_put_uint64(tbuf, r);
+ return ASN1_STRING_set(a, tbuf + off, sizeof(tbuf) - off);
}
/*
@@ -643,11 +611,9 @@ int c2i_uint64_int(uint64_t *ret, int *neg, const unsigned char **pp, long len)
int i2c_uint64_int(unsigned char *p, uint64_t r, int neg)
{
unsigned char buf[sizeof(uint64_t)];
- size_t buflen;
+ size_t off;
- buflen = asn1_put_uint64(buf, r);
- if (p == NULL)
- return i2c_ibuf(buf, buflen, neg, NULL);
- return i2c_ibuf(buf, buflen, neg, &p);
+ off = asn1_put_uint64(buf, r);
+ return i2c_ibuf(buf + off, sizeof(buf) - off, neg, &p);
}
More information about the openssl-commits
mailing list