Conversation
Signed-off-by: Dennis Lanov <dennis.lanov@gmail.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a secure local Unix-domain-socket REST path for
sonic-cliwhile preserving the existing authenticated HTTPS/TCP REST behavior.A fresh SONiC image can run the REST server with
client_auth=userby default, but the Pythonsonic-cliREST client currently talks tohttps://localhostwithout credentials. That makes local CLI REST requests fail with HTTP 401 even though remote REST authentication is intentionally enabled.This change keeps remote REST authentication unchanged and adds a separate local transport at
/var/run/rest-local.sock.Changes
/var/run/rest-local.sockSO_PEERCREDon accepted Unix connections to obtain the caller's kernel UID/GID/PID/host_etc/passwd/host_etc/passwdand/host_etc/groupCLI_USER, HTTP headers, or other client-provided identity for authorizationApiClientdefault local transport to use the Unix socket without introducing a new Python dependencyrest_cl.cpppath unchanged; it already targets/var/run/rest-local.sockAuthorization behavior
Remote TCP/TLS behavior is unchanged:
Local Unix-socket behavior:
The local username is derived server-side from kernel-provided peer credentials. Client-supplied headers or environment variables are not used for authorization.
Security rationale
The REST server's existing
client_auth=userdefault is intentionally preserved. This PR does not weaken remote REST security and does not revert the secure default.A flat "Unix socket access implies admin" bypass is also avoided because Klish itself does not enforce the local read/write privilege boundary. The Unix socket therefore bypasses only the password challenge; authorization is still enforced using the actual peer UID and host account/group data.
Testing
Python Unix-socket tests:
Results:
CLI/tests/test_cli_client.py: 7 passedThe new client tests use a real AF_UNIX HTTP server and cover GET, PATCH body delivery, default adapter wiring, and the absence of an Authorization header.
Go validation:
The repository's native REST Go test target is:
It depends on the normal
sonic-mgmt-common/ libyang / generated ocbinds build chain, which was not available in the local checkout. The same pre-existing build failure was reproduced on the unmodified tree.The new Go logic was additionally exercised in an isolated scratch module using the exact implementation logic, including a real
net.Listen("unix", ...)/net.Dial("unix", ...)round trip forSO_PEERCRED; 14 tests passed covering peer credential extraction, host identity parsing, 401/403/200 authorization behavior, and forged-header resistance.Additional checks:
All completed cleanly.
Fixes sonic-net/sonic-buildimage#29504