已关闭
fix(pki): harden X509 verification for certificate version #1657
balabala-123创建于 7月7日关闭于 7月15日
fix(pki): harden X509 verification for certificate version #1657
已关闭
balabala-123创建于 7月7日关闭于 7月15日
balabala-123
7月7日
fix(pki): harden X509 verification for certificate version
likedislike
当前Pull Request已关闭, 关闭人@liwei3013
Bbalabala-123
7月7日 创建了 pull request,commit 52f03d69
此处折叠了11条事件消息 查看更多
OopenHiTLS-bot成员
7月7日 添加了label:CI:successful
baoyi84930成员7月8日进行代码检视2
pki/x509_verify/src/hitls_x509_verify.c
@@ -951,0 +962,4 @@
962+ * out-of-band means or reject the certificate. Conforming implementations may choose to reject
963+ * all version 1 and version 2 intermediate certificates.)
964+ */
965+ if (curDepth == rootDepth) {
baoyi849307月8日评论:

[Code Guarder][high][likely] BasicConstraints pathLenConstraint is not enforced in chain verification

  • Severity: high
  • Confidence: likely
  • Reviewers: X
  • Location: pki/x509_verify/src/hitls_x509_verify.c:957-977

Problem
HITLS_X509_CertExt::maxPathLen is populated during extension parsing but X509_CheckExt never enforces it. A CA cert with pathLenConstraint=0 can still be accepted while allowing another intermediate CA below it, violating RFC 5280 chain depth semantics for path length.

Code

if (curDepth == rootDepth) {
    if (((cur->tbs.version == HITLS_X509_VERSION_3) &&
         ((curExt->extFlags & HITLS_X509_EXT_FLAG_BCONS) == 0 || !curExt->isCa))) {
        return HITLS_X509_ERR_VFY_INVALID_CA;
    }
} else {
    if (cur->tbs.version != HITLS_X509_VERSION_3) {
        return HITLS_X509_ERR_VFY_INTERCA_INVALID_VERSION;
    }
    if ((curExt->extFlags & HITLS_X509_EXT_FLAG_BCONS) == 0 || !curExt->isCa) {
        return HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS;
    }
}

Suggested Fix

if (cur->tbs.version == HITLS_X509_VERSION_3 && curExt->maxPathLen >= 0) {
    int32_t caBelow = curDepth - 1; // number of CA certs below current cert
    if (caBelow > curExt->maxPathLen) {
        BSL_ERR_PUSH_ERROR(HITLS_X509_ERR_VFY_INVALID_CA);
        return HITLS_X509_ERR_VFY_INVALID_CA;
    }
}
likedislike
balabala-123
7月14日 评论:
baoyi84930成员7月8日进行代码检视2
pki/x509_verify/src/hitls_x509_verify.c
@@ -947,0 +972,4 @@
972+ return HITLS_X509_ERR_VFY_INTERCA_INVALID_VERSION;
973+ }
974+ if ((curExt->extFlags & HITLS_X509_EXT_FLAG_BCONS) == 0 || !curExt->isCa) {
975+ return HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS;
baoyi849307月8日评论:

[Code Guarder][medium][likely] X509_CheckExt returns verification errors without pushing to error stack

  • Severity: medium
  • Confidence: likely
  • Reviewers: O
  • Location: pki/x509_verify/src/hitls_x509_verify.c:968-975

Problem
New verification checks in X509_CheckExt return distinct HITLS_X509_ERR_* values, but do not call BSL_ERR_PUSH_ERROR. This drops structured error context for callers that rely on the error stack for diagnostics/logging, unlike adjacent verification paths that always push before returning.

Code

if (curDepth == rootDepth) {
    if (((cur->tbs.version == HITLS_X509_VERSION_3) &&
         ((curExt->extFlags & HITLS_X509_EXT_FLAG_BCONS) == 0 || !curExt->isCa))) {
        return HITLS_X509_ERR_VFY_INVALID_CA;
    }
} else {
    if (cur->tbs.version != HITLS_X509_VERSION_3) {
        return HITLS_X509_ERR_VFY_INTERCA_INVALID_VERSION;
    }
    if ((curExt->extFlags & HITLS_X509_EXT_FLAG_BCONS) == 0 || !curExt->isCa) {
        return HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS;
    }
}

Suggested Fix

BSL_ERR_PUSH_ERROR(HITLS_X509_ERR_VFY_INVALID_CA);
return HITLS_X509_ERR_VFY_INVALID_CA;
...
BSL_ERR_PUSH_ERROR(HITLS_X509_ERR_VFY_INTERCA_INVALID_VERSION);
return HITLS_X509_ERR_VFY_INTERCA_INVALID_VERSION;
...
BSL_ERR_PUSH_ERROR(HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS);
return HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS;
likedislike
balabala-123
7月14日 评论:
baoyi84930成员7月8日进行代码检视3
testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.c
已过期
@@ -1029,0 +1034,4 @@
1034+ * @expect HITLS_X509_ERR_VFY_EXTENSIONS_REQUIRE_V3 for non-v3-with-extensions certs.
1035+ */
1036+/* BEGIN_CASE */
1037+void SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001(char *leafPath, char *interPath, char *rootPath, int exp)
baoyi849307月8日评论:

[Code Guarder][low][evaluate] Unwired SDV test case with undefined expected error token in documentation

  • Severity: low
  • Confidence: evaluate
  • Reviewers: O
  • Location: testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.c:1031-1037

Problem
SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001 is added but never referenced by test_suite_sdv_x509_vfy.data, so it never runs in the SDV driver. The doc comment also references HITLS_X509_ERR_VFY_EXTENSIONS_REQUIRE_V3, which is not defined in include/pki/hitls_pki_errno.h.

Code

/**
 * @test   SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001
 * @title  Reject v1/v2 certificates carrying extensions during verification.
 * @brief  Non-v3 certificates that carry X.509 extensions must be rejected in the verification phase
 * [@expect](https://gitcode.com/expect) HITLS_X509_ERR_VFY_EXTENSIONS_REQUIRE_V3 for non-v3-with-extensions certs.
 */
void SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001(char *leafPath, char *interPath, char *rootPath, int exp)

Suggested Fix

/* Add data entry in test_suite_sdv_x509_vfy.data, e.g.
SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001: ... :HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS
*/
/* Or remove the function until the feature/error code is explicitly added and wired. */
likedislike
System
系统消息系统
7月14日 评论:
balabala-123
7月14日 评论:
baoyi84930成员7月8日进行代码检视3
testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.c
已过期
@@ -1029,0 +1034,4 @@
1034+ * @expect HITLS_X509_ERR_VFY_EXTENSIONS_REQUIRE_V3 for non-v3-with-extensions certs.
1035+ */
1036+/* BEGIN_CASE */
1037+void SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001(char *leafPath, char *interPath, char *rootPath, int exp)
baoyi849307月8日评论:

[Code Guarder][low][evaluate] Unwired SDV test case with undefined expected error token in documentation

  • Severity: low
  • Confidence: evaluate
  • Reviewers: O
  • Location: testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.c:1031-1037

Problem
SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001 is added but never referenced by test_suite_sdv_x509_vfy.data, so it never runs in the SDV driver. The doc comment also references HITLS_X509_ERR_VFY_EXTENSIONS_REQUIRE_V3, which is not defined in include/pki/hitls_pki_errno.h.

Code

/**
 * @test   SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001
 * @title  Reject v1/v2 certificates carrying extensions during verification.
 * @brief  Non-v3 certificates that carry X.509 extensions must be rejected in the verification phase
 * [@expect](https://gitcode.com/expect) HITLS_X509_ERR_VFY_EXTENSIONS_REQUIRE_V3 for non-v3-with-extensions certs.
 */
void SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001(char *leafPath, char *interPath, char *rootPath, int exp)

Suggested Fix

/* Add data entry in test_suite_sdv_x509_vfy.data, e.g.
SDV_X509_VFY_EXTENSIONS_REQUIRE_V3_TC001: ... :HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS
*/
/* Or remove the function until the feature/error code is explicitly added and wired. */
likedislike
System
系统消息系统
7月14日 评论:
balabala-123
7月14日 评论:
baoyi84930成员7月8日进行代码检视2
testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.data
@@ -66,7 +66,7 @@ SDV_X509_BUILD_CERT_CHAIN_FUNC_TC008:"../testdata/cert/chain/rsa-v3/ca1.der":"..
6666 
6767SDV_X509_BUILD_CERT_CHAIN_FUNC_TC008 test crl have no cacrl vfy all
6868SDV_X509_BUILD_CERT_CHAIN_FUNC_TC008:"../testdata/cert/chain/rsa-v3/ca1.der":"../testdata/cert/chain/rsa-v3/inter.der":"../testdata/cert/chain/rsa-v3/end.der":"":"../testdata/cert/chain/rsa-v3/crl_v2.old.der":HITLS_X509_VFY_FLAG_CRL_ALL:HITLS_X509_ERR_CRL_NOT_FOUND
69-
69+ 
7070SDV_X509_BUILD_CERT_CHAIN_FUNC_TC008 test revoke endcert
7171SDV_X509_BUILD_CERT_CHAIN_FUNC_TC008:"../testdata/cert/chain/rsa-v3/ca1.der":"../testdata/cert/chain/rsa-v3/inter.der":"../testdata/cert/chain/rsa-v3/end.der":"":"../testdata/cert/chain/rsa-v3/crl_v1.der":HITLS_X509_VFY_FLAG_CRL_DEV:HITLS_X509_ERR_VFY_CERT_REVOKED
7272 
@@ -124,3 +124,18 @@ SDV_X509_CERT_VERIFY_BY_PUBKEY_FUNC_TC001:"../testdata/cert/chain/verify/rsa_cer
124124SDV_X509_CHECK_TIME_NEED_AFTER_TIME_FUNC_TC001 prevent aftertime bypass by flag tampering
125125SDV_X509_CHECK_TIME_NEED_AFTER_TIME_FUNC_TC001:"../testdata/cert/chain/verify/aftertime_ca.der":"../testdata/cert/chain/verify/aftertime_cert.der":"../testdata/cert/chain/verify/aftertime_crl.der"
126126 
127+SDV_X509_VFY_V1_INTER_CA_TC001: TC1 v1 intermediate without extensions rejected
baoyi849307月8日评论:

[Code Guarder][low][trusted] Added depth_suite certificate fixtures are unused by verification tests

  • Severity: low
  • Confidence: trusted
  • Reviewers: O
  • Location: testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.data:127-140

Problem
Certificate files under testcode/testdata/cert/chain/bcExt/depth_suite/ are added in the PR but no .data entry binds them to a runnable test case, so these artifacts are dead code/test data and unvalidated.

Code

SDV_X509_VFY_V1_INTER_CA_TC001: TC1 v1 intermediate without extensions rejected
SDV_X509_VFY_V1_INTER_CA_TC001:"../testdata/cert/chain/v1_inter/v3_leaf.der":"../testdata/cert/chain/v1_inter/v1_inter.der":"../testdata/cert/chain/v1_inter/v3_root.der":HITLS_X509_ERR_VFY_INTERCA_INVALID_VERSION
...
SDV_X509_VFY_V1_INTER_CA_TC001: TC5 v3 intermediate BasicConstraints CA=false
SDV_X509_VFY_V1_INTER_CA_TC001:"../testdata/cert/chain/bcExt/bc_leaf_ca_false.pem":"../testdata/cert/chain/bcExt/bc_inter_ca_false.pem":"../testdata/cert/chain/bcExt/bc_root_general.pem":HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS

Suggested Fix

Add depth_suite entries to this .data file and corresponding test cases, or remove the unused depth_suite PEM files:
../testdata/cert/chain/bcExt/depth_suite/depth_root.pem
../testdata/cert/chain/bcExt/depth_suite/depth_inter1.pem
../testdata/cert/chain/bcExt/depth_suite/depth_inter2.pem
../testdata/cert/chain/bcExt/depth_suite/depth_leaf_lvl1.pem
../testdata/cert/chain/bcExt/depth_suite/depth_leaf_lvl2.pem
likedislike
balabala-123
7月14日 评论:
baoyi84930成员7月8日进行代码检视2
testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.data
@@ -127,0 +133,4 @@
133+SDV_X509_VFY_V1_INTER_CA_TC001: TC3 v1 trust anchor without extensions accepted
134+SDV_X509_VFY_V1_INTER_CA_TC001:"../testdata/cert/chain/v1_inter/v3_leaf_b.der":"":"../testdata/cert/chain/v1_inter/v1_root.der":HITLS_PKI_SUCCESS
135+ 
136+SDV_X509_VFY_V1_INTER_CA_TC001: TC4 v3 intermediate missing BasicConstraints
baoyi849307月8日评论:

[Code Guarder][low][trusted] Added pathlen certificate fixtures are unused by verification tests

  • Severity: low
  • Confidence: trusted
  • Reviewers: O
  • Location: testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.data:127-140

Problem
Certificate files under testcode/testdata/cert/chain/bcExt/pathlen_* are added but not referenced by any SDV data-driven verification case, so intended pathLenConstraint behavior is untested despite new assets being introduced.

Code

SDV_X509_VFY_V1_INTER_CA_TC001: TC4 v3 intermediate missing BasicConstraints
SDV_X509_VFY_V1_INTER_CA_TC001:"../testdata/cert/chain/bcExt/bc_leaf_missing_bc.pem":"../testdata/cert/chain/bcExt/bc_inter_missing_bc.pem":"../testdata/cert/chain/bcExt/bc_root_general.pem":HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS
SDV_X509_VFY_V1_INTER_CA_TC001: TC5 v3 intermediate BasicConstraints CA=false
SDV_X509_VFY_V1_INTER_CA_TC001:"../testdata/cert/chain/bcExt/bc_leaf_ca_false.pem":"../testdata/cert/chain/bcExt/bc_inter_ca_false.pem":"../testdata/cert/chain/bcExt/bc_root_general.pem":HITLS_X509_ERR_VFY_INTERCA_INVALID_BCONS

Suggested Fix

Add .data entries and tests that consume:
../testdata/cert/chain/bcExt/pathlen_inter_lvl1.pem
../testdata/cert/chain/bcExt/pathlen_inter_lvl2.pem
../testdata/cert/chain/bcExt/pathlen_leaf_pl_exceed.pem
../testdata/cert/chain/bcExt/pathlen_multi_inter1.pem
../testdata/cert/chain/bcExt/pathlen_multi_inter2.pem
../testdata/cert/chain/bcExt/pathlen_multi_inter3.pem
../testdata/cert/chain/bcExt/pathlen_multi_leaf.pem
../testdata/cert/chain/bcExt/pathlen_multi_root.pem
../testdata/cert/chain/bcExt/pathlen_root_pl1.pem
likedislike
balabala-123
7月14日 评论:
Bbalabala-123
7月14日 update merge request[project id: 4195561, iid: 1657, commit_id: d6084447c4666fd5a74aa62476ce9f7a8a8a4328] virtual merging success
此处折叠了7条事件消息 查看更多
Lliwei3013成员
7月15日 关闭了 pull request
openHiTLS-bot成员
7月15日 评论:

openHiTLS-bot pushed a commit on branch openhitls-0.2 that referenced this pull request: 2d3b221d

likedislike