[tacacs]: Add SSH TraceId authorization attribute#27667
Conversation
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
fa88a91 to
2debd38
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This PR has backport request for branch(es): 202305,202411,202505. ---Powered by SONiC BuildBot
|
|
Validated on the pipeline build https://dev.azure.com/mssonic/build/_build/results?buildId=1128509
|
…uthorization Implements the HLD in sonic-net/SONiC#2358. Accepts a client-supplied `TraceId` env var via OpenSSH AcceptEnv, marks it readonly in the user's initial interactive bash, validates the value, and attaches it as a `TraceId=<value>` attribute on per-command TACACS+ authorization requests. Behavior is unchanged when TraceId is absent or invalid. How to verify it: - Build sonic-vs and SSH with `SendEnv TraceId` set to a valid value; confirm authorization requests include the TraceId attribute and that invalid/oversized values are silently dropped. - Run the bash_tacplus unit tests under src/tacacs/bash_tacplus/unittest/. Signed-off-by: Rod Persky <770327+Rod-Persky@users.noreply.github.com>
2debd38 to
f26d2a4
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for propagating an SSH client-supplied TraceId into TACACS+ authorization attributes, along with unit tests and OS image configuration to pass and protect the environment variable.
Changes:
- Add
TraceIdparsing/validation and include it as a TACACS+ attribute in authorization requests. - Add unit tests and mock tracking for the TraceId attribute behavior.
- Configure SSHD to accept
TraceIdviaAcceptEnvand makeTraceIdreadonly in interactive shells.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tacacs/bash_tacplus/unittest/plugin_test.c | Adds TraceId-related unit tests for authorization message behavior |
| src/tacacs/bash_tacplus/unittest/mock_helper.h | Declares mock state and reset helper for TraceId attribute capture |
| src/tacacs/bash_tacplus/unittest/mock_helper.c | Implements mock capture/reset of TraceId attribute in tac_add_attrib |
| src/tacacs/bash_tacplus/bash_tacplus.c | Implements TraceId validation/extraction and adds it to TACACS+ attributes |
| files/image_config/bash/bash.bashrc | Makes TraceId readonly (best-effort) in interactive shells |
| build_debian.sh | Ensures sshd_config includes AcceptEnv TraceId |
Signed-off-by: Rod Persky <770327+Rod-Persky@users.noreply.github.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw retry |
|
Retrying failed(or canceled) jobs... |
|
No failed(or canceled) jobs found in completed stages. Only failed(or canceled) jobs in completed stages can be retried. Stages in progress: Test. Please wait for the stages to complete and then retry again. |
|
@qiluo-msft this is a change into the existing sonic-bash plugin to capture an end to end trace ID. There is also a corresponding HLD sonic-net/SONiC#2358 . Could you please assist in identifying next steps please |
|
The backport request for branch(es) 202305,202411,202505 has been removed from PR description. Removed label(s).... ---Powered by SONiC BuildBot
|
…into dev/rodpersky/sonic-tracability
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Rod Persky <770327+Rod-Persky@users.noreply.github.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Rod Persky <770327+Rod-Persky@users.noreply.github.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |

Why I did it
Operators correlating SSH activity with TACACS+ audit logs currently have no way to tie a user's interactive shell session to a higher-level request trace. This PR implements the HLD in sonic-net/SONiC#2358 by propagating a client-supplied
SSH_CLIENT_TRACEIDfrom the SSH session into per-command TACACS+ authorization requests, so each authorized command carries the originating trace identifier. This PR also depends on sonic-net/sonic-host-services#407 for tacplus_nss configuration support.Work item tracking
How I did it
build_debian.sh: appendAcceptEnv SSH_CLIENT_TRACEIDtosshd_config(idempotent guard so re-runs don't duplicate the line) so OpenSSH will accept aSSH_CLIENT_TRACEIDenv var sent by the client viaSendEnv.files/image_config/bash/bash.bashrc: in interactive shells, markSSH_CLIENT_TRACEIDasreadonlywhen present so the user cannot mutate or unset it for the lifetime of the shell.src/tacacs/bash_tacplus/bash_tacplus.c: newget_trace_id()helper reads$SSH_CLIENT_TRACEID, enforces the TACACS+ 255-byte attribute limit, and rejects any value containing characters outside[A-Za-z0-9._:-|].send_authorization_message()attaches it as atraceid=<value>attribute on each authorization request when valid; behavior is unchanged whenSSH_CLIENT_TRACEIDis absent, empty, oversized, or contains unsafe characters (silently dropped with a debug log).src/tacacs/bash_tacplus/unittest/: extend mock_helper to captureSSH_CLIENT_TRACEIDattribute adds, and add 5 CUnit cases covering valid, missing, empty, invalid-character, and oversized values.How to verify it
SendEnv SSH_CLIENT_TRACEID(orssh -o SendEnv=SSH_CLIENT_TRACEID), export a value such asSSH_CLIENT_TRACEID=trace-123:abc.defand SSH to the DUT.traceid=trace-123:abc.defattribute alongside the usualcmd/service/protocolattributes.SSH_CLIENT_TRACEID, with an empty value, with an oversized value (>247 chars), and with disallowed characters (e.g. newline, space,;) — authorization still succeeds and notraceidattribute is sent.SSH_CLIENT_TRACEID=other/unset SSH_CLIENT_TRACEIDmust fail with "readonly variable".cd src/tacacs/bash_tacplus/unittest && make && ./plugin_testand all CUnit cases (including the 5 new TraceId tests) must pass.Which release branch to backport (provide reason below if selected)
Backport requested to 202505 (20250510), 202411 (20241110), and 202305 (20230531) although not required initially as these are major long-lived release trains used in production, and SSH↔TACACS+ traceability is needed for incident correlation on those fleets. The change is small, behind an opt-in env var, and a no-op when
TraceIdis unset, so the risk profile is acceptable for stable branches.Tested branch (Please provide the tested image version)
Description for the changelog
[tacacs]: Propagate SSH-supplied SSH_CLIENT_TRACEID env var as a TACACS+ authorization attribute for per-command audit correlation.
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)