RDKEMW-10284: Migrate stunnel to use P12 cert #364

Lasya-Prakarsha-D-V · 2025-11-10T09:43:57Z

Reason for change: To integrate SE based device cert for mTls connectivity in stunnel
Test Procedure: Build and verify SHORTS connectivity
Risks: None
Priority: P1

Reason for change: To integrate SE based device cert for mTls connectivity in stunnel Test Procedure: Build and verify SHORTS connectivity Risks: None Priority: P1 Signed-off-by: ldonth501 <LasyaPrakarsha_DonthiVenkata@comcast.com>

Copilot

Pull Request Overview

This PR refactors the startStunnel.sh script to improve stunnel configuration management, add retry logic for PID file availability, and implement secure file descriptor handling for passcode management.

Key Changes

Moved environment variable exports (TERM and HOME) to after sourcing configuration files
Enhanced device type detection with case-insensitive checking and additional logging
Implemented dynamic file descriptor allocation with named pipe for secure passcode handling
Added retry logic for stunnel PID file reading with improved error handling
Changed certificate path variable from CERT_FILE to CERT_PATH

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

Copilot · 2025-11-10T09:51:40Z

lib/rdk/startStunnel.sh

+# Get the next available file descriptor and export if its valid
+FD_NUMBER=$(get_next_fd)
+if [ $? -eq 0 ]; then
+    export FD_NUMBER
+
+    # Create a named pipe
+    PIPE=$(mktemp -u)
+    mkfifo "$PIPE"
+
+    # Open the pipe using the available FD
+    eval "exec $FD_NUMBER<>$PIPE"
+
+    #removing the pipe after opening
+    rm "$PIPE"
+
+    #Writing passcode to open file descriptor
+    echo "$(eval "$PASSCODE")" >&$FD_NUMBER &
+else
+    echo "Error: No available file descriptor to use." >&2
 fi


Missing documentation: The purpose of creating and exporting the FD_NUMBER file descriptor with the PASSCODE is not clear. Add comments explaining:

Why this file descriptor needs to be exported

Which process/command will consume it

How it's used for authentication or other purposes

This would improve maintainability and help future developers understand this security-sensitive code.

Copilot · 2025-11-10T09:51:40Z

lib/rdk/startStunnel.sh

+    #removing the pipe after opening
+    rm "$PIPE"
+
+    #Writing passcode to open file descriptor


[nitpick] Inconsistent comment style: This comment uses lowercase and no punctuation at the end. Consider maintaining consistent comment style:

# Writing passcode to open file descriptor

Suggested change

#Writing passcode to open file descriptor

# Writing passcode to open file descriptor.

Copilot · 2025-11-10T09:51:40Z

lib/rdk/startStunnel.sh

+        rm -f $STUNNEL_PID_FILE
+        rm -f $CA_FILE
+        if [ "x$CRED_INDEX" == "x0" ]; then
+            touch /tmp/.$SE_DEVICE_CERT


The variable SE_DEVICE_CERT is used without being defined in this script. Ensure this variable is properly defined in one of the sourced files, or add a validation check before use to prevent path errors:

if [ -z "$SE_DEVICE_CERT" ]; then echo_t "STUNNEL: ERROR - SE_DEVICE_CERT is not defined" else touch /tmp/.$SE_DEVICE_CERT fi

Copilot · 2025-11-10T09:51:41Z

lib/rdk/startStunnel.sh

+    rm "$PIPE"
+
+    #Writing passcode to open file descriptor
+    echo "$(eval "$PASSCODE")" >&$FD_NUMBER &


Race condition: The file descriptor is written to in a background process (&), but there's no synchronization to ensure the data is written before it's potentially read elsewhere. If the code using FD_NUMBER reads from it immediately, it might read before the data is written. Consider removing the & to write synchronously, or implement proper synchronization if async is required.

Suggested change

echo "$(eval "$PASSCODE")" >&$FD_NUMBER &

echo "$(eval "$PASSCODE")" >&$FD_NUMBER

Copilot · 2025-11-10T09:51:41Z

lib/rdk/startStunnel.sh

+    rm "$PIPE"
+
+    #Writing passcode to open file descriptor
+    echo "$(eval "$PASSCODE")" >&$FD_NUMBER &


Potential resource leak: If get_next_fd() fails (returns 1), the script continues execution without a valid file descriptor. The opened file descriptor and pipe are never cleaned up explicitly. Consider adding cleanup logic in an exit trap or failure path to ensure resources are properly released.

Suggested change

echo "$(eval "$PASSCODE")" >&$FD_NUMBER &

echo "$(eval "$PASSCODE")" >&$FD_NUMBER &

# Trap to cleanup the file descriptor on exit

trap 'if [ -n "$FD_NUMBER" ]; then eval "exec $FD_NUMBER>&-"; fi' EXIT

Copilot · 2025-11-10T09:51:44Z

lib/rdk/startStunnel.sh

+
 isShortsenabled=`tr181 Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.SHORTS.Enable 2>&1 > /dev/null`
+echo_t "isShortsenabled = $isShortsenabled "
 if [ $isShortsenabled == "false" ];then


Unsafe comparison: The variable isShortsenabled could be empty or unset, which would cause a syntax error. Use safer comparison with quotes:

if [ "$isShortsenabled" == "false" ];then

Suggested change

if [ $isShortsenabled == "false" ];then

if [ "$isShortsenabled" == "false" ];then

Copilot · 2025-11-10T09:51:44Z

lib/rdk/startStunnel.sh

+    if [ "$DEVICETYPE" == "TEST" ] || [ "$DEVICETYPE" == "test" ]; then
        echo_t "STUNNEL: Device type is TEST"
        t2CountNotify "SHORTS_DEVICE_TYPE_TEST"
-        echo "checkHost   = $JUMP_FQDN"        >> $STUNNEL_CONF_FILE
        echo "checkHost   = $DEV_SAN"          >> $STUNNEL_CONF_FILE
    else
        echo_t "STUNNEL: Device type is PROD"
        t2CountNotify "SHORTS_DEVICE_TYPE_PROD"
-        echo "checkHost   = $JUMP_FQDN"        >> $STUNNEL_CONF_FILE
        echo "checkHost   = $PROD_SAN"         >> $STUNNEL_CONF_FILE
    fi


Overly broad PROD classification: The else branch treats any non-TEST value (including malformed or unexpected values) as PROD. Consider being more explicit:

if [ "$DEVICETYPE" == "TEST" ] || [ "$DEVICETYPE" == "test" ]; then echo_t "STUNNEL: Device type is TEST" t2CountNotify "SHORTS_DEVICE_TYPE_TEST" echo "checkHost = $DEV_SAN" >> $STUNNEL_CONF_FILE elif [ "$DEVICETYPE" == "PROD" ] || [ "$DEVICETYPE" == "prod" ]; then echo_t "STUNNEL: Device type is PROD" t2CountNotify "SHORTS_DEVICE_TYPE_PROD" echo "checkHost = $PROD_SAN" >> $STUNNEL_CONF_FILE else echo_t "STUNNEL: Device type is Unknown ($DEVICETYPE)" t2CountNotify "SHORTS_DEVICE_TYPE_UNKNOWN" fi

This prevents unexpected values from being treated as production environments.

Copilot · 2025-11-10T09:51:45Z

lib/rdk/startStunnel.sh

+    # Open the pipe using the available FD
+    eval "exec $FD_NUMBER<>$PIPE"


Missing error handling: If the eval "exec $FD_NUMBER<>$PIPE" fails, the script continues without detection. Consider adding error checking:

if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor" rm "$PIPE" exit 1 fi

Suggested change

# Open the pipe using the available FD

eval "exec $FD_NUMBER<>$PIPE"

# Open the pipe using the available FD, with error handling

if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then

echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor"

rm "$PIPE"

exit 1

fi

Copilot · 2025-11-10T09:51:45Z

lib/rdk/startStunnel.sh

+    rm "$PIPE"
+
+    #Writing passcode to open file descriptor
+    echo "$(eval "$PASSCODE")" >&$FD_NUMBER &


Security concern: Using eval on the PASSCODE variable is dangerous if the variable contains user-controlled or untrusted content. This could lead to command injection. Consider using the variable directly without eval:

echo "$PASSCODE" >&$FD_NUMBER &

If the variable needs expansion, ensure proper input sanitization is in place.

Suggested change

echo "$(eval "$PASSCODE")" >&$FD_NUMBER &

echo "$PASSCODE" >&$FD_NUMBER &

Copilot · 2025-11-10T09:51:45Z

lib/rdk/startStunnel.sh

-    echo "checkHost   = $JUMP_FQDN"          >> $STUNNEL_CONF_FILE
+fi
+
+#Function to find available fd at the point of time


Grammar issue in comment: "at the point of time" should be "at this point in time" or simply "at the time".

Suggested change

#Function to find available fd at the point of time

#Function to find available fd at this point in time

Rebase with develop

Copilot

Pull request overview

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

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

Copilot · 2025-11-28T08:01:45Z

lib/rdk/startStunnel.sh

+
 isShortsenabled=`tr181 Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.SHORTS.Enable 2>&1 > /dev/null`
+echo_t "isShortsenabled = $isShortsenabled "
 if [ $isShortsenabled == "false" ];then


The variable $isShortsenabled should be quoted in the comparison. If the tr181 command fails or returns an empty string, the test will fail with a syntax error. Change to if [ "$isShortsenabled" == "false" ]; then or preferably use the POSIX-compliant if [ "$isShortsenabled" = "false" ]; then.

Suggested change

if [ $isShortsenabled == "false" ];then

if [ "$isShortsenabled" = "false" ]; then

Copilot · 2025-11-28T08:01:45Z

lib/rdk/startStunnel.sh


 /usr/bin/stunnel $STUNNEL_CONF_FILE
+if [ $? -ne 0 ]; then
+    echo_t "STUNNEL: ERROR - Failed to start stunnel process."


The error path when stunnel fails to start should include cleanup of sensitive resources. Consider adding:

Cleanup for $CA_FILE (removed in other error paths at line 214)

Closing the opened file descriptor with eval "exec $FD_NUMBER>&-" to prevent resource leaks

Cleanup for $CERT_PATH if it's a temporary file

The SE_DEVICE_CERT marker file (lines 215-216) if applicable

Suggested change

echo_t "STUNNEL: ERROR - Failed to start stunnel process."

echo_t "STUNNEL: ERROR - Failed to start stunnel process."

# Cleanup sensitive resources

rm -f "$CA_FILE"

# Close the opened file descriptor if it was opened

if [ -n "$FD_NUMBER" ]; then

eval "exec $FD_NUMBER>&-"

fi

# Remove CERT_PATH if it is a temporary file

if [ -n "$CERT_PATH" ] && [ -f "$CERT_PATH" ] && echo "$CERT_PATH" | grep -q "/tmp/"; then

rm -f "$CERT_PATH"

fi

# Create SE_DEVICE_CERT marker file if applicable

if [ "x$CRED_INDEX" == "x0" ]; then

touch /tmp/.$SE_DEVICE_CERT

fi

Copilot · 2025-11-28T08:01:45Z

lib/rdk/startStunnel.sh

+
+DEVICETYPE=`tr181 -g Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Identity.DeviceType 2>&1`
+echo_t "STUNNEL: Device type is $DEVICETYPE"
+if [ ! -z "$DEVICETYPE" ]; then


The variable $DEVICETYPE should be quoted in the comparison to handle potential empty values safely. Change to if [ ! -z "$DEVICETYPE" ]; then to prevent potential test command errors if DEVICETYPE is empty or contains special characters.

Copilot · 2025-11-28T08:01:46Z

lib/rdk/startStunnel.sh

+DEVICETYPE=`tr181 -g Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Identity.DeviceType 2>&1`
+echo_t "STUNNEL: Device type is $DEVICETYPE"
+if [ ! -z "$DEVICETYPE" ]; then
+    if [ "$DEVICETYPE" == "TEST" ] || [ "$DEVICETYPE" == "test" ]; then


Variables in the comparison should be quoted to prevent word splitting. Change to if [ "$DEVICETYPE" == "TEST" ] || [ "$DEVICETYPE" == "test" ]; then or use a case-insensitive comparison approach to be safer.

Copilot · 2025-11-28T08:01:46Z

lib/rdk/startStunnel.sh

+	    /bin/sh /lib/rdk/startTunnel.sh start ${REVERSESSHARGS}${NONSHORTSARGS}
        exit 0 


Inconsistent indentation: this line uses a tab character followed by spaces. The previous line (82) doesn't have this leading tab. For consistency and to avoid potential issues with mixed indentation, use either spaces or tabs consistently throughout the file (preferably spaces as per common shell script conventions).

Suggested change

/bin/sh /lib/rdk/startTunnel.sh start ${REVERSESSHARGS}${NONSHORTSARGS}

exit 0

/bin/sh /lib/rdk/startTunnel.sh start ${REVERSESSHARGS}${NONSHORTSARGS}

exit 0

Rebase with develop

Copilot

Pull request overview

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

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

Copilot · 2025-12-16T13:49:30Z

lib/rdk/startStunnel.sh

+    # Open the pipe using the available FD
+    eval "exec $FD_NUMBER<>$PIPE"
+
+    #removing the pipe after opening


The comment starts with lowercase 'removing' when it should start with uppercase 'Removing' for consistency with other comments in the file.

Suggested change

#removing the pipe after opening

#Removing the pipe after opening

Copilot · 2025-12-16T13:49:30Z

lib/rdk/startStunnel.sh

+        rm -f $STUNNEL_PID_FILE
+        rm -f $CA_FILE
+        if [ "x$CRED_INDEX" == "x0" ]; then
+            touch /tmp/.$SE_DEVICE_CERT
+        fi
+        echo_t "STUNNEL: stunnel-client failed to establish. Exiting..."
+        t2CountNotify "SHORTS_STUNNEL_CLIENT_FAILURE"
+        exit


Missing cleanup of CERT_PATH file on error. The old code removed CERT_FILE on failure, but with the change from CERT_FILE to CERT_PATH, the cleanup logic was not updated. Consider adding 'rm -f $CERT_PATH' to ensure sensitive certificate data is cleaned up on failure.

Copilot · 2025-12-16T13:49:30Z

lib/rdk/startStunnel.sh

+FD_NUMBER=$(get_next_fd)
+if [ $? -eq 0 ]; then
+    export FD_NUMBER
+
+    # Create a named pipe
+    PIPE=$(mktemp -u)
+    mkfifo "$PIPE"
+
+    # Open the pipe using the available FD
+    eval "exec $FD_NUMBER<>$PIPE"
+
+    #removing the pipe after opening
+    rm "$PIPE"
+
+    #Writing passcode to open file descriptor
+    echo "$(eval "$PASSCODE")" >&$FD_NUMBER &
+else
+    echo "Error: No available file descriptor to use." >&2
 fi


The file descriptor and named pipe are created but never closed or cleaned up. The FD_NUMBER remains open for the lifetime of the script and the pipe may leak resources. Consider adding cleanup logic to close the file descriptor when it's no longer needed, or add a trap to ensure cleanup on script exit.

Copilot · 2025-12-16T13:49:30Z

lib/rdk/startStunnel.sh

+    if [ $count -lt 2 ]; then
+        sleep 1
+        echo_t "STUNNEL: stunnel PID file is not available, Retrying..."
+        STUNNELPID=`cat $STUNNEL_PID_FILE`


Using backticks for command substitution is inconsistent with the rest of the code and can cause issues with nested quotes. The rest of the file uses the backtick style, but consider using $() for better readability and nesting support. Also, this read from a potentially non-existent file could fail silently - consider adding 2>/dev/null to suppress errors.

Suggested change

STUNNELPID=`cat $STUNNEL_PID_FILE`

STUNNELPID=$(cat $STUNNEL_PID_FILE 2>/dev/null)

Copilot · 2025-12-16T13:49:31Z

lib/rdk/startStunnel.sh

+
+# Get the next available file descriptor and export if its valid
+FD_NUMBER=$(get_next_fd)
+if [ $? -eq 0 ]; then


Consider using the more idiomatic pattern 'if get_next_fd; then' or storing the result in a variable and checking if it's non-empty, rather than checking $?. This makes the code more readable and follows common shell scripting best practices.

Suggested change

if [ $? -eq 0 ]; then

if [ -n "$FD_NUMBER" ]; then

Copilot · 2025-12-16T13:49:31Z

lib/rdk/startStunnel.sh

 echo "connect = $JUMP_SERVER:$JUMP_PORT" >> $STUNNEL_CONF_FILE

 extract_stunnel_client_cert



The variable CERT_PATH is used but is never defined in this script. The code previously used CERT_FILE which was likely defined elsewhere, but the rename to CERT_PATH may have introduced a bug if the source variable wasn't also renamed. This will cause the certificate file check to fail and the script to exit with an error.

Suggested change

# Ensure CERT_PATH is set, fallback to CERT_FILE if available

if [ -z "$CERT_PATH" ] && [ -n "$CERT_FILE" ]; then

CERT_PATH="$CERT_FILE"

fi

RDKEMW-10284: Migrate stunnel to use P12 cert

49caafc

Reason for change: To integrate SE based device cert for mTls connectivity in stunnel Test Procedure: Build and verify SHORTS connectivity Risks: None Priority: P1 Signed-off-by: ldonth501 <LasyaPrakarsha_DonthiVenkata@comcast.com>

Lasya-Prakarsha-D-V requested a review from a team as a code owner November 10, 2025 09:43

Copilot AI review requested due to automatic review settings November 10, 2025 09:43

Copilot AI reviewed Nov 10, 2025

View reviewed changes

Lasya-Prakarsha-D-V added 2 commits November 12, 2025 13:15

Merge pull request #369 from rdkcentral/develop

106f599

Rebase with develop

Merge pull request #379 from rdkcentral/develop

7f6ed9e

Rebase with develop

Copilot AI review requested due to automatic review settings November 28, 2025 07:53

Copilot started reviewing on behalf of Lasya-Prakarsha-D-V November 28, 2025 07:54 View session

Copilot finished reviewing on behalf of Lasya-Prakarsha-D-V November 28, 2025 08:00

Copilot AI reviewed Nov 28, 2025

View reviewed changes

Lasya-Prakarsha-D-V added 2 commits December 12, 2025 18:30

Merge pull request #391 from rdkcentral/develop

1e7eadb

Rebase with develop

Merge pull request #398 from rdkcentral/develop

8ddce61

Rebase with develop

Copilot AI review requested due to automatic review settings December 16, 2025 13:24

Copilot started reviewing on behalf of Lasya-Prakarsha-D-V December 16, 2025 13:24 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

	#Writing passcode to open file descriptor
	# Writing passcode to open file descriptor.

	echo "$(eval "$PASSCODE")" >&$FD_NUMBER &
	echo "$(eval "$PASSCODE")" >&$FD_NUMBER

	if [ $isShortsenabled == "false" ];then
	if [ "$isShortsenabled" == "false" ];then

		# Open the pipe using the available FD
		eval "exec $FD_NUMBER<>$PIPE"

	echo "$(eval "$PASSCODE")" >&$FD_NUMBER &
	echo "$PASSCODE" >&$FD_NUMBER &

	#Function to find available fd at the point of time
	#Function to find available fd at this point in time

	if [ $isShortsenabled == "false" ];then
	if [ "$isShortsenabled" = "false" ]; then

-    echo_t "STUNNEL: ERROR - Failed to start stunnel process."
+    echo_t "STUNNEL: ERROR - Failed to start stunnel process."
+    # Cleanup sensitive resources
+    rm -f "$CA_FILE"
+    # Close the opened file descriptor if it was opened
+    if [ -n "$FD_NUMBER" ]; then
+        eval "exec $FD_NUMBER>&-"
+    fi
+    # Remove CERT_PATH if it is a temporary file
+    if [ -n "$CERT_PATH" ] && [ -f "$CERT_PATH" ] && echo "$CERT_PATH" | grep -q "/tmp/"; then
+        rm -f "$CERT_PATH"
+    fi
+    # Create SE_DEVICE_CERT marker file if applicable
+    if [ "x$CRED_INDEX" == "x0" ]; then
+        touch /tmp/.$SE_DEVICE_CERT
+    fi

		/bin/sh /lib/rdk/startTunnel.sh start ${REVERSESSHARGS}${NONSHORTSARGS}
		exit 0

	#removing the pipe after opening
	#Removing the pipe after opening

	STUNNELPID=`cat $STUNNEL_PID_FILE`
	STUNNELPID=$(cat $STUNNEL_PID_FILE 2>/dev/null)

		echo "connect = $JUMP_SERVER:$JUMP_PORT" >> $STUNNEL_CONF_FILE

		extract_stunnel_client_cert

+# Ensure CERT_PATH is set, fallback to CERT_FILE if available
+if [ -z "$CERT_PATH" ] && [ -n "$CERT_FILE" ]; then
+    CERT_PATH="$CERT_FILE"
+fi

RDKEMW-10284: Migrate stunnel to use P12 cert #364

Are you sure you want to change the base?

RDKEMW-10284: Migrate stunnel to use P12 cert #364

Uh oh!

Conversation

Lasya-Prakarsha-D-V commented Nov 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

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 Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

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 Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!