-
Notifications
You must be signed in to change notification settings - Fork 315
Add unix: and npipe: transports to AZD external authentication #8371
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
Draft
Copilot
wants to merge
13
commits into
main
Choose a base branch
from
copilot/add-unix-domain-socket-windows-named-pipe-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0e07e61
Add UDS and Windows named pipe transports for external auth
Copilot 5f53437
Address review feedback: tidy imports and docs spacing
Copilot 4750628
Replace SDDL regex parsing with structured ACE walk
Copilot 333bd2b
Clarify ACE walk: rename, document unsafe cast, improve error
Copilot fb68c22
Merge branch 'main' into copilot/add-unix-domain-socket-windows-named…
bwateratmsft 78d32a9
Merge branch 'main' into copilot/add-unix-domain-socket-windows-named…
bwateratmsft 1d27979
Fix spelling issues
bwateratmsft 2e9c0f2
Missed one
bwateratmsft 850e8a9
Fix gosec findings in unix auth transport checks
Copilot 27fcf13
fix: document audited windows ACE SID cast
Copilot 36e2167
Require AZD_AUTH_KEY universally, require cert for https, reject syml…
Copilot 1e12c1c
CSpell yet again...
bwateratmsft 40199b4
Fix gosec G703 in unix auth transport socket check
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "net/url" | ||
|
|
||
| "github.com/azure/azure-dev/cli/azd/pkg/auth" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/httputil" | ||
| ) | ||
|
|
||
| // rewrittenAuthEndpoint is the canonical placeholder URL used as the | ||
| // AZD_AUTH_ENDPOINT after rewriting unix:/npipe: schemes. RemoteCredential | ||
| // formats the request URL as "<endpoint>/token?api-version=..." so this | ||
| // placeholder produces a syntactically valid URL whose host/path are | ||
| // irrelevant because the transport dials a fixed socket/pipe. | ||
| const rewrittenAuthEndpoint = "http://azd-auth" | ||
|
|
||
| // buildExternalAuthConfiguration constructs the auth.ExternalAuthConfiguration | ||
| // from the raw AZD_AUTH_* env values. It dispatches on the scheme of the | ||
| // endpoint URL: | ||
| // | ||
| // - "" or "https": existing loopback HTTPS behavior. AZD_AUTH_CERT is | ||
| // required for "https". | ||
| // - "unix": POSIX-only Unix domain socket transport. Cert MUST NOT be set. | ||
| // Key is optional but still forwarded for defense in depth. | ||
| // - "npipe": Windows-only named pipe transport. Cert MUST NOT be set. Key | ||
| // is optional but still forwarded for defense in depth. | ||
| // | ||
| // Any other scheme yields an error that lists the supported schemes. | ||
| func buildExternalAuthConfiguration(endpoint, key, cert string) (auth.ExternalAuthConfiguration, error) { | ||
| // Parse the endpoint up front so we can dispatch on its scheme. An empty | ||
| // endpoint string parses successfully with an empty scheme, which is the | ||
| // historical "no external auth configured" / "implicit http for tests" | ||
| // case. | ||
| endpointUrl, err := url.Parse(endpoint) | ||
| if err != nil { | ||
| return auth.ExternalAuthConfiguration{}, | ||
| fmt.Errorf("invalid AZD_AUTH_ENDPOINT value '%s': %w", endpoint, err) | ||
| } | ||
|
|
||
| switch endpointUrl.Scheme { | ||
| case "", "http", "https": | ||
| return buildHTTPSExternalAuth(endpoint, key, cert, endpointUrl.Scheme) | ||
| case "unix": | ||
| return buildLocalIPCExternalAuth(endpoint, key, cert, newSocketTransport) | ||
| case "npipe": | ||
| return buildLocalIPCExternalAuth(endpoint, key, cert, newPipeTransport) | ||
| default: | ||
| return auth.ExternalAuthConfiguration{}, fmt.Errorf( | ||
| "invalid AZD_AUTH_ENDPOINT value '%s': unsupported scheme %q "+ | ||
| "(supported schemes: https, unix, npipe)", | ||
| endpoint, endpointUrl.Scheme) | ||
| } | ||
| } | ||
|
|
||
| // buildHTTPSExternalAuth implements the historical HTTPS / no-scheme path. | ||
| // When a cert is provided, the scheme MUST be "https". | ||
| func buildHTTPSExternalAuth(endpoint, key, cert, scheme string) (auth.ExternalAuthConfiguration, error) { | ||
| client := &http.Client{} | ||
| if len(cert) > 0 { | ||
| transport, err := httputil.TlsEnabledTransport(cert) | ||
|
bwateratmsft marked this conversation as resolved.
|
||
| if err != nil { | ||
| return auth.ExternalAuthConfiguration{}, | ||
| fmt.Errorf("parsing AZD_AUTH_CERT: %w", err) | ||
| } | ||
| client.Transport = transport | ||
|
|
||
| if scheme != "https" { | ||
| return auth.ExternalAuthConfiguration{}, | ||
| fmt.Errorf( | ||
| "invalid AZD_AUTH_ENDPOINT value '%s': scheme must be 'https' when certificate is provided", | ||
| endpoint) | ||
| } | ||
| } | ||
| return auth.ExternalAuthConfiguration{ | ||
| Endpoint: endpoint, | ||
| Transporter: client, | ||
| Key: key, | ||
| }, nil | ||
| } | ||
|
|
||
| // buildLocalIPCExternalAuth implements the unix: / npipe: paths. Both share | ||
| // the same shape: cert is forbidden, key is optional, the transport is built | ||
| // by the platform-specific factory, and the endpoint is rewritten to a | ||
| // canonical placeholder so RemoteCredential can format request URLs. | ||
| func buildLocalIPCExternalAuth( | ||
| endpoint, key, cert string, | ||
| newTransport func(string) (http.RoundTripper, string, error), | ||
| ) (auth.ExternalAuthConfiguration, error) { | ||
| if len(cert) > 0 { | ||
| return auth.ExternalAuthConfiguration{}, fmt.Errorf( | ||
| "AZD_AUTH_CERT must not be set when AZD_AUTH_ENDPOINT uses a local IPC scheme " + | ||
| "(unix:, npipe:); the OS enforces caller identity") | ||
| } | ||
| transport, rewritten, err := newTransport(endpoint) | ||
| if err != nil { | ||
| return auth.ExternalAuthConfiguration{}, err | ||
| } | ||
| return auth.ExternalAuthConfiguration{ | ||
| Endpoint: rewritten, | ||
| Transporter: &http.Client{Transport: transport}, | ||
| Key: key, | ||
| }, nil | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| //go:build !unix && !windows | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| ) | ||
|
|
||
| // newSocketTransport is not supported on this platform. | ||
| func newSocketTransport(rawURL string) (http.RoundTripper, string, error) { | ||
| return nil, "", fmt.Errorf( | ||
| "AZD_AUTH_ENDPOINT scheme 'unix' is not supported on this platform") | ||
| } | ||
|
|
||
| // newPipeTransport is not supported on this platform. | ||
| func newPipeTransport(rawURL string) (http.RoundTripper, string, error) { | ||
| return nil, "", fmt.Errorf( | ||
| "AZD_AUTH_ENDPOINT scheme 'npipe' is not supported on this platform") | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestBuildExternalAuthConfiguration_Schemes exercises the scheme dispatch in | ||
| // buildExternalAuthConfiguration. Per-scheme transport construction (unix | ||
| // permission checks, Windows pipe SD checks) is covered by the platform- | ||
| // specific tests in auth_transport_unix_test.go / auth_transport_windows_test.go. | ||
| func TestBuildExternalAuthConfiguration_Schemes(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| endpoint string | ||
| key string | ||
| cert string | ||
| wantErrSub string // substring expected in error message; empty means no error expected | ||
| wantRewrite string // expected Endpoint on success; empty to skip | ||
| }{ | ||
| { | ||
| name: "empty endpoint, no cert preserves backward compat", | ||
| endpoint: "", | ||
| key: "k", | ||
| cert: "", | ||
| }, | ||
| { | ||
| name: "https without cert keeps current behavior (no cert required at config time)", | ||
| endpoint: "https://127.0.0.1:1234", | ||
| key: "k", | ||
| cert: "", | ||
| }, | ||
|
bwateratmsft marked this conversation as resolved.
|
||
| { | ||
| name: "http with cert is rejected because cert requires https", | ||
| endpoint: "http://127.0.0.1:1234", | ||
| key: "k", | ||
| cert: "not-a-real-cert", | ||
| wantErrSub: "AZD_AUTH_CERT", // cert parse failure fires first | ||
| }, | ||
| { | ||
| name: "http without cert is preserved for backward compat", | ||
| endpoint: "http://127.0.0.1:1234", | ||
| key: "k", | ||
| cert: "", | ||
| }, | ||
| { | ||
| name: "unix scheme rejects cert", | ||
| endpoint: "unix:/tmp/some.sock", | ||
| key: "k", | ||
| cert: "anything", | ||
| wantErrSub: "AZD_AUTH_CERT must not be set", | ||
| }, | ||
| { | ||
| name: "npipe scheme rejects cert", | ||
| endpoint: "npipe:azd-auth-x", | ||
| key: "k", | ||
| cert: "anything", | ||
| wantErrSub: "AZD_AUTH_CERT must not be set", | ||
| }, | ||
| { | ||
| name: "unknown scheme is refused with a list of supported schemes", | ||
| endpoint: "ftp://nope", | ||
| key: "k", | ||
| cert: "", | ||
| wantErrSub: "supported schemes: https, unix, npipe", | ||
| }, | ||
| { | ||
| name: "malformed url is reported", | ||
| endpoint: "://broken", | ||
| wantErrSub: "invalid AZD_AUTH_ENDPOINT", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| cfg, err := buildExternalAuthConfiguration(tt.endpoint, tt.key, tt.cert) | ||
| if tt.wantErrSub != "" { | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), tt.wantErrSub) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
| require.NotNil(t, cfg.Transporter) | ||
| require.Equal(t, tt.key, cfg.Key) | ||
| if tt.wantRewrite != "" { | ||
| require.Equal(t, tt.wantRewrite, cfg.Endpoint) | ||
| } else { | ||
| require.Equal(t, tt.endpoint, cfg.Endpoint) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestRewrittenAuthEndpoint_FormatsValidURL verifies that the placeholder | ||
| // endpoint produces a syntactically valid request URL when concatenated by | ||
| // RemoteCredential with "/token?api-version=...". | ||
| func TestRewrittenAuthEndpoint_FormatsValidURL(t *testing.T) { | ||
| t.Parallel() | ||
| require.True(t, strings.HasPrefix(rewrittenAuthEndpoint, "http://"), | ||
| "placeholder must be an absolute URL so net/http accepts it") | ||
| require.NotContains(t, rewrittenAuthEndpoint, " ") | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| //go:build unix | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "net" | ||
| "net/http" | ||
| "net/url" | ||
| "os" | ||
| "path/filepath" | ||
| "syscall" | ||
| ) | ||
|
|
||
| // newSocketTransport builds an http.RoundTripper that dispatches requests over | ||
| // the Unix domain socket identified by rawURL. The returned string is the | ||
| // rewritten endpoint placeholder that should be used in | ||
| // auth.ExternalAuthConfiguration.Endpoint. | ||
| // | ||
| // The socket file and its parent directory MUST be owned by the current uid | ||
| // and have group/other bits cleared (mode & 0o077 == 0). If either check | ||
| // fails, an error is returned and no transport is constructed. | ||
| func newSocketTransport(rawURL string) (http.RoundTripper, string, error) { | ||
| u, err := url.Parse(rawURL) | ||
| if err != nil { | ||
| return nil, "", fmt.Errorf("invalid AZD_AUTH_ENDPOINT value %q: %w", rawURL, err) | ||
| } | ||
| if u.Scheme != "unix" { | ||
| return nil, "", fmt.Errorf("internal error: newSocketTransport called with non-unix scheme %q", u.Scheme) | ||
| } | ||
|
|
||
| socketPath := u.Path | ||
| if socketPath == "" { | ||
| // url.Parse puts host of "unix:/foo" into Path but "unix://foo" puts | ||
| // "foo" into Host; fall back to Host when Path is empty. | ||
| socketPath = u.Host | ||
| } | ||
| if !filepath.IsAbs(socketPath) { | ||
| return nil, "", fmt.Errorf( | ||
| "invalid AZD_AUTH_ENDPOINT value %q: unix scheme requires an absolute socket path", rawURL) | ||
| } | ||
|
|
||
| if err := verifySocketPermissions(socketPath); err != nil { | ||
| return nil, "", err | ||
| } | ||
|
|
||
| transport := &http.Transport{ | ||
| DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { | ||
| var d net.Dialer | ||
| return d.DialContext(ctx, "unix", socketPath) | ||
| }, | ||
| } | ||
| return transport, rewrittenAuthEndpoint, nil | ||
| } | ||
|
|
||
| // verifySocketPermissions checks that the socket file and its parent | ||
| // directory are owned by the current effective uid and have group/other | ||
| // permission bits cleared. It returns a clear error when either check fails. | ||
| func verifySocketPermissions(socketPath string) error { | ||
| parent := filepath.Dir(socketPath) | ||
| if err := checkPathOwnedAndRestricted(parent, true); err != nil { | ||
| return fmt.Errorf("AZD_AUTH_ENDPOINT socket parent directory %q: %w", parent, err) | ||
| } | ||
| if err := checkPathOwnedAndRestricted(socketPath, false); err != nil { | ||
| return fmt.Errorf("AZD_AUTH_ENDPOINT socket %q: %w", socketPath, err) | ||
| } | ||
| return nil | ||
| } | ||
|
bwateratmsft marked this conversation as resolved.
|
||
|
|
||
| // checkPathOwnedAndRestricted verifies path is owned by the current euid and | ||
| // has mode bits group/other set to zero. The isDir flag is used only for | ||
| // error messages. | ||
| func checkPathOwnedAndRestricted(path string, isDir bool) error { | ||
| info, err := os.Stat(path) | ||
| if err != nil { | ||
| return fmt.Errorf("stat: %w", err) | ||
| } | ||
| sys, ok := info.Sys().(*syscall.Stat_t) | ||
| if !ok { | ||
| return fmt.Errorf("unable to read ownership information") | ||
| } | ||
| euid := uint32(os.Geteuid()) | ||
| if sys.Uid != euid { | ||
| kind := "file" | ||
| if isDir { | ||
| kind = "directory" | ||
| } | ||
| return fmt.Errorf("permissions too permissive: %s owner uid %d does not match current euid %d", | ||
| kind, sys.Uid, euid) | ||
| } | ||
| if info.Mode().Perm()&0o077 != 0 { | ||
| return fmt.Errorf( | ||
| "permissions too permissive: mode %#o grants access beyond owner (group/world bits must be 0)", | ||
| info.Mode().Perm()) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // newPipeTransport returns an error: named pipes are Windows-only. This stub | ||
| // exists so container.go can call it portably. | ||
| func newPipeTransport(rawURL string) (http.RoundTripper, string, error) { | ||
| return nil, "", fmt.Errorf( | ||
| "AZD_AUTH_ENDPOINT scheme 'npipe' is not supported on this platform; use 'unix' or 'https'") | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.