From 289817ea5e3c007cfcd0f7fc797db15b35e49ab1 Mon Sep 17 00:00:00 2001 From: Jan van de Pol Date: Wed, 1 Apr 2026 11:50:16 +0200 Subject: [PATCH 1/2] Refactor Windows credential management to use direct API calls instead of cmdkey --- internal/launcher/launcher.go | 27 +-------- internal/launcher/launcher_unix.go | 3 + internal/launcher/launcher_windows.go | 81 +++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 25 deletions(-) diff --git a/internal/launcher/launcher.go b/internal/launcher/launcher.go index 3b7212e..316057c 100644 --- a/internal/launcher/launcher.go +++ b/internal/launcher/launcher.go @@ -601,17 +601,7 @@ func (l *Launcher) storeWindowsCredential(conn *models.Connection) error { username = fmt.Sprintf("%s\\%s", conn.Domain, conn.Username) } - // Use cmdkey to store the credential - // cmdkey /generic:TERMSRV/hostname /user:username /pass:password - cmd := exec.Command("cmdkey", "/generic:TERMSRV/"+target, "/user:"+username, "/pass:"+conn.Password) - hideConsoleWindow(cmd) - - output, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("cmdkey failed: %w, output: %s", err, string(output)) - } - - return nil + return writeWindowsCredential("TERMSRV/"+target, username, conn.Password) } // RemoveWindowsCredential removes RDP credentials from Windows Credential Manager @@ -630,20 +620,7 @@ func (l *Launcher) RemoveWindowsCredential(conn *models.Connection) error { target = fmt.Sprintf("%s:%d", conn.Host, port) } - // Use cmdkey to delete the credential - // cmdkey /delete:TERMSRV/hostname - cmd := exec.Command("cmdkey", "/delete:TERMSRV/"+target) - hideConsoleWindow(cmd) - - output, err := cmd.CombinedOutput() - if err != nil { - // Don't return error if credential doesn't exist - if !strings.Contains(string(output), "not found") { - return fmt.Errorf("cmdkey delete failed: %w, output: %s", err, string(output)) - } - } - - return nil + return deleteWindowsCredential("TERMSRV/" + target) } // CleanupAllCredentials removes all MremoteGO-stored credentials from Windows Credential Manager diff --git a/internal/launcher/launcher_unix.go b/internal/launcher/launcher_unix.go index 0ce27cc..41aa414 100644 --- a/internal/launcher/launcher_unix.go +++ b/internal/launcher/launcher_unix.go @@ -11,3 +11,6 @@ import ( func hideConsoleWindow(cmd *exec.Cmd) { // Nothing to do on Unix-like systems } + +func writeWindowsCredential(target, username, password string) error { return nil } +func deleteWindowsCredential(target string) error { return nil } diff --git a/internal/launcher/launcher_windows.go b/internal/launcher/launcher_windows.go index c8d8a3b..0f76c7c 100644 --- a/internal/launcher/launcher_windows.go +++ b/internal/launcher/launcher_windows.go @@ -4,8 +4,11 @@ package launcher import ( + "fmt" "os/exec" + "runtime" "syscall" + "unsafe" ) // hideConsoleWindow sets the command attributes to hide console windows on Windows @@ -15,3 +18,81 @@ func hideConsoleWindow(cmd *exec.Cmd) { CreationFlags: 0x08000000, // CREATE_NO_WINDOW } } + +// writeWindowsCredential stores a generic credential in Windows Credential Manager +// using CredWriteW directly, avoiding cmdkey's argument-parsing issues with +// usernames or passwords that contain spaces or special characters. +func writeWindowsCredential(target, username, password string) error { + credWriteW := syscall.NewLazyDLL("advapi32.dll").NewProc("CredWriteW") + + targetPtr, err := syscall.UTF16PtrFromString(target) + if err != nil { + return fmt.Errorf("invalid target name: %w", err) + } + userPtr, err := syscall.UTF16PtrFromString(username) + if err != nil { + return fmt.Errorf("invalid username: %w", err) + } + + // Encode password as UTF-16LE bytes without null terminator, as expected by + // Windows Credential Manager for RDP credentials consumed by mstsc. + passUTF16 := syscall.StringToUTF16(password) + passUTF16 = passUTF16[:len(passUTF16)-1] // strip null terminator + passBlobSize := uint32(len(passUTF16) * 2) + + // CREDENTIALW mirrors the Win32 CREDENTIALW struct layout. + type credentialW struct { + Flags uint32 + Type uint32 + TargetName *uint16 + Comment *uint16 + LastWritten [2]uint32 + CredentialBlobSize uint32 + CredentialBlob uintptr + Persist uint32 + AttributeCount uint32 + Attributes uintptr + TargetAlias *uint16 + UserName *uint16 + } + + var blobPtr uintptr + if passBlobSize > 0 { + blobPtr = uintptr(unsafe.Pointer(&passUTF16[0])) + } + + cred := credentialW{ + Type: 1, // CRED_TYPE_GENERIC + TargetName: targetPtr, + UserName: userPtr, + CredentialBlobSize: passBlobSize, + CredentialBlob: blobPtr, + Persist: 2, // CRED_PERSIST_LOCAL_MACHINE + } + + ret, _, callErr := credWriteW.Call(uintptr(unsafe.Pointer(&cred)), 0) + runtime.KeepAlive(passUTF16) // prevent GC until after the syscall + if ret == 0 { + return fmt.Errorf("CredWriteW failed: %w", callErr) + } + return nil +} + +// deleteWindowsCredential removes a generic credential from Windows Credential Manager. +// Returns nil if the credential does not exist. +func deleteWindowsCredential(target string) error { + credDeleteW := syscall.NewLazyDLL("advapi32.dll").NewProc("CredDeleteW") + + targetPtr, err := syscall.UTF16PtrFromString(target) + if err != nil { + return fmt.Errorf("invalid target name: %w", err) + } + + const credTypeGeneric = 1 + const errorNotFound = syscall.Errno(1168) // ERROR_NOT_FOUND + ret, _, callErr := credDeleteW.Call(uintptr(unsafe.Pointer(targetPtr)), credTypeGeneric, 0) + if ret == 0 && callErr != errorNotFound { + return fmt.Errorf("CredDeleteW failed: %w", callErr) + } + return nil +} From c2947a0495ccc33dd3efb6772e1bed19dfd7d411 Mon Sep 17 00:00:00 2001 From: Jan van de Pol Date: Wed, 1 Apr 2026 12:15:21 +0200 Subject: [PATCH 2/2] Fix password encoding in writeWindowsCredential to handle errors correctly --- internal/launcher/launcher_windows.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/launcher/launcher_windows.go b/internal/launcher/launcher_windows.go index 0f76c7c..047f1a4 100644 --- a/internal/launcher/launcher_windows.go +++ b/internal/launcher/launcher_windows.go @@ -36,7 +36,10 @@ func writeWindowsCredential(target, username, password string) error { // Encode password as UTF-16LE bytes without null terminator, as expected by // Windows Credential Manager for RDP credentials consumed by mstsc. - passUTF16 := syscall.StringToUTF16(password) + passUTF16, err := syscall.UTF16FromString(password) + if err != nil { + return fmt.Errorf("invalid password: %w", err) + } passUTF16 = passUTF16[:len(passUTF16)-1] // strip null terminator passBlobSize := uint32(len(passUTF16) * 2)