gtls: verify OCSP response signature in gtls_verify_ocsp_status#21677
Open
MegaManSec wants to merge 1 commit into
Open
gtls: verify OCSP response signature in gtls_verify_ocsp_status#21677MegaManSec wants to merge 1 commit into
MegaManSec wants to merge 1 commit into
Conversation
Since aeb1a28 ("gtls: fix OCSP stapling management"), the function parses the stapled OCSP response and reads the certificate status via gnutls_ocsp_resp_get_single(), but never calls gnutls_ocsp_resp_verify() or gnutls_ocsp_resp_verify_direct(). A response with a forged or corrupted signature is accepted without question. Fix by calling gnutls_ocsp_resp_verify() against the trust list obtained from the session credentials immediately after gnutls_ocsp_resp_import(). This handles both directly-signed responses and delegated OCSP responders without requiring the issuer certificate to be present in the peer chain. The missing check only affects the CURLOPT_SSL_VERIFYSTATUS code path when CURLOPT_SSL_VERIFYPEER is disabled. With peer verification enabled, gnutls_certificate_verify_peers2() independently catches the invalid response via GNUTLS_CERT_INVALID_OCSP_STATUS before gtls_verify_ocsp_status() is reached. As a result, no attack is possible that is not already trivially achievable without OCSP stapling when peer verification is off. This is a correctness and consistency fix, not a security vulnerability. Reported-by: Joshua Rogers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Since
aeb1a281ca("gtls: fix OCSP stapling management"),gtls_verify_ocsp_statusparses the stapled OCSP response and reads the certificate status viagnutls_ocsp_resp_get_single(), but never callsgnutls_ocsp_resp_verify()orgnutls_ocsp_resp_verify_direct(). A response with a forged or corrupted signature is accepted without question.Before
aeb1a281ca, the code calledgnutls_ocsp_status_request_is_checked(session, 0), which only returns non-zero after GnuTLS has internally verified the OCSP response during the handshake. The refactor replaced this with manual parsing that omitted the verification step.Fix
Call
gnutls_ocsp_resp_verify()against the trust list obtained from the session credentials immediately aftergnutls_ocsp_resp_import(). This handles both directly-signed responses and delegated OCSP responders without requiring the issuer certificate to be present in the peer chain.Scope
The missing check only affects the
CURLOPT_SSL_VERIFYSTATUScode path whenCURLOPT_SSL_VERIFYPEERis disabled. With peer verification enabled,gnutls_certificate_verify_peers2()independently catches an invalid OCSP response viaGNUTLS_CERT_INVALID_OCSP_STATUSbeforegtls_verify_ocsp_status()is reached.Because a caller that has disabled peer verification has already abandoned certificate chain trust, there is no attack scenario enabled by the missing check that is not already trivially achievable by other means. This is a correctness and backend-consistency fix (the OpenSSL path calls
OCSP_basic_verify()unconditionally), not a security vulnerability.