Fix GnuTLS certvalid certificate validation

[rgerhards]

Motivation

• Close a GnuTLS authentication bypass where certvalid mode accepted any presented X.509 certificate without verifying the trust chain or status.
• Ensure certvalid semantics (a valid, trusted certificate chain) are enforced consistently between OpenSSL and GnuTLS code paths.

Description

• In relpTcpVerifyCertificateCallback call gnutls_certificate_verify_peers2() when pThis->authmode == eRelpAuthMode_CertValid and reject the handshake on non-zero status or error return, mirroring the OpenSSL path (change in src/tcp.c).
• Update the TLS cert-chain test to explicitly pass the CA file to the testbench so the test exercises the intended cert-chain validation behavior (edit in tests/tls-basic-certchain.sh).
• Add a regression test tests/tls-certvalid-untrusted.sh and register it in the test suite to verify certvalid rejects expired/untrusted client certificates.

Testing

• Ran autoreconf -fi && ./configure --enable-tls && make -j2 and the build completed successfully with GNUTLS and OpenSSL enabled.
• Executed make check for TLS tests including tls-basic-certvalid.sh, tls-basic-certvalid-mixed.sh, tls-basic-certchain.sh and the new tls-certvalid-untrusted.sh; all TLS tests passed in the final test run.
• Executed the new regression script (cd tests && ./tls-certvalid-untrusted.sh) and it succeeded for both GnuTLS and OpenSSL backends.

---

Codex Task

---

Summary by cubic

Fixes an authentication bypass in the GnuTLS certvalid path: previously any client certificate was accepted; now the peer chain is verified and invalid/untrusted/expired certificates are rejected, matching OpenSSL.

• In relpTcpVerifyCertificateCallback, call gnutls_certificate_verify_peers2() for certvalid and fail the handshake on error or non‑zero status with "certificate validation failed".
• Tests: pass a CA file in tls-basic-certchain.sh; add tls-certvalid-untrusted.sh, isolate its error log, assert the failure message, and register it in the suite.
• Migration: when using GnuTLS with certvalid, provide a CA file/bundle; connections relying on untrusted or expired certificates will now fail.

^{Written for commit c0bb2f0. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 4 files

Confidence score: 5/5

• In src/tcp.c, the new certvalid branch duplicates the existing certificate-verification error handling, which could allow the two paths to diverge over time; consolidate the logic or verify both blocks remain behaviorally identical.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/tcp.c">

<violation number="1" location="src/tcp.c:2748">
P3: The new certvalid branch duplicates the `gnutls_certificate_verify_peers2` + `callOnAuthErr(..., "certificate validation failed", RELP_RET_AUTH_CERT_INVL)` + `GNUTLS_E_CERTIFICATE_ERROR` block already in relpTcpChkPeerName_gtls (src/tcp.c ~lines 2649-2661). Both branches now perform the identical chain-status check and emit the same error. Consider factoring a shared helper (e.g. relpTcpVerifyCertChain(pThis)) and calling it from both the name and certvalid paths so future verification changes stay in one place.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The new certvalid branch duplicates the gnutls_certificate_verify_peers2 + callOnAuthErr(..., "certificate validation failed", RELP_RET_AUTH_CERT_INVL) + GNUTLS_E_CERTIFICATE_ERROR block already in relpTcpChkPeerName_gtls (src/tcp.c ~lines 2649-2661). Both branches now perform the identical chain-status check and emit the same error. Consider factoring a shared helper (e.g. relpTcpVerifyCertChain(pThis)) and calling it from both the name and certvalid paths so future verification changes stay in one place.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/tcp.c, line 2748:

<comment>The new certvalid branch duplicates the `gnutls_certificate_verify_peers2` + `callOnAuthErr(..., "certificate validation failed", RELP_RET_AUTH_CERT_INVL)` + `GNUTLS_E_CERTIFICATE_ERROR` block already in relpTcpChkPeerName_gtls (src/tcp.c ~lines 2649-2661). Both branches now perform the identical chain-status check and emit the same error. Consider factoring a shared helper (e.g. relpTcpVerifyCertChain(pThis)) and calling it from both the name and certvalid paths so future verification changes stay in one place.</comment>

<file context>
@@ -2743,6 +2743,14 @@ relpTcpVerifyCertificateCallback(gnutls_session_t session)
 		r = relpTcpChkPeerName(pThis, (void*)cert);
+	} else if(pThis->authmode == eRelpAuthMode_CertValid) {
+		unsigned int status = 0;
+		r = gnutls_certificate_verify_peers2(pThis->session, &status);
+		if(r < 0 || status != 0) {
+			callOnAuthErr(pThis, "", "certificate validation failed",
</file context>

[rgerhards]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: c0bb2f049f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".