Skip to content

Use TLS for certificate-free gNMI services - #28915

Open
hdwhdw wants to merge 25 commits into
sonic-net:masterfrom
hdwhdw:fix/28540-insecure-fallback
Open

Use TLS for certificate-free gNMI services#28915
hdwhdw wants to merge 25 commits into
sonic-net:masterfrom
hdwhdw:fix/28540-insecure-fallback

Conversation

@hdwhdw

@hdwhdw hdwhdw commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Why

Certificate-free gNMI currently falls back to plaintext loopback. SmartSwitch DPUs need encrypted midplane reachability without certificate provisioning.

Fixes #28540. Replaces #28564.

Microsoft ADO (number only): 39179389

What

Validation

  • Launcher syntax and null/partial-cert/CA/auth-mode truth-table checks passed.
  • All nine platform build jobs and six of nine impacted-area KVM suites passed in Azure run 1187671.
  • Source tests passed: sonic-gnmi interceptor/proxy tests, 37 focused Host Services tests, and sonic-utilities Azure run 1188156 plus focused tests.
  • Physical image SONiC.master-28915.1187671-f1a440da3 passed five direct TLS System.Time calls, metadata-routed DPUProxy System.Time, and the exercised DPU0 utility halt/reset path.
  • Exact baseline images were restored, all four DPUs were Online, temporary artifacts were removed, and the testbed was released READY.

Known CI Failure

  • The t0, t1-lag, and t2 KVM suites fail in cacl/test_cacl_application.py because its expected rules omit four FRR loopback OUTPUT rules added by earlier sonic-host-services PR 398.
  • None of PRs 749, 423, 4761, or 28915 changes CACL or those rules.
  • Retrying all three failed jobs reproduced the same failure; maintainer disposition is required.

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
@hdwhdw
hdwhdw requested a review from lguohan as a code owner August 7, 2026 22:45
Copilot AI lite review requested due to automatic review settings August 7, 2026 22:45
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the gNMI/telemetry container launch scripts in sonic-buildimage so that when no valid TLS certificate/key is configured, the services fall back to --insecure (TLS with an insecure/self-signed mode) instead of starting in plaintext (--noTLS) bound to loopback only.

Changes:

  • Replace the no-certificate fallback from --noTLS --bind_address 127.0.0.1 to --insecure in both launcher variants.
  • Keep existing behavior for configured cert/mTLS paths (CERTS/X509), while making the certificate-free path encrypted.

Reviewed changes

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

File Description
dockers/docker-sonic-telemetry/telemetry.sh Switches the “no CERTS/X509 configured” fallback from plaintext loopback to --insecure.
dockers/docker-sonic-gnmi/gnmi-native.sh Makes the same fallback change in the alternate launcher variant.

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

Comment on lines 79 to 81
else
TELEMETRY_ARGS+=" --noTLS --bind_address 127.0.0.1"
TELEMETRY_ARGS+=" --insecure"
fi

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current PR diff includes all three gitlinks: sonic-gnmi 835b6b4, sonic-host-services 798f9d1, and sonic-utilities 56d9bc76. The PR body now lists those integrated source changes.

Copilot AI review requested due to automatic review settings August 7, 2026 22:49
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated no new comments.

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
Copilot AI review requested due to automatic review settings August 8, 2026 01:41
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated no new comments.

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
Copilot AI review requested due to automatic review settings August 8, 2026 05:58
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

dockers/docker-sonic-telemetry/telemetry.sh:113

  • CLIENT_AUTH is extracted with the jq filter .client_auth (no // empty). If the GNMI JSON object exists but omits client_auth, jq -r will return the literal string null, which is not caught by the -z "$CLIENT_AUTH" branch in the new --allow_no_client_auth gating. This can accidentally require client cert auth even when no client CA is configured.

Normalize missing/null to empty so the condition behaves as intended.

CLIENT_AUTH=$(extract_field "$GNMI" '.client_auth')
if [ "$CERTIFICATE_FREE_TLS" == "true" ] || [ "$CLIENT_AUTH" == "false" ] || { [ -z "$CLIENT_AUTH" ] && [ "$HAS_CLIENT_CA" == "false" ]; }; then
    TELEMETRY_ARGS+=" --allow_no_client_auth"

dockers/docker-sonic-gnmi/gnmi-native.sh:95

  • CLIENT_AUTH is extracted with .client_auth (no // empty). If the GNMI JSON object exists but doesn’t include client_auth, jq -r returns the string null, which won’t satisfy -z "$CLIENT_AUTH" in the new --allow_no_client_auth logic. That can unintentionally enforce client cert auth even when no client CA is configured.

Use // empty so missing/null is treated as unset.

CLIENT_AUTH=$(extract_field "$GNMI" '.client_auth')
if [ "$CERTIFICATE_FREE_TLS" == "true" ] || [ "$CLIENT_AUTH" == "false" ] || { [ -z "$CLIENT_AUTH" ] && [ "$HAS_CLIENT_CA" == "false" ]; }; then
    TELEMETRY_ARGS+=" --allow_no_client_auth"
fi

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
Copilot AI review requested due to automatic review settings August 8, 2026 11:03
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated no new comments.

@hdwhdw

hdwhdw commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@hdwhdw

hdwhdw commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@hdwhdw

hdwhdw commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@hdwhdw

hdwhdw commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

…e-fallback

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
Copilot AI review requested due to automatic review settings August 8, 2026 11:15
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

dockers/docker-sonic-telemetry/telemetry.sh:28

  • ESCAPE_QUOTE is defined but never used in this script. Removing it avoids confusion about whether additional quoting/escaping is expected elsewhere.
TELEMETRY_VARS_FILE=/usr/share/sonic/templates/telemetry_vars.j2
ESCAPE_QUOTE="'\''"

dockers/docker-sonic-gnmi/gnmi-native.sh:7

  • ESCAPE_QUOTE is defined but never used in this script. Removing it avoids confusion about whether additional quoting/escaping is expected elsewhere.
TELEMETRY_VARS_FILE=/usr/share/sonic/templates/telemetry_vars.j2
ESCAPE_QUOTE="'\''"

Comment on lines +111 to 113
CLIENT_AUTH=$(extract_field "$GNMI" 'if .client_auth == null then empty else .client_auth end')
if [ "$CERTIFICATE_FREE_TLS" == "true" ] || [ "$CLIENT_AUTH" == "false" ] || { [ -z "$CLIENT_AUTH" ] && [ "$HAS_CLIENT_CA" == "false" ]; }; then
TELEMETRY_ARGS+=" --allow_no_client_auth"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intent clarified in the PR body: CA-backed paths keep the existing mTLS default, while explicit client_auth=false remains the pre-existing opt-out that requests but does not require a client certificate.

Comment on lines +92 to 94
CLIENT_AUTH=$(extract_field "$GNMI" 'if .client_auth == null then empty else .client_auth end')
if [ "$CERTIFICATE_FREE_TLS" == "true" ] || [ "$CLIENT_AUTH" == "false" ] || { [ -z "$CLIENT_AUTH" ] && [ "$HAS_CLIENT_CA" == "false" ]; }; then
TELEMETRY_ARGS+=" --allow_no_client_auth"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intent clarified in the PR body: CA-backed paths keep the existing mTLS default, while explicit client_auth=false remains the pre-existing opt-out that requests but does not require a client certificate.

@hdwhdw

hdwhdw commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hdwhdw

hdwhdw commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Verification status: Physical checks passed for the exercised DPU0 paths. Azure run 1187671 remains in progress overall; its BF3 and Mellanox jobs and artifacts passed.

Image and hardware

  • Source PR head: bd9a27817
  • Embedded image: SONiC.master-28915.1187671-f1a440da3
  • Physical SmartSwitch with one NPU and four BF3 DPUs

Evidence

  • The packaged launcher on DPU0 served TLS. System.Time passed in 5/5 direct gnoi_client -insecure calls.
  • System.Time also passed through DPUProxy on the physical NPU using metadata-routed UDS.
  • The packaged utility observed port 8080 closed and 50052 open, selected 50052, and completed gNOI halt successfully. A hardware reset followed.

Additional coverage and scope

  • Separate sonic-gnmi targeted and full CI passed; 16 focused utility tests and 37 focused Host Services tests passed.
  • Host Services Reboot was not separately invoked on hardware. Its port-selection behavior is covered by the 37 focused tests.
  • The isolated-midplane TLS mode encrypts traffic without certificate-based peer identity verification. CA-backed mTLS and explicit password/JWT authentication are available, and an unverified client CN is not logged as the principal.

The NPU and DPU0 were restored to their exact baseline images and defaults. Candidate images, temporary local/server/DUT artifacts, credentials, and the controller were removed. All four DPUs report Online; the testbed was released READY and unlocked.

@hdwhdw

hdwhdw commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1187671:

✅Stage Test:

  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.
  • Job impacted-area-kvmtest-t0 by Elastictest: retried.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: [SmartSwitch] DPU gNMI/gNOI service binds to loopback only after sonic-gnmi#712, breaking NPU proxy connectivity

3 participants