-
Notifications
You must be signed in to change notification settings - Fork 2k
Connect: add privileged updater service #63572
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
9650e79
Add privileged updater service
gzdunek 44125cf
Add integration tests for updater
gzdunek f475108
Review fixes
gzdunek 0a56298
Move privileged updater to its own module
gzdunek b7f5da4
Fix comments
gzdunek d8066ff
Interpolate registry pathnames, switch errors to AccessDenied
gzdunek bdcab1f
Improve error handling in `waitForSingleClient`
gzdunek c14c00a
Use stricter DACL for named pipe
gzdunek 4822f37
Close `conn` on context cancellation
gzdunek cc9a692
Move reading update meta to separate function
gzdunek 6c70b3a
`trace.LimitExceeded` -> `trace.Errorf`
gzdunek 7c41617
Merge branch 'master' into gzdunek/connect-updater-1
gzdunek bae3b1f
Fix test
gzdunek 5854e50
Merge branch 'master' into gzdunek/connect-updater-1
gzdunek eed2bfc
Ensure updater only allows HTTPS
gzdunek 316691e
Use TLS server in tests
gzdunek 1cadc54
Fix tests
gzdunek 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
453 changes: 453 additions & 0 deletions
453
integration/autoupdate/tools/connect_privileged_updater_windows_test.go
Large diffs are not rendered by default.
Oops, something went wrong.
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,35 @@ | ||
| // Teleport | ||
| // Copyright (C) 2026 Gravitational, Inc. | ||
| // | ||
| // This program is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU Affero General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
| // | ||
| // This program is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU Affero General Public License for more details. | ||
| // | ||
| // You should have received a copy of the GNU Affero General Public License | ||
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| package common | ||
|
|
||
| import ( | ||
| "github.com/gravitational/teleport/lib/autoupdate" | ||
| "github.com/gravitational/teleport/lib/modules" | ||
| ) | ||
|
|
||
| // TeleportToolsVersionOff indicates that managed updates are disabled ("off"). | ||
| const TeleportToolsVersionOff = "off" | ||
|
|
||
| // GetDefaultBaseURL returns the default base URL used to download artifacts. | ||
| func GetDefaultBaseURL() string { | ||
| m := modules.GetModules() | ||
| // Uses the same logic as the teleport/lib/autoupdate/tools package. | ||
| if m.BuildType() != modules.BuildOSS { | ||
| return autoupdate.DefaultBaseURL | ||
| } | ||
| return "" | ||
| } |
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,86 @@ | ||
| // Teleport | ||
| // Copyright (C) 2026 Gravitational, Inc. | ||
| // | ||
| // This program is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU Affero General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
| // | ||
| // This program is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU Affero General Public License for more details. | ||
| // | ||
| // You should have received a copy of the GNU Affero General Public License | ||
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| package common | ||
|
|
||
| import ( | ||
| "errors" | ||
|
|
||
| "github.com/gravitational/trace" | ||
| "golang.org/x/sys/windows/registry" | ||
| ) | ||
|
|
||
| const ( | ||
| // TeleportConnectPoliciesKeyPath is the Windows registry path for Teleport Connect policy settings. | ||
| TeleportConnectPoliciesKeyPath = `SOFTWARE\Policies\Teleport\TeleportConnect` | ||
| // RegistryValueToolsVersion is the policy value name that pins the managed tools version. | ||
| RegistryValueToolsVersion = "ToolsVersion" | ||
| // RegistryValueCDNBaseURL is the policy value name that configures the managed update CDN base URL. | ||
| RegistryValueCDNBaseURL = "CdnBaseUrl" | ||
| ) | ||
|
|
||
| // PolicyValues defines the managed update policy configuration. | ||
| type PolicyValues struct { | ||
| // CDNBaseURL is the base URL used to download artifacts. | ||
| CDNBaseURL string | ||
| // Version specifies the enforced application version. | ||
| Version string | ||
| } | ||
|
|
||
| // ReadRegistryPolicyValues reads system policy values (tools version and CDN base URL) for Teleport Connect. | ||
| func ReadRegistryPolicyValues(key registry.Key) (*PolicyValues, error) { | ||
| version, err := ReadRegistryValue(key, TeleportConnectPoliciesKeyPath, RegistryValueToolsVersion) | ||
| if err != nil && !trace.IsNotFound(err) { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
|
|
||
| url, err := ReadRegistryValue(key, TeleportConnectPoliciesKeyPath, RegistryValueCDNBaseURL) | ||
| if err != nil && !trace.IsNotFound(err) { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
|
|
||
| return &PolicyValues{ | ||
| CDNBaseURL: url, | ||
| Version: version, | ||
| }, nil | ||
| } | ||
|
|
||
| // ReadRegistryValue reads a registry value. | ||
| func ReadRegistryValue(hive registry.Key, pathName string, valueName string) (path string, err error) { | ||
| key, err := registry.OpenKey(hive, pathName, registry.READ) | ||
| if err != nil { | ||
| if errors.Is(err, registry.ErrNotExist) { | ||
| return "", trace.NotFound("registry key %s not found", pathName) | ||
| } | ||
| return "", trace.Wrap(err, "opening registry key %s", pathName) | ||
| } | ||
|
|
||
| defer func() { | ||
| if closeErr := key.Close(); closeErr != nil && err == nil { | ||
| err = trace.Wrap(closeErr, "closing registry key %s", pathName) | ||
| } | ||
| }() | ||
|
|
||
| path, _, err = key.GetStringValue(valueName) | ||
| if err != nil { | ||
| if errors.Is(err, registry.ErrNotExist) { | ||
| return "", trace.NotFound("registry value %s not found in %s", valueName, pathName) | ||
| } | ||
| return "", trace.Wrap(err, "reading registry value %s from %s", valueName, pathName) | ||
| } | ||
|
|
||
| return path, nil | ||
| } |
164 changes: 164 additions & 0 deletions
164
lib/teleterm/autoupdate/privilegedupdater/client_windows.go
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,164 @@ | ||
| // Teleport | ||
| // Copyright (C) 2026 Gravitational, Inc. | ||
| // | ||
| // This program is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU Affero General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
| // | ||
| // This program is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU Affero General Public License for more details. | ||
| // | ||
| // You should have received a copy of the GNU Affero General Public License | ||
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| package privilegedupdater | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "net" | ||
| "os" | ||
| "syscall" | ||
| "time" | ||
|
|
||
| "github.com/Microsoft/go-winio" | ||
| "github.com/gravitational/trace" | ||
| "golang.org/x/sys/windows" | ||
| "golang.org/x/sys/windows/svc" | ||
| "golang.org/x/sys/windows/svc/mgr" | ||
|
|
||
| "github.com/gravitational/teleport/api/utils/retryutils" | ||
| ) | ||
|
|
||
| const ( | ||
| serviceStartTimeout = 5 * time.Second | ||
| serviceStartRetryStep = 500 * time.Millisecond | ||
| serviceStartRetryMax = 500 * time.Millisecond | ||
| pipeDialTimeout = 3 * time.Second | ||
| pipeDialRetryStep = 100 * time.Millisecond | ||
| pipeDialRetryMax = 300 * time.Millisecond | ||
| ) | ||
|
|
||
| // RunServiceAndInstallUpdateFromClient is called by the client. | ||
| // It starts the update service, sends update metadata, and transfers the binary for validation and installation. | ||
| func RunServiceAndInstallUpdateFromClient(ctx context.Context, path string, forceRun bool, version string) error { | ||
| if err := ensureServiceRunning(ctx); err != nil { | ||
| // Service failed to start; fall back to client-side install (UAC). | ||
| if installErr := runInstaller(path, forceRun); installErr != nil { | ||
| return trace.Wrap(installErr, "fallback install failed after service start error: %v", err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| err := InstallUpdateFromClient(ctx, path, forceRun, version) | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| // InstallUpdateFromClient sends update metadata, and transfers the binary for validation and installation. | ||
| func InstallUpdateFromClient(ctx context.Context, path string, forceRun bool, version string) error { | ||
| conn, err := dialPipeWithRetry(ctx, PipePath) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
| defer conn.Close() | ||
|
|
||
| // The update must be read by the client running as a standard user. | ||
| // Passing the path directly to the SYSTEM service could cause it to read | ||
| // files the user is not permitted to access. | ||
| file, err := os.Open(path) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
| defer file.Close() | ||
|
|
||
| meta := updateMetadata{ForceRun: forceRun, Version: version} | ||
| return trace.Wrap(writeUpdate(conn, meta, file)) | ||
| } | ||
|
|
||
| func dialPipeWithRetry(ctx context.Context, path string) (net.Conn, error) { | ||
| ctx, cancel := context.WithTimeout(ctx, pipeDialTimeout) | ||
| defer cancel() | ||
| linearRetry, err := retryutils.NewLinear(retryutils.LinearConfig{ | ||
| Step: pipeDialRetryStep, | ||
| Max: pipeDialRetryMax, | ||
| }) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
|
|
||
| isRetryError := func(err error) bool { | ||
| return errors.Is(err, windows.ERROR_FILE_NOT_FOUND) | ||
|
cthach marked this conversation as resolved.
|
||
| } | ||
|
|
||
| var conn net.Conn | ||
| err = linearRetry.For(ctx, func() error { | ||
| conn, err = winio.DialPipeAccess(ctx, path, uint32(SafePipeReadWriteAccess)) | ||
| if err != nil && !isRetryError(err) { | ||
| return retryutils.PermanentRetryError(trace.Wrap(err)) | ||
| } | ||
| return trace.Wrap(err) | ||
| }) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
| return conn, nil | ||
| } | ||
|
|
||
| func ensureServiceRunning(ctx context.Context) error { | ||
| ctx, cancel := context.WithTimeout(ctx, serviceStartTimeout) | ||
| defer cancel() | ||
| // Avoid [mgr.Connect] because it requests elevated permissions. | ||
| scManager, err := windows.OpenSCManager(nil /*machine*/, nil /*database*/, windows.SC_MANAGER_CONNECT) | ||
| if err != nil { | ||
| return trace.Wrap(err, "opening Windows service manager") | ||
| } | ||
| defer windows.CloseServiceHandle(scManager) | ||
| serviceNamePtr, err := syscall.UTF16PtrFromString(serviceName) | ||
| if err != nil { | ||
| return trace.Wrap(err, "converting service name to UTF16") | ||
| } | ||
| serviceHandle, err := windows.OpenService(scManager, serviceNamePtr, serviceAccessFlags) | ||
| if err != nil { | ||
| return trace.Wrap(err, "opening Windows service %v", serviceName) | ||
| } | ||
| service := &mgr.Service{ | ||
| Name: serviceName, | ||
| Handle: serviceHandle, | ||
| } | ||
| defer service.Close() | ||
|
|
||
| status, err := service.Query() | ||
| if err != nil { | ||
| return trace.Wrap(err, "querying service status") | ||
| } | ||
| if status.State == svc.Running { | ||
| return nil | ||
| } | ||
|
|
||
| if err = service.Start(ServiceCommand); err != nil { | ||
| return trace.Wrap(err, "starting Windows service %s", serviceName) | ||
| } | ||
|
|
||
| linearRetry, err := retryutils.NewLinear(retryutils.LinearConfig{ | ||
| Step: serviceStartRetryStep, | ||
| Max: serviceStartRetryMax, | ||
| }) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| err = linearRetry.For(ctx, func() error { | ||
| status, err = service.Query() | ||
| if err != nil { | ||
| return retryutils.PermanentRetryError(trace.Wrap(err)) | ||
| } | ||
| if status.State != svc.Running { | ||
| return trace.Errorf("service not running yet") | ||
| } | ||
| return nil | ||
| }) | ||
| return trace.Wrap(err) | ||
| } | ||
107 changes: 107 additions & 0 deletions
107
lib/teleterm/autoupdate/privilegedupdater/protocol_windows.go
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,107 @@ | ||
| // Teleport | ||
| // Copyright (C) 2026 Gravitational, Inc. | ||
| // | ||
| // This program is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU Affero General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
| // | ||
| // This program is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU Affero General Public License for more details. | ||
| // | ||
| // You should have received a copy of the GNU Affero General Public License | ||
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| package privilegedupdater | ||
|
|
||
| import ( | ||
| "encoding/binary" | ||
| "encoding/json" | ||
| "io" | ||
| "os" | ||
|
|
||
| "github.com/gravitational/trace" | ||
|
|
||
| "github.com/gravitational/teleport/lib/utils" | ||
| ) | ||
|
|
||
| const ( | ||
| PipePath = `\\.\pipe\TeleportConnectUpdaterPipe` | ||
| maxUpdateMetadataSize = 1 * 1024 * 1024 // 1 MiB | ||
| maxUpdatePayloadSize = 1 * 1024 * 1024 * 1024 // 1 GiB | ||
| ) | ||
|
|
||
| type updateMetadata struct { | ||
| // ForceRun determines whether to run the app after installing the update. | ||
| ForceRun bool `json:"force_run"` | ||
| // Version is update version. | ||
| Version string `json:"version"` | ||
| } | ||
|
|
||
| // writeUpdate writes an update stream in the following order: | ||
| // 1. An uint32 specifying the length of the updateMetadata header. | ||
| // 2. The updateMetadata header of the specified length. | ||
| // 3. The update binary, read until EOF. | ||
| func writeUpdate(conn io.Writer, meta updateMetadata, file io.Reader) error { | ||
| if meta.Version == "" { | ||
| return trace.BadParameter("update version is required") | ||
| } | ||
|
|
||
| metaBytes, err := json.Marshal(meta) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
| if len(metaBytes) > maxUpdateMetadataSize { | ||
| return trace.BadParameter("update metadata payload too large") | ||
| } | ||
|
|
||
| if err = binary.Write(conn, binary.LittleEndian, uint32(len(metaBytes))); err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
| if _, err = conn.Write(metaBytes); err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| _, err = io.Copy(conn, file) | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| // readUpdate reads an update stream in the following order: | ||
| // 1. An uint32 specifying the length of the updateMetadata header. | ||
| // 2. The updateMetadata header of the specified length. | ||
| // 3. The update binary, read until EOF. | ||
| // | ||
| // It writes the installer to destinationPath and returns the parsed metadata. | ||
| func readUpdate(conn io.Reader, destinationPath string) (*updateMetadata, error) { | ||
| var jsonLen uint32 | ||
| if err := binary.Read(conn, binary.LittleEndian, &jsonLen); err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
| if jsonLen > maxUpdateMetadataSize { | ||
| return nil, trace.BadParameter("update metadata payload too large") | ||
| } | ||
|
|
||
| buf := make([]byte, jsonLen) | ||
| _, err := io.ReadFull(conn, buf) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
| meta := &updateMetadata{} | ||
| if err = json.Unmarshal(buf, meta); err != nil { | ||
| return nil, trace.Wrap(err, "failed to unmarshal update metadata") | ||
| } | ||
| if meta.Version == "" { | ||
| return nil, trace.BadParameter("update version is required") | ||
| } | ||
|
|
||
| outFile, err := os.OpenFile(destinationPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0600) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
|
|
||
| payloadReader := utils.LimitReader(conn, maxUpdatePayloadSize) | ||
| _, err = io.Copy(outFile, payloadReader) | ||
| return meta, trace.NewAggregate(err, outFile.Close()) | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment would make a good error log message and we could also include
errto know what the error was, no? At the momenterris just lost.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the error is currently being swallowed. The issue is that there’s no way to log it at that point - Connect has already exited, and tsh.exe doesn't write logs to file (we would need a separate file just for this call).
I addressed this in #63573 (comment). Connect now invokes
RunServiceAndInstallUpdateFromClientsynchronously, which allows it to properly capture and handle any errors returned from that call.I also moved the UAC fallback logic into the JS layer, which feels like a more appropriate place for it.