RDKB-62352 : Observed telemetry2_0 crash #247

adrojaankur · 2026-01-27T08:59:44Z

No description provided.

Rebase

Copilot

Pull request overview

This PR updates the HTTP telemetry upload path to better integrate with the RdkCertSelector-based mTLS/PKI stack and to address an observed telemetry2_0 crash when using certificate selection.

Changes:

Adds OpenSSL crypto initialization in the forked child process when LIBRDKCERTSEL_BUILD is enabled.
Configures libcurl to use an SSL engine obtained from rdkcertselector (or the default engine) before performing the HTTP request.
Updates the HTTP protocol library build to link against libcrypto when rdkcertselector support is enabled.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`source/protocol/http/curlinterface.c`	Adds OpenSSL initialization and SSL engine configuration to the `sendReportOverHTTP` child path, and introduces an engine name pointer used with libcurl when `LIBRDKCERTSEL_BUILD` is enabled.
`source/protocol/http/Makefile.am`	Extends linker flags so `libhttp.la` links against `libcrypto` alongside `librdkcertselector` when cert selector support is enabled, satisfying the new OpenSSL usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-27T09:08:43Z

source/protocol/http/curlinterface.c

+	    pEngine = rdkcertselector_getEngine(curlCertSelector);
+	    if(pEngine != NULL ) {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE, pEngine);
+		    if(code != CURLE_OK) {
+			    childCurlResponse.lineNumber = __LINE__;
+			    curl_easy_cleanup(curl);
+			    goto child_cleanReturn;
+		    }
+	    } else {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1L);
+		    if(code != CURLE_OK ) {
+			    childCurlResponse.lineNumber = __LINE__;
+			    curl_easy_cleanup(curl);
+			    goto child_cleanReturn;
+		    }
+	    }


When LIBRDKCERTSEL_BUILD and ENABLE_RED_RECOVERY_SUPPORT are enabled and state_red_enable is true, curlCertSelectorInit() only initializes curlRcvryCertSelector and leaves curlCertSelector as NULL, but here rdkcertselector_getEngine is always called with curlCertSelector. This can lead to a NULL dereference and also means the SSL engine may not match the selector (thisCertSel) that is later used for rdkcertselector_getCert. To keep the engine and certificate selection consistent and avoid crashes, this should use thisCertSel (with appropriate NULL checks) instead of hard-coding curlCertSelector.

Copilot · 2026-01-27T09:08:44Z

source/protocol/http/curlinterface.c

    if(childPid == 0)
    {
+#ifdef LIBRDKCERTSEL_BUILD 
+	OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL);


OPENSSL_init_crypto returns a status code indicating whether initialization succeeded, but the return value is currently ignored. For robustness, especially given that subsequent OpenSSL-dependent operations (like engine usage) are required for mTLS, consider checking the return value and at least logging and bailing out early on failure so that errors in crypto initialization are visible and handled deterministically.

Suggested change

OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL);

if (OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL) != 1)

{

T2Error("OPENSSL_init_crypto failed, terminating child process\n");

exit(EXIT_FAILURE);

}

Copilot · 2026-01-27T09:08:44Z

source/protocol/http/curlinterface.c

+	    pEngine = rdkcertselector_getEngine(curlCertSelector);
+	    if(pEngine != NULL ) {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE, pEngine);
+		    if(code != CURLE_OK) {
+			    childCurlResponse.lineNumber = __LINE__;
+			    curl_easy_cleanup(curl);
+			    goto child_cleanReturn;
+		    }
+	    } else {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1L);
+		    if(code != CURLE_OK ) {
+			    childCurlResponse.lineNumber = __LINE__;
+			    curl_easy_cleanup(curl);
+			    goto child_cleanReturn;
+		    }
+	    }


There are existing unit tests for sendReportOverHTTP in source/test/protocol/ProtocolTest.cpp, but the new LIBRDKCERTSEL_BUILD-specific behavior here (OpenSSL initialization and SSL engine selection / failure paths) does not appear to be exercised. To prevent regressions in the crash scenario this PR is addressing, it would be helpful to extend the tests to cover the engine selection path (both custom engine and default engine, including error returns from curl_easy_setopt).

pdmooneypub

see my comments.

source/protocol/http/curlinterface.c

pdmooneypub · 2026-01-27T18:52:47Z

source/protocol/http/curlinterface.c

                goto child_cleanReturn;
            }
 #ifdef LIBRDKCERTSEL_BUILD
+	    pEngine = rdkcertselector_getEngine(curlCertSelector);


again, from copilot, curlCertSelector is not set when in redrecovery so this could be null.

pdmooneypub · 2026-01-27T18:54:18Z

source/protocol/http/curlinterface.c

+			    goto child_cleanReturn;
+		    }
+	    }
+


copilot noticed that there were no tests added for the new code paths. can we add unit tests?

Since the OpenSSL engine logic is not part of the core telemetry functionality, the telemetry unit test NOT handles the curl request. Based on my understanding, this scenario should be covered under L2/L3 testing rather than as a unit or core telemetry test

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T06:51:49Z

source/protocol/http/curlinterface.c

+	    pEngine = rdkcertselector_getEngine(curlCertSelector);
+	    if(pEngine != NULL ) {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE, pEngine);
+		    if(code != CURLE_OK) {
+			    childCurlResponse.lineNumber = __LINE__;


rdkcertselector_getEngine is always called with curlCertSelector, even when thisCertSel has been set to curlRcvryCertSelector for state-red mode and curlCertSelector may have been freed (set to NULL) in curlCertSelectorInit. This can lead to passing a NULL or stale handle into rdkcertselector_getEngine, which is likely the cause of the observed crash and is also inconsistent with the selector actually used later (thisCertSel for rdkcertselector_getCert). Consider calling rdkcertselector_getEngine(thisCertSel) instead (with an appropriate NULL check) so that the engine configuration always matches the active cert selector and avoids dereferencing a freed/NULL selector handle.

Copilot · 2026-01-28T06:51:50Z

source/protocol/http/curlinterface.c

+		if(curlCertSelector != NULL){
+			rdkcertselector_free(&curlCertSelector);
+			curlCertSelector = NULL;


The new block freeing curlCertSelector inside curlCertSelectorInit uses tab indentation, whereas the rest of this function and file use space-based indentation. For consistency with the surrounding code and established style in this file (e.g., the curlRcvryCertSelector initialization and logging below), please switch this block to use the same space-based indentation.

Copilot · 2026-01-28T06:51:50Z

source/protocol/http/curlinterface.c

+#ifdef LIBRDKCERTSEL_BUILD 
+	if(OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL) != 1){
+		T2Info("%s, T2:OPENSSL_init_crypto failed.\n", __func__);
+	}
+#endif


The newly added #ifdef LIBRDKCERTSEL_BUILD block before curl_easy_init() is indented with tabs rather than spaces, which is inconsistent with the surrounding code in this function (e.g., the curl = curl_easy_init(); line and subsequent blocks). To follow the file's established indentation style, please re-indent this preprocessor block to use spaces like the rest of the function.

Rebase

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-03T04:00:22Z

source/protocol/http/curlinterface.c

+	    pEngine = rdkcertselector_getEngine(thisCertSel);
+	    if(pEngine != NULL ) {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE, pEngine);
+		    if(code != CURLE_OK) {
+			    childCurlResponse.lineNumber = __LINE__;
+			    curl_easy_cleanup(curl);
+			    goto child_cleanReturn;
+		    }
+	    } else {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1L);
+		    if(code != CURLE_OK ) {
+			    childCurlResponse.lineNumber = __LINE__;
+			    curl_easy_cleanup(curl);
+			    goto child_cleanReturn;
+		    }
+	    }


The new SSL engine selection logic under LIBRDKCERTSEL_BUILD (calling rdkcertselector_getEngine and configuring CURLOPT_SSLENGINE / CURLOPT_SSLENGINE_DEFAULT) does not appear to be covered by existing unit tests, even though this file already has GTest-based coverage for its static helpers (e.g. SetHeader in source/test/protocol/ProtocolTest.cpp). Since this path is specific and error-prone (different behavior depending on whether rdkcertselector_getEngine returns a non-NULL engine, plus the two curl_easy_setopt branches), please consider adding targeted tests that exercise both the engine and default-engine paths and validate the behavior when curl_easy_setopt succeeds or fails.

Copilot · 2026-02-03T04:00:23Z

source/protocol/http/curlinterface.c

+	    pEngine = rdkcertselector_getEngine(thisCertSel);
+	    if(pEngine != NULL ) {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE, pEngine);
+		    if(code != CURLE_OK) {
+			    childCurlResponse.lineNumber = __LINE__;
+			    curl_easy_cleanup(curl);
+			    goto child_cleanReturn;
+		    }
+	    } else {
+		    code = curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1L);
+		    if(code != CURLE_OK ) {
+			    childCurlResponse.lineNumber = __LINE__;


In this new SSL engine handling block, on CURLOPT_SSLENGINE / CURLOPT_SSLENGINE_DEFAULT failure you only set childCurlResponse.lineNumber before cleaning up and jumping to child_cleanReturn, but you never populate childCurlResponse.curlSetopCode with the failing code. In the parent, the log at lines 633–635 prints curl_easy_strerror(childCurlResponse.curlSetopCode), so these failures will misleadingly appear as CURLE_OK, making debugging much harder. Please set childCurlResponse.curlSetopCode = code in both error branches before the goto child_cleanReturn so the actual failure reason is propagated.

Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-03T12:18:58Z

source/protocol/http/curlinterface.c

+            pEngine = rdkcertselector_getEngine(thisCertSel);
+            if(pEngine != NULL )
+            {
+                code = curl_easy_setopt(curl, CURLOPT_SSLENGINE, pEngine);
+                if(code != CURLE_OK)
+                {
+                    childCurlResponse.lineNumber = __LINE__;
+                    curl_easy_cleanup(curl);
+                    goto child_cleanReturn;
+                }
+            }
+            else
+            {
+                code = curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1L);
+                if(code != CURLE_OK )
+                {


When CURLOPT_SSLENGINE / CURLOPT_SSLENGINE_DEFAULT fail, childCurlResponse.curlSetopCode is not updated before you set lineNumber and jump to child_cleanReturn. This means the parent-side logging will report the last successful setopt result instead of the actual failing CURLcode, making diagnosis harder; consider assigning childCurlResponse.curlSetopCode = code here, consistent with other curl_easy_setopt error paths in this file.

Copilot · 2026-02-03T12:18:58Z

source/protocol/http/curlinterface.c

+#ifdef LIBRDKCERTSEL_BUILD
+        if(OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL) != 1)
+        {
+             fprintf(stderr, "T2:OPENSSL_init_crypto failed\n"); // avoiding T2 logger in the child. 


In the child process, if OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL) returns a failure status, the code only prints to stderr but continues to use OpenSSL/cURL as if initialization had succeeded. If OPENSSL_init_crypto ever fails, subsequent crypto/TLS operations are undefined; consider treating this as a hard failure (e.g., set an error in childCurlResponse and jump to child_cleanReturn) so callers get a clear error signal instead of potentially crashing or misbehaving.

Suggested change

fprintf(stderr, "T2:OPENSSL_init_crypto failed\n"); // avoiding T2 logger in the child.

fprintf(stderr, "T2:OPENSSL_init_crypto failed\n"); // avoiding T2 logger in the child.

childCurlResponse.curlStatus = false;

childCurlResponse.curlResponse = CURLE_FAILED_INIT;

childCurlResponse.lineNumber = __LINE__;

goto child_cleanReturn;

Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>

shibu-kv · 2026-02-03T16:58:55Z

source/protocol/http/curlinterface.c

 #include <curl/curl.h>
 #include <signal.h>
+#ifdef LIBRDKCERTSEL_BUILD
+#include <openssl/crypto.h>


We shouldn't be using openssl api's directly. The libcurl apis should be the only interface.

Shibu, at one point we were hashing the passcode so we could see if it changed (without exposing it). We may want to keep this in here until we have resolved the problem with the passcode.

shibu-kv · 2026-02-03T17:00:13Z

source/protocol/http/curlinterface.c

    if(childPid == 0)
    {
+#ifdef LIBRDKCERTSEL_BUILD
+        if(OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL) != 1)


This doesn't seem to be the right thing to do.

aadroj583 and others added 2 commits December 23, 2025 06:20

Telemetry crashe due to Open SSL engine

3a0a7a2

Merge pull request #246 from rdkcentral/develop

13c9f3d

Rebase

adrojaankur requested a review from a team as a code owner January 27, 2026 08:59

Copilot AI review requested due to automatic review settings January 27, 2026 08:59

Copilot started reviewing on behalf of adrojaankur January 27, 2026 09:00 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

pdmooneypub reviewed Jan 27, 2026

View reviewed changes

adrojaankur added 2 commits January 28, 2026 12:12

Update curlinterface.c

fa28348

Update curlinterface.c

a5e4202

Copilot AI review requested due to automatic review settings January 28, 2026 06:49

Copilot started reviewing on behalf of adrojaankur January 28, 2026 06:49 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

adrojaankur added 2 commits January 28, 2026 12:30

Update curlinterface.c

6657cfe

Merge pull request #252 from rdkcentral/develop

26176b5

Rebase

Copilot AI review requested due to automatic review settings February 3, 2026 03:55

Copilot started reviewing on behalf of adrojaankur February 3, 2026 03:56 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

yogeswaransky and others added 2 commits February 3, 2026 06:22

RDKB-62352 : Observed telemetry2_0 crash

9d1e4ed

Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>

Update curlinterface.c

6573670

Copilot AI review requested due to automatic review settings February 3, 2026 12:14

Copilot started reviewing on behalf of adrojaankur February 3, 2026 12:14 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

RDKB-62352 : Observed telemetry2_0 crash

d9e649e

Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>

shibu-kv requested changes Feb 3, 2026

View reviewed changes

-	OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL);
+        if (OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL) != 1)
+        {
+            T2Error("OPENSSL_init_crypto failed, terminating child process\n");
+            exit(EXIT_FAILURE);
+        }

RDKB-62352 : Observed telemetry2_0 crash #247

Are you sure you want to change the base?

RDKB-62352 : Observed telemetry2_0 crash #247

Conversation

adrojaankur commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

pdmooneypub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pdmooneypub Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

adrojaankur Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

pdmooneypub Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

adrojaankur Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

shibu-kv Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

pdmooneypub Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

shibu-kv Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees