kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Tidy BN_bn2hex and BN_print with non-minimal inputs.

Browse Source 
These actually work as-is, but BN_bn2hex allocates more memory than
necessary, and we may as well skip the unnecessary words where we can.
Also add a test for this.

Bug: 232
Change-Id: Ie271fe9f3901d00dd5c3d7d63c1776de81a10ec7
Reviewed-on: https://boringssl-review.googlesource.com/25304
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2018-01-25 18:02:45 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent cb4e300f17
commit 23223ebbc1
2 changed files with 34 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
crypto/bn_extra/convert.c
View File

@ -77,8 +77,9 @@ int BN_bn2cbb_padded(CBB *out, size_t len, const BIGNUM *in) {
static const char hextable[] = "0123456789abcdef";
char *BN_bn2hex(const BIGNUM *bn) {
int width = bn_minimal_width(bn);
char *buf = OPENSSL_malloc(1 /* leading '-' */ + 1 /* zero is non-empty */ +
bn->top * BN_BYTES * 2 + 1 /* trailing NUL */);
width * BN_BYTES * 2 + 1 /* trailing NUL */);
if (buf == NULL) {
OPENSSL_PUT_ERROR(BN, ERR_R_MALLOC_FAILURE);
return NULL;
@ -94,7 +95,7 @@ char *BN_bn2hex(const BIGNUM *bn) {
}
int z = 0;
for (int i = bn->top - 1; i >= 0; i--) {
for (int i = width - 1; i >= 0; i--) {
for (int j = BN_BITS2 - 8; j >= 0; j -= 8) {
// strip leading zeros
int v = ((int)(bn->d[i] >> (long)j)) & 0xff;
@ -347,7 +348,7 @@ int BN_print(BIO *bp, const BIGNUM *a) {
goto end;
}
for (i = a->top - 1; i >= 0; i--) {
for (i = bn_minimal_width(a) - 1; i >= 0; i--) {
for (j = BN_BITS2 - 4; j >= 0; j -= 4) {
// strip leading zeros
v = ((int)(a->d[i] >> (long)j)) & 0x0f;

32
crypto/fipsmodule/bn/bn_test.cc
View File

@ -87,6 +87,7 @@
#include <gtest/gtest.h>
#include <openssl/bio.h>
#include <openssl/bn.h>
#include <openssl/bytestring.h>
#include <openssl/crypto.h>
@ -1966,12 +1967,17 @@ TEST_F(BNTest, NonMinimal) {
ASSERT_TRUE(two_exp_256);
ASSERT_TRUE(BN_lshift(two_exp_256.get(), BN_value_one(), 256));
// Check some comparison functions on |ten| before and after expanding.
bssl::UniquePtr<BIGNUM> zero(BN_new());
ASSERT_TRUE(zero);
BN_zero(zero.get());
for (size_t width = 1; width < 10; width++) {
SCOPED_TRACE(width);
// Make a wider version of |ten|.
// Make |ten| and |zero| wider.
EXPECT_TRUE(bn_resize_words(ten.get(), width));
EXPECT_EQ(static_cast<int>(width), ten->top);
EXPECT_TRUE(bn_resize_words(zero.get(), width));
EXPECT_EQ(static_cast<int>(width), zero->top);
EXPECT_TRUE(BN_abs_is_word(ten.get(), 10));
EXPECT_TRUE(BN_is_word(ten.get(), 10));
@ -2004,6 +2010,28 @@ TEST_F(BNTest, NonMinimal) {
EXPECT_EQ(4u, BN_num_bits(ten.get()));
EXPECT_EQ(1u, BN_num_bytes(ten.get()));
EXPECT_FALSE(BN_is_pow2(ten.get()));
bssl::UniquePtr<char> hex(BN_bn2hex(ten.get()));
EXPECT_STREQ("0a", hex.get());
hex.reset(BN_bn2hex(zero.get()));
EXPECT_STREQ("0", hex.get());
bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
ASSERT_TRUE(bio);
ASSERT_TRUE(BN_print(bio.get(), ten.get()));
const uint8_t *ptr;
size_t len;
ASSERT_TRUE(BIO_mem_contents(bio.get(), &ptr, &len));
// TODO(davidben): |BN_print| removes leading zeros within a byte, while
// |BN_bn2hex| rounds up to a byte, except for zero which it prints as
// "0". Fix this discrepancy?
EXPECT_EQ(Bytes("a"), Bytes(ptr, len));
bio.reset(BIO_new(BIO_s_mem()));
ASSERT_TRUE(bio);
ASSERT_TRUE(BN_print(bio.get(), zero.get()));
ASSERT_TRUE(BIO_mem_contents(bio.get(), &ptr, &len));
EXPECT_EQ(Bytes("0"), Bytes(ptr, len));
}
// |ten| may be resized back down to one word.