Skip to content

[~] fix issue #693: fix duplicate condition in cert verify callback#805

Open
Yanmei-Liu wants to merge 1 commit into
alibaba:mainfrom
Yanmei-Liu:fix/cert_verify_duplicate_condition
Open

[~] fix issue #693: fix duplicate condition in cert verify callback#805
Yanmei-Liu wants to merge 1 commit into
alibaba:mainfrom
Yanmei-Liu:fix/cert_verify_duplicate_condition

Conversation

@Yanmei-Liu

Copy link
Copy Markdown
Collaborator

Summary

Fix a copy-paste bug in xqc_ssl_cert_verify_cb where X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY (error 20) was checked twice, while X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT (error 2) was never checked.

These are two semantically different X509 verification errors:

  • Error 20 (X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY): issuer certificate could not be found in the local trust store
  • Error 2 (X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT): issuer of a looked-up certificate could not be found (chain broken at non-leaf)

The duplicate condition meant that when error 2 occurred, the application-layer cert_verify_cb was never invoked -- connections were rejected without giving the application a chance to handle the error (e.g., for certificate pinning scenarios).

Changes

  • src/tls/xqc_tls.c: Extract error tolerability logic into xqc_cert_verify_err_tolerable(), fix the duplicate condition
  • src/tls/xqc_tls.h: Declare the new helper function
  • tests/unittest/xqc_tls_test.c: Add unit tests covering all branches of the error filtering logic

Test plan

  • Unit tests for xqc_cert_verify_err_tolerable() added:
    • Error 20 (UNABLE_TO_GET_ISSUER_CERT_LOCALLY) tolerated
    • Error 2 (UNABLE_TO_GET_ISSUER_CERT) tolerated -- was previously broken
    • Self-signed certs (error 18, 19) accepted with ALLOW_SELF_SIGNED flag
    • Self-signed certs rejected without the flag
    • Expired cert (error 10) always rejected
    • Revoked cert (error 23) always rejected
  • Existing cert_verify and crypto_error_cert_verify integration tests in case_test.sh cover end-to-end paths
  • CI (build + run_tests + case_test.sh) passes

Fixes #693

…lback

The certificate verification callback xqc_ssl_cert_verify_cb had a
copy-paste error where X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY
(error 20) was checked twice. The second condition should have been
X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT (error 2).

This caused connections to fail when the issuer certificate was missing
from the chain (error 2), because the application-layer cert_verify_cb
was never invoked for that error class.

Fix:
- Replace the duplicate check with X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT
- Extract error tolerability logic into xqc_cert_verify_err_tolerable()
  for testability and clarity
- Add unit tests covering all branches: error 2/20 tolerated,
  self-signed allowed/rejected with flag, expired/revoked rejected

Fixes alibaba#693
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 证书验证回调重复条件【低级别】

1 participant