Preserve server TLS request flags when TLS is disabled (prevent fail-open) - #313
Draft
rgerhards wants to merge 2 commits into
Draft
Preserve server TLS request flags when TLS is disabled (prevent fail-open)#313rgerhards wants to merge 2 commits into
rgerhards wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Prevents a TLS “fail-open” downgrade in non-TLS builds by recording that TLS (and TLS+ZIP) was requested even when the TLS enable APIs return RELP_RET_ERR_NO_TLS, so relpSrvRun() won’t silently start a plaintext listener.
Changes:
- Set
relpSrv_t::bEnableTLSunconditionally inrelpSrvEnableTLS2(), while still returningRELP_RET_ERR_NO_TLSin non-TLS builds. - Set
relpSrv_t::bEnableTLSZipunconditionally inrelpSrvEnableTLSZip2(), while still returningRELP_RET_ERR_NO_TLSin non-TLS builds.
Suppressed comments (1)
src/relpsrv.c:369
relpSrvEnableTLSZip2()now dereferencespThisunconditionally (even in non-TLS builds), but it doesn’t validate the server handle (contrast with most otherrelpSrv*setters which useRELPOBJ_assert(pThis, Srv)). Adding the assertion keeps the API consistent and catches invalid handles in debug builds. Also update the#endifcomment to match the new#if !(...)guard.
ENTER_RELPFUNC;
pThis->bEnableTLSZip = 1;
#if !(defined(ENABLE_TLS) || defined(ENABLE_TLS_OPENSSL))
iRet = RELP_RET_ERR_NO_TLS;
#endif /* #ifdef ENABLE_TLS | ENABLE_TLS_OPENSSL */
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
355
to
359
| ENTER_RELPFUNC; | ||
| #if defined(ENABLE_TLS) || defined(ENABLE_TLS_OPENSSL) | ||
| pThis->bEnableTLS = 1; | ||
| #else | ||
| #if !(defined(ENABLE_TLS) || defined(ENABLE_TLS_OPENSSL)) | ||
| iRet = RELP_RET_ERR_NO_TLS; | ||
| #endif /* #ifdef ENABLE_TLS | ENABLE_TLS_OPENSSL */ |
rgerhards
force-pushed
the
codex/propose-fix-for-tls-request-issue
branch
from
August 20, 2026 16:26
8683114 to
b052140
Compare
rgerhards
force-pushed
the
codex/propose-fix-for-tls-request-issue
branch
from
August 20, 2026 16:39
b052140 to
1ca58dc
Compare
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
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.
Motivation
RELP_RET_ERR_NO_TLSbut not record that TLS was requested, allowingrelpSrvRun()to skip TLS setup and start a plaintext listener when librelp is built without TLS.Description
relpSrvEnableTLS2andrelpSrvEnableTLSZip2to setpThis->bEnableTLS/pThis->bEnableTLSZipunconditionally and still returnRELP_RET_ERR_NO_TLSin builds without TLS using a#if !(defined(ENABLE_TLS) || defined(ENABLE_TLS_OPENSSL))guard for the error path.Testing
git diff --checkwhich reported no whitespace or diff errors; this succeeded.autoreconf -fviwhich failed in the current environment becauseaclocalis not available, so full autotools regeneration could not be executed here.Codex Task
Summary by cubic
Prevents a fail-open downgrade when enabling server TLS on builds without TLS. Previously
relpSrvEnableTLS2/relpSrvEnableTLSZip2returnedRELP_RET_ERR_NO_TLSbut did not record the TLS request, sorelpSrvRun()started a plaintext listener; now the enable calls always set the request flags andrelpSrvRun()returnsRELP_RET_ERR_NO_TLSwithout starting a listener.bEnableTLS/bEnableTLSZipinrelpSrvEnableTLS2/relpSrvEnableTLSZip2; still returnRELP_RET_ERR_NO_TLSwhen TLS is unsupported; addRELPOBJ_assert(pThis, Srv)to validate input.RELP_RET_ERR_NO_TLSand must not start a listener.tls-request-no-tls(built and run only when!ENABLE_TLS_GENERIC) to verify flags are set and no TCP listener is created.Written for commit 1ca58dc. Summary will update on new commits.