Skip to content

Commit 0518482

Browse files
embhornembhorn
committed
Fix wolfSSL_sk_X509_OBJECT_deep_copy to check CTC_MAX_SKID_SIZE
1 parent 1d363f3 commit 0518482

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

‎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)