bug 1060929 - mozilla::pkix: allow explicit encodings of default-valued BOOLEANs for compatibility r=briansmith

This commit is contained in:
David Keeler 2014-09-22 09:26:10 -07:00
parent c22a53a4ed
commit 15fae978fd
3 changed files with 18 additions and 46 deletions

View File

@ -304,13 +304,7 @@ static Result
DecodeBasicConstraints(Reader& input, /*out*/ bool& isCA,
/*out*/ long& pathLenConstraint)
{
// TODO(bug 989518): cA is by default false. According to DER, default
// values must not be explicitly encoded in a SEQUENCE. So, if this
// value is present and false, it is an encoding error. However, Go Daddy
// has issued many certificates with this improper encoding, so we can't
// enforce this yet (hence passing true for allowInvalidExplicitEncoding
// to der::OptionalBoolean).
if (der::OptionalBoolean(input, true, isCA) != Success) {
if (der::OptionalBoolean(input, isCA) != Success) {
return Result::ERROR_EXTENSION_VALUE_INVALID;
}

View File

@ -283,13 +283,14 @@ Boolean(Reader& input, /*out*/ bool& value)
}
}
// This is for any BOOLEAN DEFAULT FALSE.
// (If it is present and false, this is a bad encoding.)
// TODO(bug 989518): For compatibility reasons, in some places we allow
// invalid encodings with the explicit default value.
// This is for BOOLEAN DEFAULT FALSE.
// The standard stipulates that "The encoding of a set value or sequence value
// shall not include an encoding for any component value which is equal to its
// default value." However, it appears to be common that other libraries
// incorrectly include the value of a BOOLEAN even when it's equal to the
// default value, so we allow invalid explicit encodings here.
inline Result
OptionalBoolean(Reader& input, bool allowInvalidExplicitEncoding,
/*out*/ bool& value)
OptionalBoolean(Reader& input, /*out*/ bool& value)
{
value = false;
if (input.Peek(BOOLEAN)) {
@ -297,9 +298,6 @@ OptionalBoolean(Reader& input, bool allowInvalidExplicitEncoding,
if (rv != Success) {
return rv;
}
if (!allowInvalidExplicitEncoding && !value) {
return Result::ERROR_BAD_DER;
}
}
return Success;
}
@ -543,7 +541,7 @@ OptionalExtensions(Reader& input, uint8_t tag,
return rv;
}
bool critical;
rv = OptionalBoolean(extension, false, critical);
rv = OptionalBoolean(extension, critical);
if (rv != Success) {
return rv;
}

View File

@ -126,9 +126,8 @@ TEST_F(pkixder_universal_types_tests, BooleanInvalidZeroLength)
// OptionalBoolean implements decoding of OPTIONAL BOOLEAN DEFAULT FALSE.
// If the field is present, it must be a valid encoding of a BOOLEAN with
// value TRUE. If the field is not present, it defaults to FALSE. For
// compatibility reasons, OptionalBoolean can be told to accept an encoding
// where the field is present with value FALSE (this is technically not a
// valid DER encoding).
// compatibility reasons, OptionalBoolean also accepts encodings where the field
// is present with value FALSE (this is technically not a valid DER encoding).
TEST_F(pkixder_universal_types_tests, OptionalBooleanValidEncodings)
{
{
@ -140,7 +139,7 @@ TEST_F(pkixder_universal_types_tests, OptionalBooleanValidEncodings)
Input input(DER_OPTIONAL_BOOLEAN_PRESENT_TRUE);
Reader reader(input);
bool value = false;
ASSERT_EQ(Success, OptionalBoolean(reader, false, value)) <<
ASSERT_EQ(Success, OptionalBoolean(reader, value)) <<
"Should accept the only valid encoding of a present OPTIONAL BOOLEAN";
ASSERT_TRUE(value);
ASSERT_TRUE(reader.AtEnd());
@ -156,7 +155,7 @@ TEST_F(pkixder_universal_types_tests, OptionalBooleanValidEncodings)
Input input(DER_INTEGER_05);
Reader reader(input);
bool value = true;
ASSERT_EQ(Success, OptionalBoolean(reader, false, value)) <<
ASSERT_EQ(Success, OptionalBoolean(reader, value)) <<
"Should accept a valid encoding of an omitted OPTIONAL BOOLEAN";
ASSERT_FALSE(value);
ASSERT_FALSE(reader.AtEnd());
@ -167,7 +166,7 @@ TEST_F(pkixder_universal_types_tests, OptionalBooleanValidEncodings)
ASSERT_EQ(Success, input.Init(reinterpret_cast<const uint8_t*>(""), 0));
Reader reader(input);
bool value = true;
ASSERT_EQ(Success, OptionalBoolean(reader, false, value)) <<
ASSERT_EQ(Success, OptionalBoolean(reader, value)) <<
"Should accept another valid encoding of an omitted OPTIONAL BOOLEAN";
ASSERT_FALSE(value);
ASSERT_TRUE(reader.AtEnd());
@ -182,27 +181,12 @@ TEST_F(pkixder_universal_types_tests, OptionalBooleanInvalidEncodings)
0x00 // false
};
{
Input input(DER_OPTIONAL_BOOLEAN_PRESENT_FALSE);
Reader reader(input);
bool value;
// If the second parameter to OptionalBoolean is false, invalid encodings
// that include the field even when it is the DEFAULT FALSE are rejected.
bool allowInvalidEncodings = false;
ASSERT_EQ(Result::ERROR_BAD_DER,
OptionalBoolean(reader, allowInvalidEncodings, value)) <<
"Should reject an invalid encoding of present OPTIONAL BOOLEAN";
}
{
Input input(DER_OPTIONAL_BOOLEAN_PRESENT_FALSE);
Reader reader(input);
bool value = true;
// If the second parameter to OptionalBoolean is true, invalid encodings
// that include the field even when it is the DEFAULT FALSE are accepted.
bool allowInvalidEncodings = true;
ASSERT_EQ(Success, OptionalBoolean(reader, allowInvalidEncodings, value)) <<
"Should now accept an invalid encoding of present OPTIONAL BOOLEAN";
ASSERT_EQ(Success, OptionalBoolean(reader, value)) <<
"Should accept an invalid, default-value encoding of OPTIONAL BOOLEAN";
ASSERT_FALSE(value);
ASSERT_TRUE(reader.AtEnd());
}
@ -216,13 +200,9 @@ TEST_F(pkixder_universal_types_tests, OptionalBooleanInvalidEncodings)
{
Input input(DER_OPTIONAL_BOOLEAN_PRESENT_42);
Reader reader(input);
// Even with the second parameter to OptionalBoolean as true, encodings
// of BOOLEAN that are invalid altogether are rejected.
bool allowInvalidEncodings = true;
bool value;
ASSERT_EQ(Result::ERROR_BAD_DER,
OptionalBoolean(reader, allowInvalidEncodings, value)) <<
"Should reject another invalid encoding of present OPTIONAL BOOLEAN";
ASSERT_EQ(Result::ERROR_BAD_DER, OptionalBoolean(reader, value)) <<
"Should reject an invalid-valued encoding of OPTIONAL BOOLEAN";
}
}