Skip to content

Commit 39a478d

Browse files
embhornembhorn
committed
Fix wolfSSL_sk_X509_OBJECT_deep_copy to check CTC_MAX_SKID_SIZE
1 parent b17755b commit 39a478d

3 files changed

Lines changed: 194 additions & 6 deletions

File tree

‎src/x509.c‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11711,14 +11711,23 @@ WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* wolfSSL_sk_X509_OBJECT_deep_copy(
1171111711
cert->isCA = req->isCa;
1171211712
cert->basicConstSet = req->basicConstSet;
1171311713
#ifdef WOLFSSL_CERT_EXT
11714-
if (req->subjKeyIdSz != 0) {
11715-
XMEMCPY(cert->skid, req->subjKeyId, req->subjKeyIdSz);
11714+
if (req->subjKeyIdSz <= CTC_MAX_SKID_SIZE) {
11715+
if (req->subjKeyId) {
11716+
XMEMCPY(cert->skid, req->subjKeyId, req->subjKeyIdSz);
11717+
}
1171611718
cert->skidSz = (int)req->subjKeyIdSz;
1171711719
}
11718-
if (req->keyUsageSet)
11719-
cert->keyUsage = req->keyUsage;
11720+
else {
11721+
WOLFSSL_MSG("Subject Key ID too large");
11722+
WOLFSSL_ERROR_VERBOSE(BUFFER_E);
11723+
ret = WOLFSSL_FAILURE;
11724+
}
11725+
if (ret == WOLFSSL_SUCCESS) {
11726+
if (req->keyUsageSet)
11727+
cert->keyUsage = req->keyUsage;
1172011728

11721-
cert->extKeyUsage = req->extKeyUsage;
11729+
cert->extKeyUsage = req->extKeyUsage;
11730+
}
1172211731
#endif
1172311732

1172411733
XMEMCPY(cert->challengePw, req->challengePw, CTC_NAME_SIZE);

‎tests/api/test_x509.c‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,3 +878,180 @@ int test_x509_CertFromX509_akid_overflow(void)
878878
#endif
879879
return EXPECT_RESULT();
880880
}
881+
882+
/* Test that ReqCertFromX509 rejects an oversized SubjectKeyIdentifier
883+
* extension when re-encoding a parsed CSR. */
884+
int test_x509_ReqCertFromX509_skid_overflow(void)
885+
{
886+
EXPECT_DECLS;
887+
#if defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_CERT_GEN) && \
888+
defined(WOLFSSL_CERT_REQ) && !defined(NO_BIO) && \
889+
(defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL))
890+
#ifdef WOLFSSL_SMALL_STACK
891+
unsigned char* buf = NULL;
892+
#else
893+
unsigned char buf[16384];
894+
#endif
895+
size_t pos = 0;
896+
WOLFSSL_X509* req = NULL;
897+
WOLFSSL_BIO* bio = NULL;
898+
unsigned char* der = NULL;
899+
900+
#ifdef WOLFSSL_SMALL_STACK
901+
buf = (unsigned char*)XMALLOC(16384, NULL, DYNAMIC_TYPE_TMP_BUFFER);
902+
ExpectNotNull(buf);
903+
if (buf == NULL)
904+
return EXPECT_RESULT();
905+
#endif
906+
907+
#define PUT1(b) do { buf[pos++] = (b); } while(0)
908+
#define PUTN(p, n) do { XMEMCPY(buf + pos, (p), (n)); pos += (n); } while(0)
909+
910+
#define TLV_HDR(tag, n, out, hlen) do { \
911+
size_t _i = 0; \
912+
(out)[_i++] = (tag); \
913+
if ((n) < 0x80u) { (out)[_i++] = (unsigned char)(n); } \
914+
else if ((n) < 0x100u) { (out)[_i++] = 0x81; \
915+
(out)[_i++] = (unsigned char)(n); } \
916+
else if ((n) < 0x10000u) { (out)[_i++] = 0x82; \
917+
(out)[_i++] = (unsigned char)((n)>>8); \
918+
(out)[_i++] = (unsigned char)(n); } \
919+
(hlen) = _i; \
920+
} while(0)
921+
922+
#define WRAP(start, tag) do { \
923+
size_t _len = pos - (start); \
924+
unsigned char _hdr[6]; size_t _hlen; \
925+
TLV_HDR((tag), _len, _hdr, _hlen); \
926+
XMEMMOVE(buf + (start) + _hlen, buf + (start), _len); \
927+
XMEMCPY(buf + (start), _hdr, _hlen); \
928+
pos += _hlen; \
929+
} while(0)
930+
931+
{
932+
size_t req_start = pos;
933+
size_t s;
934+
int i;
935+
936+
/* CertificationRequestInfo */
937+
s = pos;
938+
/* version = v1 */
939+
PUT1(0x02); PUT1(0x01); PUT1(0x00);
940+
941+
/* subject: CN=A */
942+
{
943+
size_t name = pos, rdn = pos, atv = pos;
944+
unsigned char cn[] = {0x06,0x03,0x55,0x04,0x03};
945+
PUTN(cn, sizeof(cn));
946+
PUT1(0x0C); PUT1(0x01); PUT1('A');
947+
WRAP(atv, 0x30);
948+
WRAP(rdn, 0x31);
949+
WRAP(name, 0x30);
950+
}
951+
952+
/* subjectPublicKeyInfo: EC P-256 with generator point */
953+
{
954+
size_t spki = pos, alg = pos, bs;
955+
unsigned char ecpk[] = {0x06,0x07,0x2A,0x86,0x48,0xCE,
956+
0x3D,0x02,0x01};
957+
unsigned char p256[] = {0x06,0x08,0x2A,0x86,0x48,0xCE,
958+
0x3D,0x03,0x01,0x07};
959+
static const unsigned char p256G[64] = {
960+
0x6B,0x17,0xD1,0xF2,0xE1,0x2C,0x42,0x47,
961+
0xF8,0xBC,0xE6,0xE5,0x63,0xA4,0x40,0xF2,
962+
0x77,0x03,0x7D,0x81,0x2D,0xEB,0x33,0xA0,
963+
0xF4,0xA1,0x39,0x45,0xD8,0x98,0xC2,0x96,
964+
0x4F,0xE3,0x42,0xE2,0xFE,0x1A,0x7F,0x9B,
965+
0x8E,0xE7,0xEB,0x4A,0x7C,0x0F,0x9E,0x16,
966+
0x2B,0xCE,0x33,0x57,0x6B,0x31,0x5E,0xCE,
967+
0xCB,0xB6,0x40,0x68,0x37,0xBF,0x51,0xF5
968+
};
969+
PUTN(ecpk, sizeof(ecpk));
970+
PUTN(p256, sizeof(p256));
971+
WRAP(alg, 0x30);
972+
bs = pos;
973+
PUT1(0x00);
974+
PUT1(0x04);
975+
PUTN(p256G, sizeof(p256G));
976+
WRAP(bs, 0x03);
977+
WRAP(spki, 0x30);
978+
}
979+
980+
/* attributes [0]: extensionRequest(subjectKeyIdentifier) */
981+
{
982+
size_t attrs = pos, attr = pos, values, exts;
983+
size_t ext, extVal, skid;
984+
unsigned char extReqOid[] = {0x06,0x09,0x2A,0x86,0x48,0x86,
985+
0xF7,0x0D,0x01,0x09,0x0E};
986+
unsigned char skidOid[] = {0x06,0x03,0x55,0x1D,0x0E};
987+
988+
PUTN(extReqOid, sizeof(extReqOid));
989+
values = pos;
990+
exts = pos;
991+
ext = pos;
992+
PUTN(skidOid, sizeof(skidOid));
993+
extVal = pos;
994+
skid = pos;
995+
for (i = 0; i < CTC_MAX_SKID_SIZE + 1; i++) {
996+
PUT1((unsigned char)(0x41 + (i % 16)));
997+
}
998+
WRAP(skid, 0x04);
999+
WRAP(extVal, 0x04);
1000+
WRAP(ext, 0x30);
1001+
WRAP(exts, 0x30);
1002+
WRAP(values, 0x31);
1003+
WRAP(attr, 0x30);
1004+
WRAP(attrs, 0xA0);
1005+
}
1006+
WRAP(s, 0x30);
1007+
1008+
/* signatureAlgorithm: ecdsa-with-SHA256 */
1009+
s = pos;
1010+
{
1011+
unsigned char oid[] = {0x06,0x08,0x2A,0x86,0x48,0xCE,
1012+
0x3D,0x04,0x03,0x02};
1013+
PUTN(oid, sizeof(oid));
1014+
}
1015+
WRAP(s, 0x30);
1016+
1017+
/* signatureValue: parser does not verify this for the regression */
1018+
s = pos;
1019+
{
1020+
PUT1(0x00);
1021+
size_t sig = pos;
1022+
PUT1(0x02); PUT1(0x01); PUT1(0x01);
1023+
PUT1(0x02); PUT1(0x01); PUT1(0x01);
1024+
WRAP(sig, 0x30);
1025+
}
1026+
WRAP(s, 0x03);
1027+
WRAP(req_start, 0x30);
1028+
}
1029+
1030+
req = wolfSSL_X509_REQ_d2i(NULL, buf, (int)pos);
1031+
ExpectNotNull(req);
1032+
if (req != NULL) {
1033+
ExpectIntEQ((int)req->subjKeyIdSz, CTC_MAX_SKID_SIZE + 1);
1034+
}
1035+
1036+
bio = wolfSSL_BIO_new(wolfSSL_BIO_s_mem());
1037+
ExpectNotNull(bio);
1038+
if ((req != NULL) && (bio != NULL)) {
1039+
ExpectIntEQ(wolfSSL_i2d_X509_REQ_bio(bio, req), WOLFSSL_FAILURE);
1040+
ExpectIntEQ(wolfSSL_i2d_X509_REQ(req, &der),
1041+
WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
1042+
ExpectNull(der);
1043+
}
1044+
1045+
wolfSSL_BIO_free(bio);
1046+
wolfSSL_X509_free(req);
1047+
#ifdef WOLFSSL_SMALL_STACK
1048+
XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
1049+
#endif
1050+
1051+
#undef PUT1
1052+
#undef PUTN
1053+
#undef TLV_HDR
1054+
#undef WRAP
1055+
#endif
1056+
return EXPECT_RESULT();
1057+
}

‎tests/api/test_x509.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ int test_x509_set_serialNumber(void);
2828
int test_x509_verify_cert_hostname_check(void);
2929
int test_x509_time_field_overread_via_tls(void);
3030
int test_x509_CertFromX509_akid_overflow(void);
31+
int test_x509_ReqCertFromX509_skid_overflow(void);
3132

3233
#define TEST_X509_DECLS \
3334
TEST_DECL_GROUP("x509", test_x509_rfc2818_verification_callback), \
3435
TEST_DECL_GROUP("x509", test_x509_GetCAByAKID), \
3536
TEST_DECL_GROUP("x509", test_x509_set_serialNumber), \
3637
TEST_DECL_GROUP("x509", test_x509_verify_cert_hostname_check), \
3738
TEST_DECL_GROUP("x509", test_x509_time_field_overread_via_tls), \
38-
TEST_DECL_GROUP("x509", test_x509_CertFromX509_akid_overflow)
39+
TEST_DECL_GROUP("x509", test_x509_CertFromX509_akid_overflow), \
40+
TEST_DECL_GROUP("x509", test_x509_ReqCertFromX509_skid_overflow)
3941

4042
#endif /* WOLFCRYPT_TEST_X509_H */

0 commit comments

Comments
 (0)