-
Notifications
You must be signed in to change notification settings - Fork 5
RDKEMW-10284: Migrate stunnel to use P12 cert #364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
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>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (
TERMandHOME) 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_FILEtoCERT_PATH
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rebase with develop
Rebase with develop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Rebase with develop
Rebase with develop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Rebase with develop
Rebase with develop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Rebase with develop
Rebase with develop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lib/rdk/startStunnel.sh:199
- The error handling around stunnel startup exits before running the "cleanup sensitive files" section, meaning
$STUNNEL_CONF_FILEand any sensitive data referenced by$D_FILEare left on disk whenever/usr/bin/stunnelreturns a non‑zero exit code. To preserve the previous behavior and avoid leaking sensitive material, perform the cleanup (or at least remove these files) before exiting on stunnel startup failure.
/usr/bin/stunnel $STUNNEL_CONF_FILE
if [ $? -ne 0 ]; then
echo_t "STUNNEL: ERROR - Failed to start stunnel process."
exit 1
fi
# cleanup sensitive files early
rm -f $STUNNEL_CONF_FILE
rm -f $D_FILE
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lib/rdk/startStunnel.sh:199
- When stunnel exits with a non-zero status, the
exit 1path prevents the subsequent cleanup block from running, so$STUNNEL_CONF_FILEand$D_FILEare left on disk even though they are marked as sensitive. To avoid persisting sensitive configuration or key material on stunnel startup failures, the cleanup of these files should also execute on the error path (for example by moving or duplicating the cleanup before exiting on failure).
/usr/bin/stunnel $STUNNEL_CONF_FILE
if [ $? -ne 0 ]; then
echo_t "STUNNEL: ERROR - Failed to start stunnel process."
exit 1
fi
# cleanup sensitive files early
rm -f $STUNNEL_CONF_FILE
rm -f $D_FILE
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change: Applying valid co pilot review comments Test Procedure: Build and verify SHORTS connectivity Risks: None Priority: P1 Signed-off-by: ldonth501 <LasyaPrakarsha_DonthiVenkata@comcast.com>
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