wine/gecko
0
0
Fork 0
mirror of https://gitlab.winehq.org/wine/wine-gecko.git synced 2024-09-13 09:24:08 -07:00
Code Issues Packages Projects Releases Wiki Activity

Bug 989564, Part 1: Decode basic constraints extension using mozilla::pkix::der, r=keeler

Browse Source 
--HG--
extra : rebase_source : 89560218a69596868cb8a93c69ee72656b0abf77
This commit is contained in:
Brian Smith 2014-05-05 09:55:57 -07:00
parent 1663974cbd
commit 07edc768dc
3 changed files with 279 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
security/pkix/lib/pkixcheck.cpp
View File

@ -24,11 +24,11 @@
#include <limits>
#include "pkix/bind.h"
#include "pkix/pkix.h"
#include "pkixcheck.h"
#include "pkixder.h"
#include "pkixutil.h"
#include "secder.h"
namespace mozilla { namespace pkix {
@ -166,32 +166,15 @@ CheckCertificatePolicies(BackCert& cert, EndEntityOrCA endEntityOrCA,
return Fail(RecoverableError, SEC_ERROR_POLICY_VALIDATION_FAILED);
}
static const long UNLIMITED_PATH_LEN = -1; // must be less than zero
// BasicConstraints ::= SEQUENCE {
// cA BOOLEAN DEFAULT FALSE,
// pathLenConstraint INTEGER (0..MAX) OPTIONAL }
der::Result
DecodeBasicConstraints(const SECItem* encodedBasicConstraints,
CERTBasicConstraints& basicConstraints)
static der::Result
DecodeBasicConstraints(der::Input& input, /*out*/ bool& isCA,
/*out*/ long& pathLenConstraint)
{
PR_ASSERT(encodedBasicConstraints);
if (!encodedBasicConstraints) {
return der::Fail(SEC_ERROR_INVALID_ARGS);
}
basicConstraints.isCA = false;
basicConstraints.pathLenConstraint = 0;
der::Input input;
if (input.Init(encodedBasicConstraints->data, encodedBasicConstraints->len)
!= der::Success) {
return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
}
if (der::ExpectTagAndIgnoreLength(input, der::SEQUENCE) != der::Success) {
return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
}
bool isCA = false;
// 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
@ -201,30 +184,15 @@ DecodeBasicConstraints(const SECItem* encodedBasicConstraints,
if (der::OptionalBoolean(input, true, isCA) != der::Success) {
return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
}
basicConstraints.isCA = isCA;
if (input.Peek(der::INTEGER)) {
SECItem pathLenConstraintEncoded;
if (der::Integer(input, pathLenConstraintEncoded) != der::Success) {
return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
}
long pathLenConstraint = DER_GetInteger(&pathLenConstraintEncoded);
if (pathLenConstraint >= std::numeric_limits<int>::max() ||
pathLenConstraint < 0) {
return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
}
basicConstraints.pathLenConstraint = static_cast<int>(pathLenConstraint);
// TODO(bug 985025): If isCA is false, pathLenConstraint MUST NOT
// be included (as per RFC 5280 section 4.2.1.9), but for compatibility
// reasons, we don't check this for now.
} else if (basicConstraints.isCA) {
// If this is a CA but the path length is omitted, it is unlimited.
basicConstraints.pathLenConstraint = CERT_UNLIMITED_PATH_CONSTRAINT;
}
if (der::End(input) != der::Success) {
// TODO(bug 985025): If isCA is false, pathLenConstraint MUST NOT
// be included (as per RFC 5280 section 4.2.1.9), but for compatibility
// reasons, we don't check this for now.
if (OptionalInteger(input, UNLIMITED_PATH_LEN, pathLenConstraint)
!= der::Success) {
return der::Fail(SEC_ERROR_EXTENSION_VALUE_INVALID);
}
return der::Success;
}
@ -235,17 +203,24 @@ CheckBasicConstraints(const BackCert& cert,
bool isTrustAnchor,
unsigned int subCACount)
{
CERTBasicConstraints basicConstraints;
bool isCA = false;
long pathLenConstraint = UNLIMITED_PATH_LEN;
if (cert.encodedBasicConstraints) {
if (DecodeBasicConstraints(cert.encodedBasicConstraints,
basicConstraints) != der::Success) {
return RecoverableError;
der::Input input;
if (input.Init(cert.encodedBasicConstraints->data,
cert.encodedBasicConstraints->len) != der::Success) {
return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
}
if (der::Nested(input, der::SEQUENCE,
bind(DecodeBasicConstraints, _1, ref(isCA),
ref(pathLenConstraint))) != der::Success) {
return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
}
if (der::End(input) != der::Success) {
return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
}
} else {
// Synthesize a non-CA basic constraints by default
basicConstraints.isCA = false;
basicConstraints.pathLenConstraint = 0;
// "If the basic constraints extension is not present in a version 3
// certificate, or the extension is present but the cA boolean is not
// asserted, then the certified public key MUST NOT be used to verify
@ -261,8 +236,7 @@ CheckBasicConstraints(const BackCert& cert,
// basicConstraints extension if they are v1. v1 is encoded
// implicitly.
if (!nssCert->version.data && !nssCert->version.len) {
basicConstraints.isCA = true;
basicConstraints.pathLenConstraint = CERT_UNLIMITED_PATH_CONSTRAINT;
isCA = true;
}
}
}
@ -270,7 +244,7 @@ CheckBasicConstraints(const BackCert& cert,
if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
// CA certificates are not trusted as EE certs.
if (basicConstraints.isCA) {
if (isCA) {
// XXX: We use SEC_ERROR_CA_CERT_INVALID here so we can distinguish
// this error from other errors, given that NSS does not have a "CA cert
// used as end-entity" error code since it doesn't have such a
@ -290,15 +264,13 @@ CheckBasicConstraints(const BackCert& cert,
PORT_Assert(endEntityOrCA == EndEntityOrCA::MustBeCA);
// End-entity certificates are not allowed to act as CA certs.
if (!basicConstraints.isCA) {
if (!isCA) {
return Fail(RecoverableError, SEC_ERROR_CA_CERT_INVALID);
}
if (basicConstraints.pathLenConstraint >= 0) {
if (subCACount >
static_cast<unsigned int>(basicConstraints.pathLenConstraint)) {
return Fail(RecoverableError, SEC_ERROR_PATH_LEN_CONSTRAINT_INVALID);
}
if (pathLenConstraint >= 0 &&
static_cast<long>(subCACount) > pathLenConstraint) {
return Fail(RecoverableError, SEC_ERROR_PATH_LEN_CONSTRAINT_INVALID);
}
return Success;

110
security/pkix/lib/pkixder.h
View File

@ -372,6 +372,32 @@ Skip(Input& input, uint8_t tag, /*out*/ SECItem& value)
// Universal types
namespace internal {
// This parser will only parse values between 0..127. If this range is
// increased then callers will need to be changed.
template <typename T> inline Result
IntegralValue(Input& input, uint8_t tag, T& value)
{
// Conveniently, all the Integers that we actually have to be able to parse
// are positive and very small. Consequently, this parser is *much* simpler
// than a general Integer parser would need to be.
if (ExpectTagAndLength(input, tag, 1) != Success) {
return Failure;
}
uint8_t valueByte;
if (input.Read(valueByte) != Success) {
return Failure;
}
if (valueByte & 0x80) { // negative
return Fail(SEC_ERROR_BAD_DER);
}
value = valueByte;
return Success;
}
} // namespace internal
inline Result
Boolean(Input& input, /*out*/ bool& value)
{
@ -411,13 +437,12 @@ OptionalBoolean(Input& input, bool allowInvalidExplicitEncoding,
return Success;
}
// This parser will only parse values between 0..127. If this range is
// increased then callers will need to be changed.
inline Result
Enumerated(Input& input, uint8_t& value)
{
if (ExpectTagAndLength(input, ENUMERATED | 0, 1) != Success) {
return Failure;
}
return input.Read(value);
return internal::IntegralValue(input, ENUMERATED | 0, value);
}
inline Result
@ -438,34 +463,40 @@ GeneralizedTime(Input& input, PRTime& time)
return Success;
}
// This parser will only parse values between 0..127. If this range is
// increased then callers will need to be changed.
inline Result
Integer(Input& input, /*out*/ SECItem& value)
Integer(Input& input, /*out*/ uint8_t& value)
{
uint16_t length;
if (ExpectTagAndGetLength(input, INTEGER, length) != Success) {
if (internal::IntegralValue(input, INTEGER, value) != Success) {
return Failure;
}
return Success;
}
if (input.Skip(length, value) != Success) {
// This parser will only parse values between 0..127. If this range is
// increased then callers will need to be changed. The default value must be
// -1; defaultValue is only a parameter to make it clear in the calling code
// what the default value is.
inline Result
OptionalInteger(Input& input, long defaultValue, /*out*/ long& value)
{
// If we need to support a different default value in the future, we need to
// test that parsedValue != defaultValue.
if (defaultValue != -1) {
return Fail(SEC_ERROR_INVALID_ARGS);
}
if (!input.Peek(INTEGER)) {
value = defaultValue;
return Success;
}
uint8_t parsedValue;
if (Integer(input, parsedValue) != Success) {
return Failure;
}
if (value.len == 0) {
return Fail(SEC_ERROR_BAD_DER);
}
// Check for overly-long encodings. If the first byte is 0x00 then the high
// bit on the second byte must be 1; otherwise the same *positive* value
// could be encoded without the leading 0x00 byte. If the first byte is 0xFF
// then the second byte must NOT have its high bit set; otherwise the same
// *negative* value could be encoded without the leading 0xFF byte.
if (value.len > 1) {
if ((value.data[0] == 0x00 && (value.data[1] & 0x80) == 0) ||
(value.data[0] == 0xff && (value.data[1] & 0x80) != 0)) {
return Fail(SEC_ERROR_BAD_DER);
}
}
value = parsedValue;
return Success;
}
@ -506,7 +537,7 @@ AlgorithmIdentifier(Input& input, SECAlgorithmID& algorithmID)
}
inline Result
CertificateSerialNumber(Input& input, /*out*/ SECItem& serialNumber)
CertificateSerialNumber(Input& input, /*out*/ SECItem& value)
{
// http://tools.ietf.org/html/rfc5280#section-4.1.2.2:
//
@ -519,7 +550,32 @@ CertificateSerialNumber(Input& input, /*out*/ SECItem& serialNumber)
// that are negative or zero. Certificate users SHOULD be prepared to
// gracefully handle such certificates."
return Integer(input, serialNumber);
uint16_t length;
if (ExpectTagAndGetLength(input, INTEGER, length) != Success) {
return Failure;
}
if (input.Skip(length, value) != Success) {
return Failure;
}
if (value.len == 0) {
return Fail(SEC_ERROR_BAD_DER);
}
// Check for overly-long encodings. If the first byte is 0x00 then the high
// bit on the second byte must be 1; otherwise the same *positive* value
// could be encoded without the leading 0x00 byte. If the first byte is 0xFF
// then the second byte must NOT have its high bit set; otherwise the same
// *negative* value could be encoded without the leading 0xFF byte.
if (value.len > 1) {
if ((value.data[0] == 0x00 && (value.data[1] & 0x80) == 0) ||
(value.data[0] == 0xff && (value.data[1] & 0x80) != 0)) {
return Fail(SEC_ERROR_BAD_DER);
}
}
return Success;
}
// x.509 and OCSP both use this same version numbering scheme, though OCSP

216
security/pkix/test/gtest/pkixder_universal_types_tests.cpp
View File

@ -22,14 +22,16 @@
* limitations under the License.
*/
#include <functional>
#include <limits>
#include <vector>
#include <gtest/gtest.h>
#include "pkix/bind.h"
#include "pkixder.h"
#include "stdint.h"
using namespace mozilla::pkix::der;
using namespace std;
namespace {
@ -76,17 +78,17 @@ TEST_F(pkixder_universal_types_tests, BooleanTrue42)
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
static const uint8_t DER_BOOLEAN_TRUE[] = {
0x01, // INTEGER
0x01, // length
0xff // true
};
TEST_F(pkixder_universal_types_tests, BooleanTrueFF)
{
const uint8_t DER_BOOLEAN_TRUE_FF[] = {
0x01, // INTEGER
0x01, // length
0xff // true
};
Input input;
ASSERT_EQ(Success,
input.Init(DER_BOOLEAN_TRUE_FF, sizeof DER_BOOLEAN_TRUE_FF));
input.Init(DER_BOOLEAN_TRUE, sizeof DER_BOOLEAN_TRUE));
bool value = false;
ASSERT_EQ(Success, Boolean(input, value));
@ -265,55 +267,118 @@ TEST_F(pkixder_universal_types_tests, GeneralizedTimeInvalidZeroLength)
ASSERT_EQ(SEC_ERROR_INVALID_TIME, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, Integer)
TEST_F(pkixder_universal_types_tests, Integer_0_127)
{
const uint8_t DER_INTEGUR[] = {
for (uint8_t i = 0; i <= 127; ++i) {
const uint8_t DER[] = {
0x02, // INTEGER
0x01, // length
i, // value
};
Input input;
ASSERT_EQ(Success, input.Init(DER, sizeof DER));
uint8_t value = i + 1; // initialize with a value that is NOT i.
ASSERT_EQ(Success, Integer(input, value));
ASSERT_EQ(i, value);
}
}
TEST_F(pkixder_universal_types_tests, Integer_Negative1)
{
// This is a valid integer value but our integer parser cannot parse
// negative values.
static const uint8_t DER[] = {
0x02, // INTEGER
0x01, // length
0xff, // -1 (two's complement)
};
Input input;
ASSERT_EQ(Success, input.Init(DER, sizeof DER));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, Integer_Negative128)
{
// This is a valid integer value but our integer parser cannot parse
// negative values.
static const uint8_t DER[] = {
0x02, // INTEGER
0x01, // length
0x80, // -128 (two's complement)
};
Input input;
ASSERT_EQ(Success, input.Init(DER, sizeof DER));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, Integer_128)
{
// This is a valid integer value but our integer parser cannot parse
// values that require more than one byte to encode.
static const uint8_t DER[] = {
0x02, // INTEGER
0x02, // length
0x00, 0x80 // 128
};
Input input;
ASSERT_EQ(Success, input.Init(DER, sizeof DER));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, Integer11223344)
{
// This is a valid integer value but our integer parser cannot parse
// values that require more than one byte to be encoded.
static const uint8_t DER[] = {
0x02, // INTEGER
0x04, // length
0x11, 0x22, 0x33, 0x44 // 0x11223344
};
Input input;
ASSERT_EQ(Success, input.Init(DER_INTEGUR, sizeof DER_INTEGUR));
ASSERT_EQ(Success, input.Init(DER, sizeof DER));
const uint8_t expectedItemData[] = { 0x11, 0x22, 0x33, 0x44 };
SECItem item;
memset(&item, 0x00, sizeof item);
ASSERT_EQ(Success, Integer(input, item));
ASSERT_EQ(siBuffer, item.type);
ASSERT_EQ((size_t) 4, item.len);
ASSERT_TRUE(item.data);
ASSERT_EQ(0, memcmp(item.data, expectedItemData, sizeof expectedItemData));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, OneByte)
TEST_F(pkixder_universal_types_tests, IntegerTruncatedOneByte)
{
const uint8_t DER_INTEGUR[] = {
const uint8_t DER_INTEGER_TRUNCATED[] = {
0x02, // INTEGER
0x01, // length
0x11 // 0x11
// MISSING DATA HERE
};
Input input;
ASSERT_EQ(Success, input.Init(DER_INTEGUR, sizeof DER_INTEGUR));
ASSERT_EQ(Success,
input.Init(DER_INTEGER_TRUNCATED, sizeof DER_INTEGER_TRUNCATED));
const uint8_t expectedItemData[] = { 0x11 };
SECItem item;
memset(&item, 0x00, sizeof item);
ASSERT_EQ(Success, Integer(input, item));
ASSERT_EQ(siBuffer, item.type);
ASSERT_EQ((size_t) 1, item.len);
ASSERT_TRUE(item.data);
ASSERT_EQ(0, memcmp(item.data, expectedItemData, sizeof expectedItemData));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, IntegerTruncated)
TEST_F(pkixder_universal_types_tests, IntegerTruncatedLarge)
{
const uint8_t DER_INTEGER_TRUNCATED[] = {
0x02, // INTEGER
@ -326,14 +391,9 @@ TEST_F(pkixder_universal_types_tests, IntegerTruncated)
ASSERT_EQ(Success,
input.Init(DER_INTEGER_TRUNCATED, sizeof DER_INTEGER_TRUNCATED));
SECItem item;
memset(&item, 0x00, sizeof item);
ASSERT_EQ(Failure, Integer(input, item));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
ASSERT_EQ(0, item.type);
ASSERT_EQ(0, item.len);
}
TEST_F(pkixder_universal_types_tests, IntegerZeroLength)
@ -346,9 +406,8 @@ TEST_F(pkixder_universal_types_tests, IntegerZeroLength)
Input input;
ASSERT_EQ(Success, input.Init(DER_INTEGER_ZERO_LENGTH,
sizeof DER_INTEGER_ZERO_LENGTH));
SECItem item;
ASSERT_EQ(Failure, Integer(input, item));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
@ -363,9 +422,9 @@ TEST_F(pkixder_universal_types_tests, IntegerOverlyLong1)
Input input;
ASSERT_EQ(Success, input.Init(DER_INTEGER_OVERLY_LONG1,
sizeof DER_INTEGER_OVERLY_LONG1));
SECItem item;
ASSERT_EQ(Failure, Integer(input, item));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, IntegerOverlyLong2)
@ -379,9 +438,60 @@ TEST_F(pkixder_universal_types_tests, IntegerOverlyLong2)
Input input;
ASSERT_EQ(Success, input.Init(DER_INTEGER_OVERLY_LONG2,
sizeof DER_INTEGER_OVERLY_LONG2));
uint8_t value;
ASSERT_EQ(Failure, Integer(input, value));
ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
}
SECItem item;
ASSERT_EQ(Failure, Integer(input, item));
TEST_F(pkixder_universal_types_tests, OptionalIntegerSupportedDefault)
{
// The input is a BOOLEAN and not INTEGER for the input so we'll not parse
// anything and instead use the default value.
Input input;
ASSERT_EQ(Success, input.Init(DER_BOOLEAN_TRUE, sizeof DER_BOOLEAN_TRUE));
long value = 1;
ASSERT_EQ(Success, OptionalInteger(input, -1, value));
ASSERT_EQ(-1, value);
bool boolValue;
ASSERT_EQ(Success, Boolean(input, boolValue));
}
TEST_F(pkixder_universal_types_tests, OptionalIntegerUnsupportedDefault)
{
// The same as the previous test, except with an unsupported default value
// passed in.
Input input;
ASSERT_EQ(Success, input.Init(DER_BOOLEAN_TRUE, sizeof DER_BOOLEAN_TRUE));
long value;
ASSERT_EQ(Failure, OptionalInteger(input, 0, value));
ASSERT_EQ(SEC_ERROR_INVALID_ARGS, PR_GetError());
}
TEST_F(pkixder_universal_types_tests, OptionalIntegerSupportedDefaultAtEnd)
{
static const uint8_t dummy = 1;
Input input;
ASSERT_EQ(Success, input.Init(&dummy, 0));
long value = 1;
ASSERT_EQ(Success, OptionalInteger(input, -1, value));
ASSERT_EQ(-1, value);
}
TEST_F(pkixder_universal_types_tests, OptionalIntegerNonDefaultValue)
{
static const uint8_t DER[] = {
0x02, // INTEGER
0x01, // length
0x00
};
Input input;
ASSERT_EQ(Success, input.Init(DER, sizeof DER));
long value = 2;
ASSERT_EQ(Success, OptionalInteger(input, -1, value));
ASSERT_EQ(0, value);
ASSERT_TRUE(input.AtEnd());
}
TEST_F(pkixder_universal_types_tests, Null)