RDK-61551: Revssh Hardening - Entertainement devices

[Lasya-Prakarsha-D-V]

Reason for change: Enforcing SHORTS connection for PROD builds
Test procedure: NON-shorts ssh attempt for prod build should fail
Risks: Medium
Priority: P1

[Copilot]

🟡 Changes recommended

Unresolved failure-classification issues can misreport errors and mix concurrent session logs.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR enforces SHORTS for PROD reverse-SSH connections and adds failure telemetry/classification.

Changes:

• Blocks non-SHORTS attempts on PROD builds.
• Adds certificate, launch, connection, and failure notifications.
• Classifies stunnel failures from logs.

File summaries

File	Review summary
lib/rdk/startStunnel.sh	Critical: fallback failures at lines 65/106 can misclassify unknown TLS, network, or process errors as SSH authentication (2 votes). Moderate: support both self-signed and self signed wording (1 vote). Moderate: shared-log line cutoff can mix concurrent sessions (1 vote).

Review details

Suppressed comments (3)

lib/rdk/startStunnel.sh:61

• The standard OpenSSL/stunnel wording is self-signed certificate (with a hyphen), so this alternative does not match that common verification failure. It will therefore fall through to SHORTS_SSH_AUTH_FAILURE and misclassify a TLS verification error; match both spellings (for example, with self[- ]signed).

    elif echo "$session_log" | grep -Eqi "certificate verify failed|verification error|certificate has expired|self signed certificate"; then

lib/rdk/startStunnel.sh:109

• This branch only records telemetry; it does not stop the non-SHORTS invocation. Control falls through to line 275, which starts SSH with SHORTSARGS (or with no SHORTS-specific arguments), so a PROD call carrying NONSHORTSARGS can still establish a session instead of failing as required by the test procedure. Return a failure here, before creating the stunnel, when a plain trigger is detected.

    if [ -n "$NONSHORTSARGS" ]; then
        echo_t "STUNNEL: plain reverse SSH trigger attempted on PROD build; SHORTS is mandatory."
        t2CountNotify "SHORTS_MANDATORY_NON_SHORTS_BLOCKED"
    fi

lib/rdk/startStunnel.sh:236

• A line-count cutoff cannot isolate this invocation because every stunnel instance writes to the shared $LOG_FILE. If another session starts after this marker, its SAN/TLS lines are included and can cause this session's failure to be reported under the wrong root cause. Use per-session stunnel output or add a session identifier that is filtered during classification.

LOG_LINE_MARK=$(wc -l < "$LOG_FILE" 2>/dev/null)
LOG_LINE_MARK=${LOG_LINE_MARK:-0}

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The PROD non-SHORTS path can still establish a session, and unmatched failures are misclassified in telemetry.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

lib/rdk/startStunnel.sh:66

• When none of the stunnel patterns matches—including an empty/stale log or a failure before ssh starts—the fallback is reported as SHORTS_SSH_AUTH_FAILURE even though there is no authentication evidence. This makes the new root-cause telemetry inaccurate; emit the auth bucket only for an explicit authentication error and route unmatched failures to a generic/unknown bucket.

    else
        t2CountNotify "SHORTS_SSH_AUTH_FAILURE"

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Preserve the SHORTS_DEVICE_TYPE_UNKNOWN telemetry notification before approval.

Review details

Suppressed comments (1)

lib/rdk/startStunnel.sh:145

• Removing SHORTS_DEVICE_TYPE_UNKNOWN means an empty device-type response is no longer visible in telemetry, even though this fallback is a security-sensitive condition. Preserve the existing count notification while applying the PROD SAN policy.

    echo_t "STUNNEL: Device type is unknown; applying PROD SAN policy."
    echo "checkHost   = $PROD_SAN"             >> $STUNNEL_CONF_FILE

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Retain unknown-device telemetry while applying the secure production SAN fallback.

Review details

Suppressed comments (1)

lib/rdk/startStunnel.sh:145

• The unknown-device fallback still represents a distinct failure mode, but replacing SHORTS_DEVICE_TYPE_UNKNOWN with only the PROD SAN write silently removes the existing telemetry signal. This makes devices with an unset/unreadable DeviceType invisible to the monitoring that distinguished TEST/PROD/UNKNOWN; retain the notification while applying the secure PROD SAN fallback.

    echo "checkHost   = $PROD_SAN"             >> $STUNNEL_CONF_FILE

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Preserve the existing unknown-device and count-based telemetry signals.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

lib/rdk/startStunnel.sh:117

• Changing SHORTS_STUNNEL_CERT_FAILURE from t2CountNotify to t2ValNotify changes the established telemetry signal from a count event to a value event, so existing count-based profiles and dashboards may stop recording this failure. Retain the count notification and use a separately defined value event for the detail if that detail is required.

    t2ValNotify "SHORTS_STUNNEL_CERT_FAILURE" "Required cert/CA file not found"

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

No unresolved review issues were identified.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Address the two moderate telemetry issues before approval.

Review details

Suppressed comments (2)

lib/rdk/startStunnel.sh:145

• The unknown-device branch used to emit SHORTS_DEVICE_TYPE_UNKNOWN; replacing it removes that signal for the cases where the identity lookup is unavailable, so operators can no longer distinguish unknown device types from normal production devices. Preserve the notification while adding the production-SAN fallback.

    echo "checkHost   = $PROD_SAN"             >> $STUNNEL_CONF_FILE

lib/rdk/startStunnel.sh:117

• SHORTS_STUNNEL_CERT_FAILURE was emitted with t2CountNotify before this change, but this switches the existing event to a value notification. That changes the T2 event type and can break existing count-based aggregation or consumers; keep the count event and use the new per-file value events for the additional detail.

    t2CountNotify "SHORTS_STUNNEL_CERT_FAILURE"

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unknown device-type failures currently lose important telemetry needed to distinguish them from normal production devices.

Review details

Suppressed comments (1)

lib/rdk/startStunnel.sh:145

• This replacement drops the existing SHORTS_DEVICE_TYPE_UNKNOWN telemetry whenever the DeviceType query is empty or fails. Because this branch is now also the security-sensitive fallback that selects the production SAN, losing the event makes identity-query failures indistinguishable from normal PROD devices and removes an important operational signal; keep the unknown-device notification and append the fallback checkHost configuration instead of replacing it.

    echo "checkHost   = $PROD_SAN"             >> $STUNNEL_CONF_FILE

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Address the two unresolved moderate findings before approval.

Review details

Suppressed comments (2)

lib/rdk/startStunnel.sh:145

• This replaces the existing SHORTS_DEVICE_TYPE_UNKNOWN count with the PROD SAN fallback. When the device-type lookup is empty, the fallback still runs but the unknown-device telemetry is lost, so monitoring can no longer distinguish this case. Keep the count notification in this branch and add the PROD checkHost line instead of replacing it.

    echo "checkHost   = $PROD_SAN"             >> $STUNNEL_CONF_FILE

lib/rdk/startStunnel.sh:96

• On a PROD invocation this branch only skips ${NONSHORTSARGS} and then falls through to line 244, where ${SHORTSARGS} is appended. The script's own usage and # [ $# -lt 5 ] check allow the sixth argument to be absent, so an empty SHORTSARGS leaves only ${REVERSESSHARGS} and can still launch SSH without the SHORTS-specific arguments. Reject PROD requests without a non-empty SHORTSARGS (and make that argument part of the enforced interface) before continuing.

    if [ "$isShortsenabled" = "false" ]; then
        echo_t "STUNNEL: SHORTS RFC is false on PROD build; enforcing SHORTS."
        t2CountNotify "SHORTS_MANDATORY_NON_SHORTS_BLOCKED"
    fi

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

No unresolved issues were identified.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Copilot review overview

🔵 Needs a closer look

Correct the telemetry API usage before approval.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified, and the change is fully reviewed.

Review effort: Lite
Findings: None