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

Always push errors on BIO_read_asn1 failure.

Browse Source 
This is consistent with the old behavior of d2i_*_fp and avoids tripping
Conscrypt's unnecessarily fragile error-handling (see
https://github.com/google/conscrypt/pull/552).

Additionally, by source inspection, CPython expects
ASN1_R_HEADER_TOO_LONG on EOF, analogously to PEM_R_NO_START_LINE. Fix
that. The other errors are a bit haphazard in the old implementation
(that code is really hard to follow), so I didn't match it too
carefully. In particular, OpenSSL would report ASN1_R_HEADER_TOO_LONG on
some generic tag parsing, but that is inconsistent with
ASN1_R_HEADER_TOO_LONG being an EOF signal.

Update-Note: https://boringssl-review.googlesource.com/32106 may have
caused some compatibility issues. This should fix it.

Change-Id: Idfe2746ffd7733de4338e14c58a40753e98a791e
Reviewed-on: https://boringssl-review.googlesource.com/c/32444
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2018-10-11 12:40:59 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent 2d98d49cf7
commit 80aa694975
2 changed files with 47 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
crypto/bio/bio.c
View File

@ -61,6 +61,7 @@
#include <limits.h>
#include <string.h>
#include <openssl/asn1.h>
#include <openssl/err.h>
#include <openssl/mem.h>
#include <openssl/thread.h>
@ -481,11 +482,28 @@ static int bio_read_all(BIO *bio, uint8_t **out, size_t *out_len,
}
}
// For compatibility with existing |d2i_*_bio| callers, |BIO_read_asn1| uses
// |ERR_LIB_ASN1| errors.
OPENSSL_DECLARE_ERROR_REASON(ASN1, ASN1_R_DECODE_ERROR)
OPENSSL_DECLARE_ERROR_REASON(ASN1, ASN1_R_HEADER_TOO_LONG)
OPENSSL_DECLARE_ERROR_REASON(ASN1, ASN1_R_NOT_ENOUGH_DATA)
OPENSSL_DECLARE_ERROR_REASON(ASN1, ASN1_R_TOO_LONG)
int BIO_read_asn1(BIO *bio, uint8_t **out, size_t *out_len, size_t max_len) {
uint8_t header[6];
static const size_t kInitialHeaderLen = 2;
if (BIO_read(bio, header, kInitialHeaderLen) != (int) kInitialHeaderLen) {
int ret = BIO_read(bio, header, kInitialHeaderLen);
if (ret == 0) {
// Historically, OpenSSL returned |ASN1_R_HEADER_TOO_LONG| when |d2i_*_bio|
// could not read anything. CPython conditions on this to determine if |bio|
// was empty.
OPENSSL_PUT_ERROR(ASN1, ASN1_R_HEADER_TOO_LONG);
return 0;
}
if (ret != (int) kInitialHeaderLen) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_NOT_ENOUGH_DATA);
return 0;
}
@ -494,6 +512,7 @@ int BIO_read_asn1(BIO *bio, uint8_t **out, size_t *out_len, size_t max_len) {
if ((tag & 0x1f) == 0x1f) {
// Long form tags are not supported.
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return 0;
}
@ -507,34 +526,41 @@ int BIO_read_asn1(BIO *bio, uint8_t **out, size_t *out_len, size_t max_len) {
if ((tag & 0x20 /* constructed */) != 0 && num_bytes == 0) {
// indefinite length.
return bio_read_all(bio, out, out_len, header, kInitialHeaderLen,
max_len);
if (!bio_read_all(bio, out, out_len, header, kInitialHeaderLen,
max_len)) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_NOT_ENOUGH_DATA);
return 0;
}
return 1;
}
if (num_bytes == 0 || num_bytes > 4) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return 0;
}
if (BIO_read(bio, header + kInitialHeaderLen, num_bytes) !=
(int)num_bytes) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_NOT_ENOUGH_DATA);
return 0;
}
header_len = kInitialHeaderLen + num_bytes;
uint32_t len32 = 0;
unsigned i;
for (i = 0; i < num_bytes; i++) {
for (unsigned i = 0; i < num_bytes; i++) {
len32 <<= 8;
len32 |= header[kInitialHeaderLen + i];
}
if (len32 < 128) {
// Length should have used short-form encoding.
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return 0;
}
if ((len32 >> ((num_bytes-1)*8)) == 0) {
// Length should have been at least one byte shorter.
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return 0;
}
@ -544,6 +570,7 @@ int BIO_read_asn1(BIO *bio, uint8_t **out, size_t *out_len, size_t max_len) {
if (len + header_len < len ||
len + header_len > max_len ||
len > INT_MAX) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_TOO_LONG);
return 0;
}
len += header_len;
@ -551,11 +578,13 @@ int BIO_read_asn1(BIO *bio, uint8_t **out, size_t *out_len, size_t max_len) {
*out = OPENSSL_malloc(len);
if (*out == NULL) {
OPENSSL_PUT_ERROR(ASN1, ERR_R_MALLOC_FAILURE);
return 0;
}
OPENSSL_memcpy(*out, header, header_len);
if (BIO_read(bio, (*out) + header_len, len - header_len) !=
(int) (len - header_len)) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_NOT_ENOUGH_DATA);
OPENSSL_free(*out);
return 0;
}

13
crypto/x509/x509_test.cc
View File

@ -1671,3 +1671,16 @@ TEST(X509Test, PEMX509Info) {
PEM_X509_INFO_read_bio(bio.get(), infos.get(), nullptr, nullptr));
EXPECT_EQ(2 * OPENSSL_ARRAY_SIZE(kExpected), sk_X509_INFO_num(infos.get()));
}
TEST(X509Test, ReadBIOEmpty) {
bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(nullptr, 0));
ASSERT_TRUE(bio);
// CPython expects |ASN1_R_HEADER_TOO_LONG| on EOF, to terminate a series of
// certificates.
bssl::UniquePtr<X509> x509(d2i_X509_bio(bio.get(), nullptr));
EXPECT_FALSE(x509);
uint32_t err = ERR_get_error();
EXPECT_EQ(ERR_LIB_ASN1, ERR_GET_LIB(err));
EXPECT_EQ(ASN1_R_HEADER_TOO_LONG, ERR_GET_REASON(err));
}