Chiebot-Mirror
/
boringssl
mirror of https://gitee.com/mirrors/boringssl.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix EDIPartyName parsing and GENERAL_NAME_cmp.

Browse Source 
See also CVE-2020-1971, f960d81215ebf3f65e03d4d5d857fb9b666d6920, and
aa0ad2011d3e7ad8a611da274ef7d9c7706e289b from upstream OpenSSL.

Unlike upstream's version, this CL opts for a simpler edipartyname_cmp.
GENERAL_NAME_cmp is already unsuitable for ordering, just equality,
which means there's no need to preserve return values from
ASN1_STRING_cmp. Additionally, the ASN.1 structure implies most fields
cannot be NULL.

(The change from other to x400Address is a no-op. They're the same type.
Just x400Address is a little clearer. Historical quirks of the
GENERAL_NAME structure.)

Change-Id: I4b0ffe8e931c8ef916794a486e6a0d6d684c0cc1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44404
Reviewed-by: Adam Langley <agl@google.com>
chromium-5359
David Benjamin 4 years ago committed by Adam Langley
parent 455b78d5f9
commit aa4ecb4926
3 changed files with 217 additions and 5 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 179
      crypto/x509/x509_test.cc
  2. 37
      crypto/x509v3/v3_genn.c
  3. 6
      include/openssl/x509v3.h

179
crypto/x509/x509_test.cc
Unescape View File

@ -2813,3 +2813,182 @@ TEST(X509Test, AlgorithmParameters) {
EXPECT_EQ(ERR_LIB_X509, ERR_GET_LIB(err));
EXPECT_EQ(X509_R_INVALID_PARAMETER, ERR_GET_REASON(err));
}
TEST(X509Test, GeneralName) {
const std::vector<uint8_t> kNames[] = {
// [0] {
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2.1 }
// [0] {
// SEQUENCE {}
// }
// }
{0xa0, 0x13, 0x06, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04,
0x01, 0x84, 0xb7, 0x09, 0x02, 0x01, 0xa0, 0x02, 0x30, 0x00},
// [0] {
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2.1 }
// [0] {
// [APPLICATION 0] {}
// }
// }
{0xa0, 0x13, 0x06, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04,
0x01, 0x84, 0xb7, 0x09, 0x02, 0x01, 0xa0, 0x02, 0x60, 0x00},
// [0] {
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2.1 }
// [0] {
// UTF8String { "a" }
// }
// }
{0xa0, 0x14, 0x06, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04,
0x01, 0x84, 0xb7, 0x09, 0x02, 0x01, 0xa0, 0x03, 0x0c, 0x01, 0x61},
// [0] {
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2.2 }
// [0] {
// UTF8String { "a" }
// }
// }
{0xa0, 0x14, 0x06, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04,
0x01, 0x84, 0xb7, 0x09, 0x02, 0x02, 0xa0, 0x03, 0x0c, 0x01, 0x61},
// [0] {
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2.1 }
// [0] {
// UTF8String { "b" }
// }
// }
{0xa0, 0x14, 0x06, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04,
0x01, 0x84, 0xb7, 0x09, 0x02, 0x01, 0xa0, 0x03, 0x0c, 0x01, 0x62},
// [0] {
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2.1 }
// [0] {
// BOOLEAN { TRUE }
// }
// }
{0xa0, 0x14, 0x06, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04,
0x01, 0x84, 0xb7, 0x09, 0x02, 0x01, 0xa0, 0x03, 0x01, 0x01, 0xff},
// [0] {
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2.1 }
// [0] {
// BOOLEAN { FALSE }
// }
// }
{0xa0, 0x14, 0x06, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04,
0x01, 0x84, 0xb7, 0x09, 0x02, 0x01, 0xa0, 0x03, 0x01, 0x01, 0x00},
// [1 PRIMITIVE] { "a" }
{0x81, 0x01, 0x61},
// [1 PRIMITIVE] { "b" }
{0x81, 0x01, 0x62},
// [2 PRIMITIVE] { "a" }
{0x82, 0x01, 0x61},
// [2 PRIMITIVE] { "b" }
{0x82, 0x01, 0x62},
// [4] {
// SEQUENCE {
// SET {
// SEQUENCE {
// # commonName
// OBJECT_IDENTIFIER { 2.5.4.3 }
// UTF8String { "a" }
// }
// }
// }
// }
{0xa4, 0x0e, 0x30, 0x0c, 0x31, 0x0a, 0x30, 0x08, 0x06, 0x03, 0x55, 0x04,
0x03, 0x0c, 0x01, 0x61},
// [4] {
// SEQUENCE {
// SET {
// SEQUENCE {
// # commonName
// OBJECT_IDENTIFIER { 2.5.4.3 }
// UTF8String { "b" }
// }
// }
// }
// }
{0xa4, 0x0e, 0x30, 0x0c, 0x31, 0x0a, 0x30, 0x08, 0x06, 0x03, 0x55, 0x04,
0x03, 0x0c, 0x01, 0x62},
// [5] {
// [1] {
// UTF8String { "a" }
// }
// }
{0xa5, 0x05, 0xa1, 0x03, 0x0c, 0x01, 0x61},
// [5] {
// [1] {
// UTF8String { "b" }
// }
// }
{0xa5, 0x05, 0xa1, 0x03, 0x0c, 0x01, 0x62},
// [5] {
// [0] {
// UTF8String {}
// }
// [1] {
// UTF8String { "a" }
// }
// }
{0xa5, 0x09, 0xa0, 0x02, 0x0c, 0x00, 0xa1, 0x03, 0x0c, 0x01, 0x61},
// [5] {
// [0] {
// UTF8String { "a" }
// }
// [1] {
// UTF8String { "a" }
// }
// }
{0xa5, 0x0a, 0xa0, 0x03, 0x0c, 0x01, 0x61, 0xa1, 0x03, 0x0c, 0x01, 0x61},
// [5] {
// [0] {
// UTF8String { "b" }
// }
// [1] {
// UTF8String { "a" }
// }
// }
{0xa5, 0x0a, 0xa0, 0x03, 0x0c, 0x01, 0x62, 0xa1, 0x03, 0x0c, 0x01, 0x61},
// [6 PRIMITIVE] { "a" }
{0x86, 0x01, 0x61},
// [6 PRIMITIVE] { "b" }
{0x86, 0x01, 0x62},
// [7 PRIMITIVE] { `11111111` }
{0x87, 0x04, 0x11, 0x11, 0x11, 0x11},
// [7 PRIMITIVE] { `22222222`}
{0x87, 0x04, 0x22, 0x22, 0x22, 0x22},
// [7 PRIMITIVE] { `11111111111111111111111111111111` }
{0x87, 0x10, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
0x11, 0x11, 0x11, 0x11, 0x11, 0x11},
// [7 PRIMITIVE] { `22222222222222222222222222222222` }
{0x87, 0x10, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22,
0x22, 0x22, 0x22, 0x22, 0x22, 0x22},
// [8 PRIMITIVE] { 1.2.840.113554.4.1.72585.2.1 }
{0x88, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
0x09, 0x02, 0x01},
// [8 PRIMITIVE] { 1.2.840.113554.4.1.72585.2.2 }
{0x88, 0x0d, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
0x09, 0x02, 0x02},
};
// Every name should be equal to itself and not equal to any others.
for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(kNames); i++) {
SCOPED_TRACE(Bytes(kNames[i]));
const uint8_t *ptr = kNames[i].data();
bssl::UniquePtr<GENERAL_NAME> a(
d2i_GENERAL_NAME(nullptr, &ptr, kNames[i].size()));
ASSERT_TRUE(a);
for (size_t j = 0; j < OPENSSL_ARRAY_SIZE(kNames); j++) {
SCOPED_TRACE(Bytes(kNames[j]));
ptr = kNames[j].data();
bssl::UniquePtr<GENERAL_NAME> b(
d2i_GENERAL_NAME(nullptr, &ptr, kNames[j].size()));
ASSERT_TRUE(b);
if (i == j) {
EXPECT_EQ(GENERAL_NAME_cmp(a.get(), b.get()), 0);
} else {
EXPECT_NE(GENERAL_NAME_cmp(a.get(), b.get()), 0);
}
}
}
}

37
crypto/x509v3/v3_genn.c
Unescape View File

@ -72,8 +72,9 @@ ASN1_SEQUENCE(OTHERNAME) = {
IMPLEMENT_ASN1_FUNCTIONS(OTHERNAME)
ASN1_SEQUENCE(EDIPARTYNAME) = {
ASN1_IMP_OPT(EDIPARTYNAME, nameAssigner, DIRECTORYSTRING, 0),
ASN1_IMP_OPT(EDIPARTYNAME, partyName, DIRECTORYSTRING, 1)
/* DirectoryString is a CHOICE type, so use explicit tagging. */
ASN1_EXP_OPT(EDIPARTYNAME, nameAssigner, DIRECTORYSTRING, 0),
ASN1_EXP(EDIPARTYNAME, partyName, DIRECTORYSTRING, 1)
} ASN1_SEQUENCE_END(EDIPARTYNAME)
IMPLEMENT_ASN1_FUNCTIONS(EDIPARTYNAME)
@ -102,6 +103,24 @@ IMPLEMENT_ASN1_FUNCTIONS(GENERAL_NAMES)
IMPLEMENT_ASN1_DUP_FUNCTION(GENERAL_NAME)
static int edipartyname_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b)
{
/* nameAssigner is optional and may be NULL. */
if (a->nameAssigner == NULL) {
if (b->nameAssigner != NULL) {
return -1;
}
} else {
if (b->nameAssigner == NULL ||
ASN1_STRING_cmp(a->nameAssigner, b->nameAssigner) != 0) {
return -1;
}
}
/* partyName may not be NULL. */
return ASN1_STRING_cmp(a->partyName, b->partyName);
}
/* Returns 0 if they are equal, != 0 otherwise. */
int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b)
{
@ -111,8 +130,11 @@ int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b)
return -1;
switch (a->type) {
case GEN_X400:
result = ASN1_TYPE_cmp(a->d.x400Address, b->d.x400Address);
break;
case GEN_EDIPARTY:
result = ASN1_TYPE_cmp(a->d.other, b->d.other);
result = edipartyname_cmp(a->d.ediPartyName, b->d.ediPartyName);
break;
case GEN_OTHERNAME:
@ -159,8 +181,11 @@ void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, void *value)
{
switch (type) {
case GEN_X400:
a->d.x400Address = value;
break;
case GEN_EDIPARTY:
a->d.other = value;
a->d.ediPartyName = value;
break;
case GEN_OTHERNAME:
@ -194,8 +219,10 @@ void *GENERAL_NAME_get0_value(const GENERAL_NAME *a, int *ptype)
*ptype = a->type;
switch (a->type) {
case GEN_X400:
return a->d.x400Address;
case GEN_EDIPARTY:
return a->d.other;
return a->d.ediPartyName;
case GEN_OTHERNAME:
return a->d.otherName;

6
include/openssl/x509v3.h
Unescape View File

@ -474,6 +474,12 @@ DECLARE_ASN1_FUNCTIONS(AUTHORITY_KEYID)
DECLARE_ASN1_FUNCTIONS(GENERAL_NAME)
OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a);
// GENERAL_NAME_cmp returns zero if |a| and |b| are equal and a non-zero
// value otherwise. Note this function does not provide a comparison suitable
// for sorting.
//
// TODO(davidben): Const-correct this function.
OPENSSL_EXPORT int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b);

Write Preview
Loading…
Cancel
Save