Reset `http_buf_len` on NTLM proxy retry to prevent assert crash by parfeon · Pull Request #252 · pubnub/c-core

parfeon · 2026-07-27T14:17:12Z

feat(ntlm): parse domain from username in SSPI identity for NTLM proxy auth

fill_sspi_identity now splits DOMAIN\user and user@domain formats into separate User and
Domain fields so proxies requiring domain-qualified credentials can authenticate.

fix(proxy): reset http_buf_len on NTLM proxy retry to prevent assert crash

Reset http_buf_len to 0 in pbproxyFinRetry so the stale 407 body length does not trip the
assert guarding proxy_saved_path memcpy on the next CONNECT attempt.

fix(ntlm): correct inverted SSPI max-token guard in pbntlm_packer_sspi.c

The warning fired on the normal case instead of when SSPI needs more space than our buffer provides.
Flip the comparison and update the log message.

refactor(ntlm): use dynamic allocation for Type-2 challenge decode in pbntlm_core_handle

Replace the 512-byte stack buffer with pbbase64_decode_alloc_std so large NTLM Type-2 challenges
from domain-joined proxies are no longer silently dropped.

refactor(ntlm): raise PUBNUB_NTLM_MAX_TOKEN from 1024 to 4096

NTLMv2 Type-3 with extended target info and MIC frequently exceeds 1024 bytes. Raised to 4096 to
cover all known SSPI scenarios.

test(proxy): add NTLM proxy regression tests

Add tests for oversized 407 body triggering retry-path assert, and direct pbntlm_core_handle
tests with 600, 2048, and 4200 byte challenges.

…y auth `fill_sspi_identity` now splits `DOMAIN\user` and `user@domain` formats into separate User and Domain fields so proxies requiring domain-qualified credentials can authenticate. fix(proxy): reset `http_buf_len` on NTLM proxy retry to prevent assert crash Reset `http_buf_len` to 0 in `pbproxyFinRetry` so the stale 407 body length does not trip the assert guarding `proxy_saved_path` memcpy on the next CONNECT attempt. fix(ntlm): correct inverted SSPI max-token guard in `pbntlm_packer_sspi.c` The warning fired on the normal case instead of when SSPI needs more space than our buffer provides. Flip the comparison and update the log message. refactor(ntlm): use dynamic allocation for Type-2 challenge decode in `pbntlm_core_handle` Replace the 512-byte stack buffer with `pbbase64_decode_alloc_std` so large NTLM Type-2 challenges from domain-joined proxies are no longer silently dropped. refactor(ntlm): raise `PUBNUB_NTLM_MAX_TOKEN` from 1024 to 4096 NTLMv2 Type-3 with extended target info and MIC frequently exceeds 1024 bytes. Raised to 4096 to cover all known SSPI scenarios. test(proxy): add NTLM proxy regression tests Add tests for oversized 407 body triggering retry-path assert, and direct `pbntlm_core_handle` tests with 600, 2048, and 4200 byte challenges.

…inkage Add two sample apps demonstrating synchronous publish through an NTLM proxy: manual configuration with explicit host/port/credentials and automatic detection from Windows system settings. build(proxy): link `pbntlm_packer_sspi.c` on Windows instead of the stub `pbntlm_packer_std.c` Move `pbntlm_packer_std.c` to `PROXY_SOURCE_FILES_POSIX` and add `pbntlm_packer_sspi.c` to `PROXY_SOURCE_FILES_WINDOWS` so the Windows library build has functional NTLM authentication.

Xavrax

Overall looks good to me.
Verify my comments and can be released.

BTW. I see changes in makefiles, shouldn't we update CMake too?

Xavrax · 2026-07-30T18:27:52Z

+    data = pbbase64_decode_alloc_std(base64_msg, length);
+    if (NULL == data.ptr) {
+        PUBNUB_LOG_ERROR(pb, "Base64 decode (alloc) of NTLM challenge failed");


I like these change in case of readability, however we're losing potentially part of the info (I see that it might return values like -14 and -15. (which is meaningless without a code to debug, but still).

Sadly, we will loose those but I'm not really sure about possibility that it will happen here.

Xavrax · 2026-07-30T18:32:57Z

+    if (sspi_max_token > PUBNUB_NTLM_MAX_TOKEN) {
        PUBNUB_LOG_WARNING(
            context,
-            "NTLM token for SSPI is smaller (%lu) that maximum allowed (%d).",
+            "SSPI max token (%lu) exceeds PUBNUB_NTLM_MAX_TOKEN (%d). "
+            "Type-2 challenges larger than %d bytes will be rejected.",
            sspi_max_token,
+            PUBNUB_NTLM_MAX_TOKEN,
            PUBNUB_NTLM_MAX_TOKEN);
    }


That's kinda specific change. Why we reverse that? (I mean, why the previous log is no longer needed)

Well, before there were strange warning, that token surprisingly smaller than configured max limit - there is nothing wrong with token to be smaller than allocated maximum size :)
It was just misleading.

Xavrax · 2026-07-30T19:00:37Z

        pbntlm_core_deinit(pb);
        return;
    }
    (void)pbntlm_unpack_type2(pb, &pb->ntlm_context, data);


Are we sure that we can omit this value?
As I understood, it might fail for SPI build (passes the std one).

As when it fails on SPI, there is assertion in unpack_type3 which verifies if the size of data is higher than 0 (whereas fail of this function does it).

Ofc, handling error earlier won't safe the whole thing, but it will be consistent between build types.

Good catch!

Added error handling - now we check the return value and deinit/abort on failure, consistent with the other error paths in this function. Also added a test for invalid base64 input.

Xavrax · 2026-07-30T19:04:12Z


 /** Maximum supported length of the NTLM token (message) */
-#define PUBNUB_NTLM_MAX_TOKEN 1024
+#define PUBNUB_NTLM_MAX_TOKEN 4096


We extended the maximum amount of characters for the proxy however at core/pubnub_netcore.c:1340, we have defined header variable that is hardcoded to 1024.

You're right.

Added PUBNUB_PROXY_AUTH_HEADER_MAX_SIZE macro that derives the proper buffer size from PUBNUB_NTLM_MAX_TOKEN (base64 expansion + header prefix room) and used it for hedr in pubnub_netcore.c.

Check the return value of `pbntlm_unpack_type2` and abort the NTLM handshake on failure instead of advancing to the Authenticate state with empty token data. fix(proxy): size proxy auth header buffer for base64-encoded NTLM tokens Add `PUBNUB_PROXY_AUTH_HEADER_MAX_SIZE` macro derived from `PUBNUB_NTLM_MAX_TOKEN` and use it for the `hedr` buffer in `pubnub_netcore.c` so it can hold a full base64-encoded 4096-byte token. test(proxy): add invalid base64 failure path test for `pbntlm_core_handle` Add `NTLM_core_handle_rejects_invalid_base64` and update the oversized-token test to verify the context is properly deinited on unpack failure.

parfeon · 2026-07-30T20:22:36Z

BTW. I see changes in makefiles, shouldn't we update CMake too?

Those are test apps for manual NTLM testing

parfeon · 2026-07-30T20:22:45Z

@pubnub-release-bot release

pubnub-release-bot · 2026-07-30T20:43:25Z

🚀 Release successfully completed 🚀

parfeon added 2 commits July 27, 2026 16:50

parfeon self-assigned this Jul 27, 2026

parfeon requested review from KGronek-Pubnub and Xavrax as code owners July 27, 2026 14:17

parfeon added status: done This issue is considered resolved. priority: high This PR should be reviewed ASAP. type: feature This PR contains new feature. type: fix This PR contains fixes to existing features. labels Jul 27, 2026

Xavrax approved these changes Jul 30, 2026

View reviewed changes

parfeon and others added 2 commits July 30, 2026 22:38

Merge branch 'master' into CLEN-3631-fix-proxy-retry-assertion

a97252c

pubnub-release-bot and others added 2 commits July 30, 2026 20:23

PubNub SDK v7.3.0 release.

e530765

docs(pubnub-yml): fix change log entry format

8164322

parfeon merged commit 80cd7be into master Jul 30, 2026
7 checks passed

parfeon deleted the CLEN-3631-fix-proxy-retry-assertion branch July 30, 2026 20:41

Uh oh!

Conversation

parfeon commented Jul 27, 2026

Uh oh!

Xavrax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parfeon commented Jul 30, 2026

Uh oh!

parfeon commented Jul 30, 2026

Uh oh!

Uh oh!

pubnub-release-bot commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants