diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5255437..d57c3d3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -45,7 +45,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '1.23' + go-version: '1.24' - name: Install Linux dependencies if: matrix.os == 'ubuntu-latest' || matrix.os == 'ubuntu-24.04-arm' @@ -53,6 +53,11 @@ jobs: sudo apt-get update sudo apt-get install -y gcc libgl1-mesa-dev xorg-dev + - name: Test + env: + CGO_ENABLED: 1 + run: go test ./... + - name: Build (Windows) if: matrix.os == 'windows-latest' env: diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ef1ee8..d10e128 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,47 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added +- **Bitwarden integration**: connection passwords can be stored as `bw://` + references and are resolved through the Bitwarden CLI at connect time. Works + with bitwarden.com, self-hosted Bitwarden and Vaultwarden. + See [docs/BITWARDEN-SETUP.md](docs/BITWARDEN-SETUP.md). +- Optional field selector on references: `bw:///username`, `/totp`, + `/notes`; the password is the default. +- **Bitwarden item picker** in the add and edit connection dialogs, with search + and a vault sync button, plus a "Store password in Bitwarden" option that + creates a login item and replaces the password with its reference. +- Secret provider abstraction (`secrets.Provider`, `secrets.Registry`), so the + configuration manager, launcher and GUI no longer depend on a single password + manager. +- Unit tests for the secret providers, reference parsing and the encryption + helper, plus a `go test ./...` step in CI. + +### Changed +- The 1Password authentication warning at start-up is now a generic secret + provider check, runs off the UI goroutine, and only asks about providers that + the configuration actually references. + +### Security +- The `bw serve` helper process is bound to loopback on a random port, started + only when a Bitwarden reference is used, and terminated on exit. It is also + placed in a Windows job object, and given a parent death signal on Linux, so + it does not survive a crash. +- Passwords are no longer passed on a command line, where any local process + could read them out of the process list: + - SSH on Windows uses PuTTY `-pwfile` with a private temporary file that is + deleted as soon as PuTTY has started, instead of `-pw`. + - SSH on Linux and macOS passes the password to `sshpass` through the + `SSHPASS` environment variable instead of `-p`; the temporary file is + removed by the generated snippet before `ssh` starts. + - RDP credentials are written to the Windows Credential Manager through the + API instead of `cmdkey /pass:`. + +### Known limitations +- On Linux, `xfreerdp` is still invoked with `/p:`. + ## [1.0.4] - 2026-01-28 ### Fixed diff --git a/README.md b/README.md index c56db0a..c288b36 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # MremoteGO -> A modern, cross-platform remote connection manager with git-friendly YAML configs and 1Password integration. +> A modern, cross-platform remote connection manager with git-friendly YAML configs and 1Password and Bitwarden integration. -[![Go Version](https://img.shields.io/badge/Go-1.23+-00ADD8?style=flat&logo=go)](https://go.dev/) +[![Go Version](https://img.shields.io/badge/Go-1.24+-00ADD8?style=flat&logo=go)](https://go.dev/) [![License](https://img.shields.io/badge/License-MIT-blue.svg)](LICENSE) [![Platform](https://img.shields.io/badge/Platform-Windows%20%7C%20Linux%20%7C%20macOS-lightgrey)](https://github.com/jaydenthorup/mremotego) @@ -10,13 +10,14 @@ **The Problem**: mRemoteNG uses XML configs that are painful to diff, merge, and share with teams. Passwords are awkwardly encrypted per-machine. -**The Solution**: MremoteGO uses clean YAML configs that work beautifully with git, plus optional 1Password integration for secure team password sharing. +**The Solution**: MremoteGO uses clean YAML configs that work beautifully with git, plus optional 1Password and Bitwarden integration for secure team password sharing. ## ✨ Features - 🎨 **Modern GUI** - Clean interface with connection tree, search, and quick actions - 🔐 **Password Encryption** - AES-256-GCM encryption at rest with master password - 🔑 **1Password Integration** - Store passwords securely using `op://` references +- 🛡️ **Bitwarden Integration** - Store passwords in Bitwarden or Vaultwarden using `bw://` references - 📝 **Git-Friendly** - YAML configs are easy to diff, merge, and review - 🖥️ **Cross-Platform** - Windows, Linux, macOS (AMD64 & ARM64) - ⚡ **Fast** - Native GUI with instant connections @@ -69,7 +70,7 @@ Simply run the executable without arguments: 1. Click **[+]** or press `Ctrl+N` 2. Fill in connection details (name, protocol, host, credentials) -3. Optionally push password to 1Password +3. Optionally pick a password from Bitwarden, or push it to 1Password or Bitwarden 4. Click **Save** **Connecting:** @@ -145,13 +146,14 @@ connections: host: dev.example.com port: 3389 username: developer + password: bw://8f3c1d9a-4e2b-4c77-9f10-1a2b3c4d5e6f # Bitwarden reference ``` ## 🔐 Security ### Password Storage Options -MremoteGO supports three password storage methods: +MremoteGO supports four password storage methods: 1. **1Password Integration** (Recommended for teams): - Store passwords securely in 1Password vaults @@ -160,20 +162,27 @@ MremoteGO supports three password storage methods: - Supports biometric unlock - See [1Password Setup Guide](docs/1PASSWORD-SETUP.md) -2. **Encrypted** (Recommended for local use): +2. **Bitwarden Integration** (Recommended for teams): + - Store passwords in Bitwarden, self-hosted Bitwarden or Vaultwarden + - Use `bw://item-id` references in your config + - Safe to commit configs to git + - Pick items from the vault directly in the connection dialog + - See [Bitwarden Setup Guide](docs/BITWARDEN-SETUP.md) + +3. **Encrypted** (Recommended for local use): - AES-256-GCM encryption with PBKDF2 key derivation (100,000 iterations) - Master password required on startup - Passwords stored as `enc:base64(salt+nonce+ciphertext)` - See [Encryption Guide](docs/ENCRYPTION.md) -3. **Plain Text** (Not recommended): +4. **Plain Text** (Not recommended): - For testing or when other methods aren't suitable - Should not be committed to git - Use `.gitignore` to exclude `connections.yaml` and `config.yaml` ### Best Practices -- ✅ Use 1Password for team environments +- ✅ Use 1Password or Bitwarden for team environments - ✅ Use encryption for personal configs - ✅ Add `config.yaml` and `connections.yaml` to `.gitignore` - ✅ Use separate configs for different environments @@ -186,13 +195,14 @@ MremoteGO supports three password storage methods: - **[GUI Guide](docs/GUI-GUIDE.md)** - Complete GUI reference - **[Encryption Guide](docs/ENCRYPTION.md)** - Password encryption details - **[1Password Setup](docs/1PASSWORD-SETUP.md)** - Secure password management +- **[Bitwarden Setup](docs/BITWARDEN-SETUP.md)** - Bitwarden and Vaultwarden integration - **[Password Management](docs/PASSWORD-MANAGEMENT.md)** - Security best practices ## 🛠️ Development ### Prerequisites -- Go 1.23 or later +- Go 1.24 or later - For Linux: `gcc`, `libgl1-mesa-dev`, `xorg-dev` - For GUI builds: Fyne dependencies @@ -224,7 +234,7 @@ mremotego/ │ ├── crypto/ # Encryption/decryption │ ├── gui/ # Fyne GUI components │ ├── launcher/ # Protocol launchers (SSH, RDP, etc.) -│ └── secrets/ # 1Password integration +│ └── secrets/ # Password manager providers (1Password, Bitwarden) ├── pkg/ │ └── models/ # Data models └── docs/ # Documentation @@ -299,7 +309,7 @@ This project is licensed under the MIT License - see the [LICENSE](LICENSE) file ### 📋 Planned Features #### Password Managers -- [ ] Bitwarden CLI integration (`bw://` references) +- [x] Bitwarden CLI integration (`bw://` references) - [ ] LastPass CLI integration (`lpass://` references) - [ ] HashiCorp Vault integration - [ ] Pass (password-store) integration for Linux diff --git a/cmd/mremotego-gui/main.go b/cmd/mremotego-gui/main.go index 6842fbf..23f5e0e 100644 --- a/cmd/mremotego-gui/main.go +++ b/cmd/mremotego-gui/main.go @@ -13,6 +13,7 @@ import ( "github.com/jaydenthorup/mremotego/cmd/mremotego/cmd" "github.com/jaydenthorup/mremotego/internal/config" "github.com/jaydenthorup/mremotego/internal/gui" + "github.com/jaydenthorup/mremotego/internal/secrets" ) func main() { @@ -37,6 +38,11 @@ func runGUI() { myApp := app.NewWithID("com.mremotego.app") myApp.Settings().SetTheme(&customTheme{}) + // Secret providers may run helper processes; make sure they are stopped + // however the application exits. + myApp.Lifecycle().SetOnStopped(secrets.Shutdown) + defer secrets.Shutdown() + // Set application icon (ignore errors - icon is optional) if icon := gui.GetAppIcon(); icon != nil { myApp.SetIcon(icon) diff --git a/cmd/mremotego/cmd/root.go b/cmd/mremotego/cmd/root.go index b699e3e..0a9fb41 100644 --- a/cmd/mremotego/cmd/root.go +++ b/cmd/mremotego/cmd/root.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/cobra" "github.com/jaydenthorup/mremotego/internal/config" + "github.com/jaydenthorup/mremotego/internal/secrets" ) var ( @@ -21,7 +22,13 @@ in a human-readable YAML format that works great with version control.`, // Execute runs the root command func Execute() { - if err := rootCmd.Execute(); err != nil { + err := rootCmd.Execute() + + // Stop helper processes started by secret providers. This cannot be a + // defer because the error path calls os.Exit. + secrets.Shutdown() + + if err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } diff --git a/docs/BITWARDEN-SETUP.md b/docs/BITWARDEN-SETUP.md new file mode 100644 index 0000000..c653f62 --- /dev/null +++ b/docs/BITWARDEN-SETUP.md @@ -0,0 +1,162 @@ +# Bitwarden Setup Guide + +MremoteGO can read connection passwords from Bitwarden, so that: + +- Passwords never appear in your config files +- Configs stay safe to commit and share via git +- Credentials can be rotated in one place for the whole team + +This works with bitwarden.com, self-hosted Bitwarden and Vaultwarden. + +## How it works + +Bitwarden has no library interface for other applications, so MremoteGO uses +the official Bitwarden CLI. On first use it starts `bw serve` as a hidden child +process bound to `127.0.0.1` on a random free port, reads what it needs over +that local API, and stops the process when MremoteGO exits. + +MremoteGO never asks for, sees or stores your master password. The child +process inherits the `BW_SESSION` variable, so you unlock the vault once in +your terminal and every lookup after that is authorised by that session. + +## Quick Setup + +### 1. Install the Bitwarden CLI + +```powershell +winget install Bitwarden.CLI +``` + +```bash +# macOS +brew install bitwarden-cli + +# Linux +npm install -g @bitwarden/cli +``` + +Verify with `bw --version`. + +### 2. Point the CLI at your server + +Only needed for self-hosted Bitwarden or Vaultwarden: + +```powershell +bw config server https://vault.example.com +``` + +### 3. Log in and unlock + +```powershell +bw login +$env:BW_SESSION = bw unlock --raw +bw status # should report "unlocked" +``` + +```bash +bw login +export BW_SESSION="$(bw unlock --raw)" +bw status +``` + +### 4. Start MremoteGO from that same terminal + +```powershell +.\mremotego.exe +``` + +The session key is inherited by MremoteGO and by the `bw serve` process it +starts. Launching MremoteGO from a desktop shortcut instead will leave the +vault locked, and it will show you these instructions. + +## Using Bitwarden References + +### In your config + +Instead of a plain text password, store a reference to the vault item: + +```yaml +connections: + - name: "Production Server" + protocol: ssh + host: prod.example.com + username: admin + password: bw://8f3c1d9a-4e2b-4c77-9f10-1a2b3c4d5e6f +``` + +Reference format: + +| Reference | Resolves to | +|-----------|-------------| +| `bw://` | the item's password | +| `bw:///password` | the item's password | +| `bw:///username` | the item's username | +| `bw:///totp` | the current TOTP code | +| `bw:///notes` | the item's notes | + +References use the item id rather than its name, so renaming an item in +Bitwarden does not break your config. + +### Finding an item id + +**In the GUI:** click **Bitwarden...** next to the password field when adding +or editing a connection. Search your vault, pick an item, and the reference is +filled in for you along with the username. + +**On the command line:** + +```powershell +bw list items --search "production" | ConvertFrom-Json | Select-Object id, name +``` + +### Creating items from MremoteGO + +1. Type the password into the password field as usual +2. Tick **Store password in Bitwarden** +3. Save + +MremoteGO creates a login item named after the connection, with the username +and a URI of `://`, then replaces the password in your config +with the new `bw://` reference. + +## Security notes + +- **`bw serve` has no authentication of its own.** Anything running as your + user on your machine could talk to it while it is up. MremoteGO limits the + exposure by binding it to loopback on a random port, starting it only when a + Bitwarden reference is actually used, and terminating it on exit. On Windows + the process is placed in a job object and on Linux it gets a parent death + signal, so it also dies if MremoteGO crashes. +- **`BW_SESSION` unlocks your whole vault.** Treat it like a password: do not + put it in a script that others can read, and close the terminal when done. +- **References are not encrypted at rest**, on purpose. They contain no secret + material, and leaving them readable is what keeps configs diffable in git. + This matches how `op://` references are handled. + +## Troubleshooting + +**"Bitwarden CLI (bw) is not installed or not in PATH"** +Install the CLI and make sure `bw --version` works in the same terminal you +start MremoteGO from. + +**"bitwarden vault is locked"** +The session key is missing or expired. Run `bw unlock --raw` again, set +`BW_SESSION`, and restart MremoteGO from that terminal. + +**"not logged in to bitwarden"** +Run `bw login`. For a self-hosted server, run `bw config server ` first. + +**An item you just created elsewhere is not in the picker** +The CLI serves items from a local cache. Click **Sync vault** in the picker, or +run `bw sync`. + +**RDP connects but prompts for the password** +RDP is deliberately allowed to continue when a reference cannot be resolved, so +you still get a login prompt instead of an error. Check the vault state as +above. + +## See also + +- [Password Management](PASSWORD-MANAGEMENT.md) - all password options +- [1Password Setup](1PASSWORD-SETUP.md) - the other supported password manager +- [Bitwarden CLI documentation](https://bitwarden.com/help/cli/) diff --git a/docs/ENCRYPTION.md b/docs/ENCRYPTION.md index 5b119ab..4ff690b 100644 --- a/docs/ENCRYPTION.md +++ b/docs/ENCRYPTION.md @@ -14,7 +14,7 @@ MremoteGO supports encrypting passwords at rest in the configuration file using - **AES-256-GCM**: Industry-standard authenticated encryption - **Unique Salt**: Each password gets a random 16-byte salt - **Random Nonce**: Each encryption uses a unique nonce -- **1Password Integration**: 1Password references (`op://...`) are NOT encrypted (no need) +- **Password Manager Integration**: references (`op://...`, `bw://...`) are NOT encrypted (no need) ## Using Encryption in the GUI diff --git a/docs/PASSWORD-MANAGEMENT.md b/docs/PASSWORD-MANAGEMENT.md index 9833c71..3b36fe1 100644 --- a/docs/PASSWORD-MANAGEMENT.md +++ b/docs/PASSWORD-MANAGEMENT.md @@ -7,6 +7,7 @@ MremoteGO provides flexible and secure password management with multiple options | Method | Security | Team Sharing | Auto-Login | Best For | |--------|----------|--------------|------------|----------| | 1Password | ✅ High | ✅ Yes | ✅ Yes | **Recommended** - Teams | +| Bitwarden | ✅ High | ✅ Yes | ✅ Yes | **Recommended** - Teams, self-hosting | | Plain Text | ⚠️ Low | ❌ No | ✅ Yes | Personal/testing only | | No Password | ✅ Manual | N/A | ❌ No | SSH keys, certificates | @@ -34,6 +35,35 @@ connections: - ✅ Automatic password rotation support - ✅ Audit logs +## Bitwarden Integration (Recommended) + +Store passwords in Bitwarden, bitwarden.com or self-hosted, and reference them +in config files. + +### Setup + +See [BITWARDEN-SETUP.md](BITWARDEN-SETUP.md) for complete setup instructions. + +### Usage + +```yaml +connections: + - name: "My Server" + password: bw://8f3c1d9a-4e2b-4c77-9f10-1a2b3c4d5e6f # Secure reference +``` + +References use the item id, so renaming an item does not break the config. A +field can be selected explicitly with `bw:///username`, `/totp` or +`/notes`. + +### Benefits +- ✅ Passwords never stored in config files +- ✅ Safe to commit configs to git +- ✅ Team password sharing +- ✅ Works with self-hosted Bitwarden and Vaultwarden +- ✅ Item picker built into the connection dialog +- ✅ Master password never handled by MremoteGO + ## Plain Text Passwords For personal use or testing environments: @@ -61,10 +91,10 @@ MremoteGO uses **Windows Credential Manager** for seamless RDP connections. ### How It Works -1. **First Connection**: Password (from 1Password or plain text) is stored in Windows Credential Manager - ``` - cmdkey /generic:TERMSRV/hostname /user:username /pass:password - ``` +1. **First Connection**: Password (from a password manager or plain text) is + stored in Windows Credential Manager as a generic `TERMSRV/hostname` + credential, written through the Credential Manager API rather than the + `cmdkey` command, so the password never appears on a command line. 2. **Subsequent Connections**: Windows automatically retrieves credentials ``` @@ -77,7 +107,7 @@ MremoteGO uses **Windows Credential Manager** for seamless RDP connections. - ✅ Native Windows integration - ✅ Persistent across sessions - ✅ User-specific security -- ✅ Works with 1Password references +- ✅ Works with password manager references - ✅ No passwords in temporary files ### Managing Stored Credentials @@ -103,17 +133,24 @@ cmdkey /list | Select-String "TERMSRV" | ForEach-Object { ## SSH Password Handling ### Windows (PuTTY) -MremoteGO uses PuTTY with password auto-fill: +MremoteGO uses PuTTY with password auto-fill. The password is written to a +private temporary file and passed with `-pwfile`, which PuTTY reads at start-up: ``` -putty.exe -ssh -P 22 -l username -pw password hostname +putty.exe -ssh -P 22 -l username -pwfile hostname ``` +The file is deleted as soon as PuTTY is running. PuTTY's `-pw` option is not +used because it puts the password on the command line, where any local process +can read it. ### Linux/Mac (Native SSH) Uses native ssh client: ``` ssh username@hostname -p 22 ``` -Note: Password is passed via environment or expected to use SSH keys. +When a password is set and `sshpass` is installed, the password is read from a +private temporary file into the `SSHPASS` environment variable and the file is +removed before `ssh` starts, so the password appears neither in the process +list nor on disk for longer than necessary. ### SSH Key Authentication (Recommended) For better security, use SSH keys instead of passwords: @@ -164,21 +201,21 @@ password: mypassword123 ## Best Practices ### For Teams -1. ✅ Use 1Password for all passwords -2. ✅ Store configs in git with `op://` references +1. ✅ Use 1Password or Bitwarden for all passwords +2. ✅ Store configs in git with `op://` or `bw://` references 3. ✅ Use shared vaults for team credentials 4. ✅ Enable biometric unlock 5. ✅ Regular access audits ### For Personal Use -1. ✅ Use 1Password if you have it +1. ✅ Use 1Password or Bitwarden if you have one 2. ⚠️ Plain text is acceptable for local dev 3. ✅ Use SSH keys where possible 4. ✅ Keep config file permissions restricted 5. ✅ Don't commit passwords to public repos ### For Production -1. ✅ Use 1Password or enterprise password manager +1. ✅ Use 1Password, Bitwarden or another enterprise password manager 2. ✅ Certificate-based authentication where possible 3. ✅ SSH keys instead of passwords 4. ✅ Regular credential rotation @@ -205,7 +242,7 @@ password: mypassword123 - Consider using SSH keys instead ### Password visible in process list -- This is normal for command-line tools -- Use 1Password to minimize exposure -- Passwords are only visible briefly during connection -- Windows Credential Manager used for RDP (not in process list) +- MremoteGO no longer passes passwords on a command line +- SSH uses PuTTY `-pwfile` on Windows and `SSHPASS` on Linux/Mac +- RDP uses the Windows Credential Manager API +- Known limitation: on Linux, `xfreerdp` is still invoked with `/p:` diff --git a/docs/README.md b/docs/README.md index 63c4010..7ebb45d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -8,6 +8,7 @@ ## Security & Passwords - **[1Password Setup](1PASSWORD-SETUP.md)** - Secure password management with 1Password +- **[Bitwarden Setup](BITWARDEN-SETUP.md)** - Secure password management with Bitwarden or Vaultwarden - **[Password Management](PASSWORD-MANAGEMENT.md)** - All password options and security details ## Main Documentation @@ -21,6 +22,7 @@ - [Add your first connection](QUICKSTART.md#first-connection) - [Set up 1Password](1PASSWORD-SETUP.md#quick-setup) +- [Set up Bitwarden](BITWARDEN-SETUP.md#quick-setup) - [Organize connections](GUI-GUIDE.md#connection-tree) - [Share configs with team](1PASSWORD-SETUP.md#using-1password-references) @@ -28,6 +30,7 @@ - [RDP password issues](PASSWORD-MANAGEMENT.md#rdp-auto-login) - [1Password not working](1PASSWORD-SETUP.md#troubleshooting) +- [Bitwarden not working](BITWARDEN-SETUP.md#troubleshooting) - [SSH connections failing](QUICKSTART.md#troubleshooting) - [GUI issues](GUI-GUIDE.md#troubleshooting) @@ -38,6 +41,7 @@ | `QUICKSTART.md` | 5-minute getting started guide | | `GUI-GUIDE.md` | Complete GUI interface documentation | | `1PASSWORD-SETUP.md` | Setting up 1Password integration | +| `BITWARDEN-SETUP.md` | Setting up Bitwarden integration | | `PASSWORD-MANAGEMENT.md` | Security and password options | ## Contributing diff --git a/go.mod b/go.mod index acfcd70..ae704eb 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,10 @@ go 1.24.0 require ( fyne.io/fyne/v2 v2.7.2 + github.com/danieljoos/wincred v1.2.3 github.com/spf13/cobra v1.8.0 golang.org/x/crypto v0.47.0 - golang.org/x/term v0.39.0 + golang.org/x/sys v0.40.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -42,6 +43,5 @@ require ( github.com/yuin/goldmark v1.7.8 // indirect golang.org/x/image v0.24.0 // indirect golang.org/x/net v0.48.0 // indirect - golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.33.0 // indirect ) diff --git a/go.sum b/go.sum index 5f4159b..d8488e6 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,8 @@ fyne.io/systray v1.12.0/go.mod h1:RVwqP9nYMo7h5zViCBHri2FgjXF7H2cub7MAq4NSoLs= github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg= github.com/BurntSushi/toml v1.5.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= +github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/felixge/fgprof v0.9.3 h1:VvyZxILNuCiUCSXtPtYmmtGvb65nqXh2QFWc0Wpf2/g= @@ -69,6 +71,8 @@ github.com/srwiley/oksvg v0.0.0-20221011165216-be6e8873101c h1:km8GpoQut05eY3GiY github.com/srwiley/oksvg v0.0.0-20221011165216-be6e8873101c/go.mod h1:cNQ3dwVJtS5Hmnjxy6AgTPd0Inb3pW05ftPSX7NZO7Q= github.com/srwiley/rasterx v0.0.0-20220730225603-2ab79fcdd4ef h1:Ch6Q+AZUxDBCVqdkI8FSpFyZDtCVBc2VmejdNrm5rRQ= github.com/srwiley/rasterx v0.0.0-20220730225603-2ab79fcdd4ef/go.mod h1:nXTWP6+gD5+LUJ8krVhhoeHjvHTutPxMYl5SvkcnJNE= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/yuin/goldmark v1.7.8 h1:iERMLn0/QJeHFhxSt3p6PeN9mGnvIKSpG9YYorDMnic= @@ -81,8 +85,6 @@ golang.org/x/net v0.48.0 h1:zyQRTTrjc33Lhh0fBgT/H3oZq9WuvRR5gPC70xpDiQU= golang.org/x/net v0.48.0/go.mod h1:+ndRgGjkh8FGtu1w1FGbEC31if4VrNVMuKTgcAAnQRY= golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/term v0.39.0 h1:RclSuaJf32jOqZz74CkPA9qFuVTX7vhLlpfj/IGWlqY= -golang.org/x/term v0.39.0/go.mod h1:yxzUCTP/U+FzoxfdKmLaA0RV1WgE0VY7hXBwKtY/4ww= golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/config/manager.go b/internal/config/manager.go index 90888da..da3b9ce 100644 --- a/internal/config/manager.go +++ b/internal/config/manager.go @@ -14,18 +14,18 @@ import ( // Manager handles configuration file operations type Manager struct { - configPath string - config *models.Config - onePasswordProvider *secrets.OnePasswordProvider - encryptionProvider *crypto.EncryptionProvider + configPath string + config *models.Config + secrets *secrets.Registry + encryptionProvider *crypto.EncryptionProvider } // NewManager creates a new configuration manager func NewManager(configPath string) *Manager { return &Manager{ - configPath: configPath, - onePasswordProvider: secrets.NewOnePasswordProvider(), - encryptionProvider: nil, // Will be set when master password is provided + configPath: configPath, + secrets: secrets.Default(), + encryptionProvider: nil, // Will be set when master password is provided } } @@ -382,14 +382,31 @@ func (m *Manager) UpdateConnection(name string, updates *models.Connection) erro return nil } -// IsOnePasswordReference checks if a password is a 1Password reference -func (m *Manager) IsOnePasswordReference(password string) bool { - return m.onePasswordProvider.IsReference(password) +// Secrets returns the secret provider registry used by this manager. +func (m *Manager) Secrets() *secrets.Registry { + return m.secrets } -// CreateOnePasswordItem creates a new 1Password item and returns the reference -func (m *Manager) CreateOnePasswordItem(vault, title, username, password string) (string, error) { - return m.onePasswordProvider.CreateItem(vault, title, username, password) +// IsSecretReference reports whether a password is a reference handled by one of +// the configured secret providers, for example "op://" or "bw://". +func (m *Manager) IsSecretReference(password string) bool { + return m.secrets.IsReference(password) +} + +// CreateSecretItem stores a login item in the provider identified by scheme and +// returns the reference that should be written to the configuration. +func (m *Manager) CreateSecretItem(scheme string, req secrets.CreateItemRequest) (string, error) { + provider, ok := m.secrets.ByScheme(scheme) + if !ok { + return "", fmt.Errorf("unknown secret provider: %s", scheme) + } + + creator, ok := provider.(secrets.ItemCreator) + if !ok { + return "", fmt.Errorf("%s cannot create items", provider.Name()) + } + + return creator.CreateItem(req) } // saveRecentFile saves the current config path as the most recently used file diff --git a/internal/crypto/encryption.go b/internal/crypto/encryption.go index b66ab3a..9cfcfa0 100644 --- a/internal/crypto/encryption.go +++ b/internal/crypto/encryption.go @@ -10,6 +10,7 @@ import ( "io" "strings" + "github.com/jaydenthorup/mremotego/internal/secrets" "golang.org/x/crypto/pbkdf2" ) @@ -172,7 +173,8 @@ func (p *EncryptionProvider) DecryptIfNeeded(value string) (string, error) { } // ShouldEncrypt checks if a value should be encrypted -// Returns false for empty strings, 1Password references, or already encrypted values +// Returns false for empty strings, secret manager references, or already +// encrypted values func (p *EncryptionProvider) ShouldEncrypt(value string) bool { if !p.enabled || value == "" { return false @@ -183,8 +185,9 @@ func (p *EncryptionProvider) ShouldEncrypt(value string) bool { return false } - // Don't encrypt 1Password references - if strings.HasPrefix(value, "op://") { + // Don't encrypt references to an external secret manager; they contain no + // secret material and stay readable so configs remain diffable in git. + if secrets.IsKnownReference(value) { return false } diff --git a/internal/crypto/encryption_test.go b/internal/crypto/encryption_test.go new file mode 100644 index 0000000..79ca3a3 --- /dev/null +++ b/internal/crypto/encryption_test.go @@ -0,0 +1,60 @@ +package crypto + +import "testing" + +func TestShouldEncrypt(t *testing.T) { + provider := NewEncryptionProvider("master") + + tests := map[string]bool{ + "hunter2": true, + "": false, + "op://Private/Server/password": false, + "bw://8f3c-item-id": false, + "enc:AAAA": false, + } + + for value, want := range tests { + if got := provider.ShouldEncrypt(value); got != want { + t.Errorf("ShouldEncrypt(%q) = %v, want %v", value, got, want) + } + } +} + +func TestShouldEncryptDisabledWithoutMasterPassword(t *testing.T) { + provider := NewEncryptionProvider("") + + if provider.ShouldEncrypt("hunter2") { + t.Error("ShouldEncrypt should be false when no master password is set") + } +} + +func TestEncryptDecryptRoundTrip(t *testing.T) { + provider := NewEncryptionProvider("master") + + encrypted, err := provider.Encrypt("hunter2") + if err != nil { + t.Fatalf("Encrypt returned error: %v", err) + } + if !provider.IsEncrypted(encrypted) { + t.Fatalf("Encrypt produced %q which is not recognised as encrypted", encrypted) + } + + decrypted, err := provider.Decrypt(encrypted) + if err != nil { + t.Fatalf("Decrypt returned error: %v", err) + } + if decrypted != "hunter2" { + t.Errorf("Decrypt = %q, want %q", decrypted, "hunter2") + } +} + +func TestDecryptWithWrongPasswordFails(t *testing.T) { + encrypted, err := NewEncryptionProvider("master").Encrypt("hunter2") + if err != nil { + t.Fatalf("Encrypt returned error: %v", err) + } + + if _, err := NewEncryptionProvider("other").Decrypt(encrypted); err == nil { + t.Error("expected decryption with the wrong master password to fail") + } +} diff --git a/internal/gui/dialogs.go b/internal/gui/dialogs.go index 93e3140..e8245ec 100644 --- a/internal/gui/dialogs.go +++ b/internal/gui/dialogs.go @@ -1,7 +1,6 @@ package gui import ( - "fmt" "strconv" "strings" "time" @@ -110,7 +109,7 @@ func (w *MainWindow) showAddConnectionDialog() { usernameEntry.SetPlaceHolder("username") passwordEntry := widget.NewEntry() - passwordEntry.SetPlaceHolder("password or op://vault/item/field") + passwordEntry.SetPlaceHolder(passwordPlaceholder) domainEntry := widget.NewEntry() domainEntry.SetPlaceHolder("domain (for RDP)") @@ -127,34 +126,24 @@ func (w *MainWindow) showAddConnectionDialog() { folderSelect := widget.NewSelect(folderNames, nil) folderSelect.SetSelected("(Root)") - // 1Password integration - storeTo1PasswordCheck := widget.NewCheck("Store password in 1Password", nil) - vaultSelect := widget.NewSelect([]string{"DevOps", "Private", "Employee"}, nil) - vaultSelect.SetSelected("DevOps") - vaultSelect.Hide() - - storeTo1PasswordCheck.OnChanged = func(checked bool) { - if checked { - vaultSelect.Show() - } else { - vaultSelect.Hide() - } + // Password manager integration + secretStore := w.newSecretStoreControls(usernameEntry, passwordEntry) + + items := []*widget.FormItem{ + {Text: "Name", Widget: nameEntry}, + {Text: "Protocol", Widget: protocolSelect}, + {Text: "Host", Widget: hostEntry}, + {Text: "Port", Widget: portEntry}, + {Text: "Username", Widget: usernameEntry}, + {Text: "Password", Widget: secretStore.passwordWidget(passwordEntry)}, + {Text: "Domain", Widget: domainEntry}, + {Text: "Description", Widget: descriptionEntry}, + {Text: "Folder", Widget: folderSelect}, } + items = append(items, secretStore.formItems()...) form := &widget.Form{ - Items: []*widget.FormItem{ - {Text: "Name", Widget: nameEntry}, - {Text: "Protocol", Widget: protocolSelect}, - {Text: "Host", Widget: hostEntry}, - {Text: "Port", Widget: portEntry}, - {Text: "Username", Widget: usernameEntry}, - {Text: "Password", Widget: passwordEntry}, - {Text: "Domain", Widget: domainEntry}, - {Text: "Description", Widget: descriptionEntry}, - {Text: "Folder", Widget: folderSelect}, - {Text: "", Widget: storeTo1PasswordCheck}, - {Text: "Vault", Widget: vaultSelect}, - }, + Items: items, OnSubmit: func() { conn := models.NewConnection(nameEntry.Text, models.Protocol(protocolSelect.Selected)) conn.Host = hostEntry.Text @@ -173,17 +162,17 @@ func (w *MainWindow) showAddConnectionDialog() { conn.Port = conn.Protocol.GetDefaultPort() } - // If user wants to store in 1Password, create the item - if storeTo1PasswordCheck.Checked && conn.Password != "" && !w.manager.IsOnePasswordReference(conn.Password) { - vault := vaultSelect.Selected - reference, err := w.manager.CreateOnePasswordItem(vault, conn.Name, conn.Username, conn.Password) - if err != nil { - dialog.ShowError(fmt.Errorf("Failed to create 1Password item: %w", err), w.window) - return - } - // Replace password with 1Password reference - conn.Password = reference - dialog.ShowInformation("Success", fmt.Sprintf("Password stored in 1Password vault '%s'", vault), w.window) + // If requested, push the password to a password manager and keep + // only the reference in the configuration. + value, providerName, err := secretStore.storeIfRequested( + conn.Name, conn.Username, conn.Password, string(conn.Protocol), conn.Host) + if err != nil { + dialog.ShowError(err, w.window) + return + } + conn.Password = value + if providerName != "" { + dialog.ShowInformation("Success", "Password stored in "+providerName, w.window) } // Add to selected folder or root @@ -303,48 +292,37 @@ func (w *MainWindow) showEditConnectionDialog(conn *models.Connection) { folderSelect := widget.NewSelect(folderNames, nil) folderSelect.SetSelected(currentFolder) - // 1Password integration for edit - storeTo1PasswordCheck := widget.NewCheck("Push password to 1Password", nil) - vaultSelect := widget.NewSelect([]string{"DevOps", "Private", "Employee"}, nil) - vaultSelect.SetSelected("DevOps") - vaultSelect.Hide() - - storeTo1PasswordCheck.OnChanged = func(checked bool) { - if checked { - vaultSelect.Show() - } else { - vaultSelect.Hide() - } + // Password manager integration + secretStore := w.newSecretStoreControls(usernameEntry, passwordEntry) + + items := []*widget.FormItem{ + {Text: "Name", Widget: nameEntry}, + {Text: "Protocol", Widget: protocolSelect}, + {Text: "Host", Widget: hostEntry}, + {Text: "Port", Widget: portEntry}, + {Text: "Username", Widget: usernameEntry}, + {Text: "Password", Widget: secretStore.passwordWidget(passwordEntry)}, + {Text: "Domain", Widget: domainEntry}, + {Text: "Description", Widget: descriptionEntry}, + {Text: "Folder", Widget: folderSelect}, } + items = append(items, secretStore.formItems()...) form := &widget.Form{ - Items: []*widget.FormItem{ - {Text: "Name", Widget: nameEntry}, - {Text: "Protocol", Widget: protocolSelect}, - {Text: "Host", Widget: hostEntry}, - {Text: "Port", Widget: portEntry}, - {Text: "Username", Widget: usernameEntry}, - {Text: "Password", Widget: passwordEntry}, - {Text: "Domain", Widget: domainEntry}, - {Text: "Description", Widget: descriptionEntry}, - {Text: "Folder", Widget: folderSelect}, - {Text: "", Widget: storeTo1PasswordCheck}, - {Text: "Vault", Widget: vaultSelect}, - }, + Items: items, OnSubmit: func() { - // If user wants to push password to 1Password - if storeTo1PasswordCheck.Checked && passwordEntry.Text != "" && !w.manager.IsOnePasswordReference(passwordEntry.Text) { - vault := vaultSelect.Selected - reference, err := w.manager.CreateOnePasswordItem(vault, nameEntry.Text, usernameEntry.Text, passwordEntry.Text) - if err != nil { - dialog.ShowError(fmt.Errorf("Failed to create 1Password item: %w", err), w.window) - return - } - // Replace password with 1Password reference - conn.Password = reference - dialog.ShowInformation("Success", fmt.Sprintf("Password stored in 1Password vault '%s'", vault), w.window) - } else { - conn.Password = passwordEntry.Text + // If requested, push the password to a password manager and keep + // only the reference in the configuration. + value, providerName, err := secretStore.storeIfRequested( + nameEntry.Text, usernameEntry.Text, passwordEntry.Text, + protocolSelect.Selected, hostEntry.Text) + if err != nil { + dialog.ShowError(err, w.window) + return + } + conn.Password = value + if providerName != "" { + dialog.ShowInformation("Success", "Password stored in "+providerName, w.window) } conn.Name = nameEntry.Text diff --git a/internal/gui/mainwindow.go b/internal/gui/mainwindow.go index 0b707cc..6b1dd63 100644 --- a/internal/gui/mainwindow.go +++ b/internal/gui/mainwindow.go @@ -12,6 +12,7 @@ import ( "fyne.io/fyne/v2/widget" "github.com/jaydenthorup/mremotego/internal/config" "github.com/jaydenthorup/mremotego/internal/launcher" + "github.com/jaydenthorup/mremotego/internal/secrets" "github.com/jaydenthorup/mremotego/pkg/models" ) @@ -557,50 +558,63 @@ func (w *MainWindow) showAbout() { func (w *MainWindow) Show() { w.window.Show() - // Check 1Password CLI authentication status after window is shown - w.check1PasswordAuth() + // Check secret provider authentication status after window is shown + w.checkSecretProviderAuth() } -// check1PasswordAuth checks if 1Password CLI needs authentication -func (w *MainWindow) check1PasswordAuth() { - // Check if there are any 1Password references in use - hasOpReferences := false +// checkSecretProviderAuth warns about secret providers that are referenced by a +// connection but cannot currently deliver secrets. +func (w *MainWindow) checkSecretProviderAuth() { + registry := w.launcher.Secrets() + + // Collect the providers actually referenced by the configuration; there is + // no point asking the user to sign in to a provider they do not use. + var used []secrets.Provider + seen := make(map[string]bool) var checkConnections func([]*models.Connection) checkConnections = func(conns []*models.Connection) { for _, conn := range conns { if conn.IsFolder() { checkConnections(conn.Children) - } else if strings.HasPrefix(conn.Password, "op://") { - hasOpReferences = true - return + continue + } + if provider, ok := registry.ProviderFor(conn.Password); ok && !seen[provider.Scheme()] { + seen[provider.Scheme()] = true + used = append(used, provider) } } } checkConnections(w.manager.GetConfig().Connections) - // Only check if there are actually 1Password references in use - if !hasOpReferences { + if len(used) == 0 { return } - // Check if 1Password CLI is authenticated - opProvider := w.launcher.GetOnePasswordProvider() - if opProvider.IsEnabled() && !opProvider.IsAuthenticated() { - // Show a helpful dialog - content := widget.NewLabel(opProvider.GetAuthenticationInstructions()) - content.Wrapping = fyne.TextWrapWord - - scrollContainer := container.NewVScroll(content) - scrollContainer.SetMinSize(fyne.NewSize(600, 400)) - - dialog.ShowCustom( - "1Password CLI Not Authenticated", - "OK", - scrollContainer, - w.window, - ) - } -} // Reload refreshes the window with the loaded config + // Checking authentication can start a helper process and talk to it, so it + // must not run on the UI goroutine. + go func() { + for _, provider := range used { + if !provider.IsEnabled() || provider.IsAuthenticated() { + continue + } + + title := provider.Name() + " Not Authenticated" + instructions := provider.GetAuthenticationInstructions() + + fyne.Do(func() { + content := widget.NewLabel(instructions) + content.Wrapping = fyne.TextWrapWord + + scrollContainer := container.NewVScroll(content) + scrollContainer.SetMinSize(fyne.NewSize(600, 400)) + + dialog.ShowCustom(title, "OK", scrollContainer, w.window) + }) + } + }() +} + +// Reload refreshes the window with the loaded config func (w *MainWindow) Reload() { w.buildConnectionMap() w.tree.Refresh() diff --git a/internal/gui/secretstore.go b/internal/gui/secretstore.go new file mode 100644 index 0000000..cc62281 --- /dev/null +++ b/internal/gui/secretstore.go @@ -0,0 +1,264 @@ +package gui + +import ( + "fmt" + "strings" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/container" + "fyne.io/fyne/v2/dialog" + "fyne.io/fyne/v2/layout" + "fyne.io/fyne/v2/widget" + "github.com/jaydenthorup/mremotego/internal/secrets" +) + +// passwordPlaceholder documents every accepted form of the password field. +const passwordPlaceholder = "password, op://vault/item/field or bw://item-id" + +// secretStoreControls are the shared widgets that let a connection dialog pick +// an existing secret or push a new one to a password manager. Both the add and +// the edit dialog use them, so the two stay in step. +type secretStoreControls struct { + window *MainWindow + + opCheck *widget.Check + bwCheck *widget.Check + vaultSelect *widget.Select + pickButton *widget.Button +} + +// newSecretStoreControls builds the controls for one dialog. usernameEntry and +// passwordEntry are filled in when the user picks an existing item. +func (w *MainWindow) newSecretStoreControls(usernameEntry, passwordEntry *widget.Entry) *secretStoreControls { + c := &secretStoreControls{window: w} + + c.vaultSelect = widget.NewSelect([]string{"DevOps", "Private", "Employee"}, nil) + c.vaultSelect.SetSelected("DevOps") + c.vaultSelect.Hide() + + c.opCheck = widget.NewCheck("Store password in 1Password", nil) + c.bwCheck = widget.NewCheck("Store password in Bitwarden", nil) + + // Storing the same password in two vaults would leave one of them stale, so + // the two options exclude each other. + c.opCheck.OnChanged = func(checked bool) { + if checked { + c.bwCheck.SetChecked(false) + c.vaultSelect.Show() + } else { + c.vaultSelect.Hide() + } + } + + bitwarden := w.bitwardenProvider() + + c.bwCheck.OnChanged = func(checked bool) { + if !checked { + return + } + c.opCheck.SetChecked(false) + // Starting the helper process takes a moment; do it now so that saving + // the form does not block. + if bitwarden != nil { + bitwarden.Warmup() + } + } + + c.pickButton = widget.NewButton("Bitwarden...", func() { + w.showBitwardenPicker(func(item secrets.BitwardenItem) { + passwordEntry.SetText("bw://" + item.ID) + if usernameEntry.Text == "" { + usernameEntry.SetText(item.Username) + } + }) + }) + + if bitwarden == nil || !bitwarden.IsEnabled() { + c.bwCheck.SetText("Store password in Bitwarden (bw CLI not installed)") + c.bwCheck.Disable() + c.pickButton.Hide() + } + + return c +} + +// passwordWidget wraps the password entry so the picker button sits next to it. +func (c *secretStoreControls) passwordWidget(passwordEntry *widget.Entry) fyne.CanvasObject { + return container.NewBorder(nil, nil, nil, c.pickButton, passwordEntry) +} + +// formItems returns the rows to append to a connection form. +func (c *secretStoreControls) formItems() []*widget.FormItem { + return []*widget.FormItem{ + {Text: "", Widget: c.opCheck}, + {Text: "", Widget: c.bwCheck}, + {Text: "Vault", Widget: c.vaultSelect}, + } +} + +// storeIfRequested pushes the password to the selected password manager and +// returns the value to write to the configuration: either a reference to the +// newly created item, or the password unchanged. providerName names the manager +// that was used, or is empty when nothing was stored. +func (c *secretStoreControls) storeIfRequested(title, username, password, protocol, host string) (value string, providerName string, err error) { + // Nothing to store, or the value already is a reference. + if password == "" || c.window.manager.IsSecretReference(password) { + return password, "", nil + } + + req := secrets.CreateItemRequest{ + Title: title, + Username: username, + Password: password, + } + + var scheme string + switch { + case c.opCheck.Checked: + scheme, providerName = secrets.SchemeOnePassword, "1Password" + req.Vault = c.vaultSelect.Selected + case c.bwCheck.Checked: + scheme, providerName = secrets.SchemeBitwarden, "Bitwarden" + // Recording the target makes the item useful in the Bitwarden clients + // as well, not just here. + if protocol != "" && host != "" { + req.URI = protocol + "://" + host + } + default: + return password, "", nil + } + + reference, err := c.window.manager.CreateSecretItem(scheme, req) + if err != nil { + return "", providerName, fmt.Errorf("failed to create %s item: %w", providerName, err) + } + + return reference, providerName, nil +} + +// bitwardenProvider returns the Bitwarden provider, or nil when it is not +// registered. +func (w *MainWindow) bitwardenProvider() *secrets.BitwardenProvider { + provider, ok := w.launcher.Secrets().ByScheme(secrets.SchemeBitwarden) + if !ok { + return nil + } + + bitwarden, _ := provider.(*secrets.BitwardenProvider) + return bitwarden +} + +// showBitwardenPicker lets the user choose a login item from their vault. The +// chosen item is reported as a reference, so the password itself never reaches +// the configuration file. +func (w *MainWindow) showBitwardenPicker(onPick func(secrets.BitwardenItem)) { + bitwarden := w.bitwardenProvider() + if bitwarden == nil || !bitwarden.IsEnabled() { + dialog.ShowError(fmt.Errorf("Bitwarden CLI (bw) is not installed or not in PATH"), w.window) + return + } + + var all []secrets.BitwardenItem + var filtered []secrets.BitwardenItem + var selected *secrets.BitwardenItem + + statusLabel := widget.NewLabel("Loading vault items...") + + list := widget.NewList( + func() int { return len(filtered) }, + func() fyne.CanvasObject { return widget.NewLabel("") }, + func(id widget.ListItemID, obj fyne.CanvasObject) { + if id >= len(filtered) { + return + } + item := filtered[id] + label := item.Name + if item.Username != "" { + label += " - " + item.Username + } + obj.(*widget.Label).SetText(label) + }, + ) + list.OnSelected = func(id widget.ListItemID) { + if id < len(filtered) { + item := filtered[id] + selected = &item + } + } + + searchEntry := widget.NewEntry() + searchEntry.SetPlaceHolder("Search by name, username or URL") + + // The whole vault is fetched once and filtered locally: asking the CLI on + // every keystroke would mean a round trip per character. + applyFilter := func(query string) { + query = strings.ToLower(strings.TrimSpace(query)) + + filtered = filtered[:0] + for _, item := range all { + if query == "" || + strings.Contains(strings.ToLower(item.Name), query) || + strings.Contains(strings.ToLower(item.Username), query) || + strings.Contains(strings.ToLower(item.URI), query) { + filtered = append(filtered, item) + } + } + + selected = nil + list.UnselectAll() + list.Refresh() + statusLabel.SetText(fmt.Sprintf("%d of %d items", len(filtered), len(all))) + } + + searchEntry.OnChanged = applyFilter + + // load fetches the vault in the background; every UI update goes through + // fyne.Do because Fyne widgets may only be touched from the UI goroutine. + load := func(sync bool) { + statusLabel.SetText("Loading vault items...") + + go func() { + if sync { + if err := bitwarden.Sync(); err != nil { + fyne.Do(func() { + statusLabel.SetText("Sync failed: " + err.Error()) + }) + return + } + } + + items, err := bitwarden.ListLoginItems("") + fyne.Do(func() { + if err != nil { + statusLabel.SetText("Failed to load items") + dialog.ShowError(err, w.window) + return + } + + all = items + applyFilter(searchEntry.Text) + }) + }() + } + + syncButton := widget.NewButton("Sync vault", func() { load(true) }) + + content := container.NewBorder( + searchEntry, + container.NewHBox(statusLabel, layout.NewSpacer(), syncButton), + nil, + nil, + list, + ) + + picker := dialog.NewCustomConfirm("Pick from Bitwarden", "Use", "Cancel", content, + func(confirmed bool) { + if confirmed && selected != nil { + onPick(*selected) + } + }, w.window) + picker.Resize(fyne.NewSize(600, 500)) + picker.Show() + + load(false) +} diff --git a/internal/launcher/credential_compat_windows_test.go b/internal/launcher/credential_compat_windows_test.go new file mode 100644 index 0000000..b002154 --- /dev/null +++ b/internal/launcher/credential_compat_windows_test.go @@ -0,0 +1,88 @@ +//go:build windows + +package launcher + +import ( + "bytes" + "os/exec" + "testing" + + "github.com/danieljoos/wincred" +) + +// TestCredentialMatchesCmdkey verifies that the credential written through the +// Credential Manager API is byte for byte what "cmdkey /generic" would have +// written. mstsc reads the blob as UTF-16LE, so storing plain UTF-8 would +// produce a credential that looks valid but silently fails to log in. +func TestCredentialMatchesCmdkey(t *testing.T) { + if _, err := exec.LookPath("cmdkey"); err != nil { + t.Skip("cmdkey not available") + } + + const ( + user = "tester" + password = "passw0rd123" + refTgt = "TERMSRV/mremotego-selftest-cmdkey" + apiTgt = "TERMSRV/mremotego-selftest-api" + ) + + // Start from a clean slate: a leftover credential from an earlier run would + // make the comparison meaningless. + _ = deleteGenericCredential(refTgt) + _ = deleteGenericCredential(apiTgt) + + out, err := exec.Command("cmdkey", "/generic:"+refTgt, "/user:"+user, "/pass:"+password).CombinedOutput() + if err != nil { + t.Skipf("cmdkey refused to store the reference credential: %v (%s)", err, out) + } + defer deleteGenericCredential(refTgt) + + reference, err := wincred.GetGenericCredential(refTgt) + if err != nil { + t.Skipf("cmdkey credential could not be read back: %v", err) + } + + if err := writeGenericCredential(apiTgt, user, password); err != nil { + t.Fatalf("writeGenericCredential returned error: %v", err) + } + defer deleteGenericCredential(apiTgt) + + written, err := wincred.GetGenericCredential(apiTgt) + if err != nil { + t.Fatalf("failed to read back the credential we wrote: %v", err) + } + + if !bytes.Equal(reference.CredentialBlob, written.CredentialBlob) { + t.Errorf("blob mismatch:\n cmdkey % x\n api % x", + reference.CredentialBlob, written.CredentialBlob) + } + if written.UserName != reference.UserName { + t.Errorf("user name = %q, want %q", written.UserName, reference.UserName) + } + if written.Persist != reference.Persist { + t.Errorf("persist = %v, want %v", written.Persist, reference.Persist) + } +} + +// TestUTF16LE pins the encoding independently of whether cmdkey is available. +func TestUTF16LE(t *testing.T) { + got := utf16LE("ab") + want := []byte{'a', 0x00, 'b', 0x00} + + if !bytes.Equal(got, want) { + t.Errorf("utf16LE(\"ab\") = % x, want % x", got, want) + } + + // No byte order mark and no terminating NUL, matching what cmdkey stores. + if len(utf16LE("")) != 0 { + t.Errorf("utf16LE(\"\") = % x, want empty", utf16LE("")) + } +} + +// TestDeleteMissingCredentialIsNotAnError guards the cleanup path, which runs +// for every RDP connection that is removed. +func TestDeleteMissingCredentialIsNotAnError(t *testing.T) { + if err := deleteGenericCredential("TERMSRV/mremotego-selftest-absent"); err != nil { + t.Errorf("deleting a missing credential returned: %v", err) + } +} diff --git a/internal/launcher/credential_other.go b/internal/launcher/credential_other.go new file mode 100644 index 0000000..3b1b12b --- /dev/null +++ b/internal/launcher/credential_other.go @@ -0,0 +1,10 @@ +//go:build !windows + +package launcher + +// The Windows Credential Manager has no counterpart on other platforms; RDP +// clients there take credentials by other means. + +func writeGenericCredential(target, username, password string) error { return nil } + +func deleteGenericCredential(target string) error { return nil } diff --git a/internal/launcher/credential_windows.go b/internal/launcher/credential_windows.go new file mode 100644 index 0000000..d430193 --- /dev/null +++ b/internal/launcher/credential_windows.go @@ -0,0 +1,56 @@ +//go:build windows + +package launcher + +import ( + "encoding/binary" + "errors" + "unicode/utf16" + + "github.com/danieljoos/wincred" + "golang.org/x/sys/windows" +) + +// writeGenericCredential stores a credential in the Windows Credential Manager. +// +// This replaces shelling out to cmdkey, which takes the password on its command +// line where every local process can read it. +// +// The blob must be UTF-16LE: that is what cmdkey writes and what mstsc expects. +// Storing plain UTF-8 makes the credential look valid while silently failing to +// log in. Persistence is enterprise, matching "cmdkey /generic". +func writeGenericCredential(target, username, password string) error { + cred := wincred.NewGenericCredential(target) + cred.UserName = username + cred.CredentialBlob = utf16LE(password) + cred.Persist = wincred.PersistEnterprise + + return cred.Write() +} + +// deleteGenericCredential removes a credential. A credential that is already +// gone is not an error. +func deleteGenericCredential(target string) error { + cred, err := wincred.GetGenericCredential(target) + if err != nil { + if errors.Is(err, windows.ERROR_NOT_FOUND) { + return nil + } + return err + } + + return cred.Delete() +} + +// utf16LE encodes a string as UTF-16 little endian without a byte order mark +// and without a terminating NUL, which is how Windows stores credential blobs. +func utf16LE(s string) []byte { + encoded := utf16.Encode([]rune(s)) + + out := make([]byte, len(encoded)*2) + for i, r := range encoded { + binary.LittleEndian.PutUint16(out[i*2:], r) + } + + return out +} diff --git a/internal/launcher/launcher.go b/internal/launcher/launcher.go index 3b7212e..2a5b6c2 100644 --- a/internal/launcher/launcher.go +++ b/internal/launcher/launcher.go @@ -8,6 +8,7 @@ import ( "runtime" "strconv" "strings" + "time" "github.com/jaydenthorup/mremotego/internal/secrets" "github.com/jaydenthorup/mremotego/pkg/models" @@ -15,19 +16,23 @@ import ( // Launcher handles launching connections type Launcher struct { - onePasswordProvider *secrets.OnePasswordProvider + secrets *secrets.Registry } // NewLauncher creates a new launcher func NewLauncher() *Launcher { + // Remove password files left behind by a run that was killed. + cleanupStalePasswordFiles() + return &Launcher{ - onePasswordProvider: secrets.NewOnePasswordProvider(), + secrets: secrets.Default(), } } -// GetOnePasswordProvider returns the 1Password provider for checking authentication status -func (l *Launcher) GetOnePasswordProvider() *secrets.OnePasswordProvider { - return l.onePasswordProvider +// Secrets returns the secret provider registry, for example to check +// authentication status before launching a connection. +func (l *Launcher) Secrets() *secrets.Registry { + return l.secrets } // Launch launches a connection based on its protocol @@ -36,18 +41,19 @@ func (l *Launcher) Launch(conn *models.Connection) error { return fmt.Errorf("cannot launch a folder") } - // Resolve 1Password reference if needed (make a copy to avoid modifying the original) + // Resolve a secret manager reference if needed (make a copy to avoid + // modifying the original) resolvedConn := *conn - if l.onePasswordProvider.IsReference(conn.Password) { - resolved, err := l.onePasswordProvider.ResolveSecret(conn.Password) + if provider, ok := l.secrets.ProviderFor(conn.Password); ok { + resolved, err := provider.ResolveSecret(conn.Password) if err != nil { // For RDP, we can continue without a password (will prompt) // For other protocols that require a password, return the error if conn.Protocol != models.ProtocolRDP { - return fmt.Errorf("failed to resolve password from 1Password: %w", err) + return fmt.Errorf("failed to resolve password from %s: %w", provider.Name(), err) } - // RDP: Clear the password so it doesn't try to use the op:// reference - fmt.Printf("Warning: Failed to resolve password from 1Password: %v (RDP will prompt for credentials)\n", err) + // RDP: Clear the password so it doesn't try to use the reference + fmt.Printf("Warning: Failed to resolve password from %s: %v (RDP will prompt for credentials)\n", provider.Name(), err) resolvedConn.Password = "" } else { resolvedConn.Password = resolved @@ -92,9 +98,17 @@ func (l *Launcher) launchSSH(conn *models.Connection) error { args = append(args, "-l", conn.Username) } - // Add password if provided (for auto-login) + // Add password if provided (for auto-login). PuTTY documents -pw as + // insecure because the command line is visible to every local process, + // so the password is handed over in a private file instead. + cleanup := func() {} if conn.Password != "" { - args = append(args, "-pw", conn.Password) + passwordFile, remove, err := writePasswordFile(conn.Password) + if err != nil { + return fmt.Errorf("failed to prepare password for PuTTY: %w", err) + } + cleanup = remove + args = append(args, "-pwfile", passwordFile) } // Add extra args if provided @@ -109,9 +123,16 @@ func (l *Launcher) launchSSH(conn *models.Connection) error { cmd := exec.Command("putty.exe", args...) // Don't hide putty - it's a GUI application we want to see if err := cmd.Start(); err != nil { + cleanup() // Fall back to ssh command return l.launchSSHFallback(conn) } + + // PuTTY reads the password file while parsing its arguments, so it can + // be removed as soon as the process is up. Waiting for the process also + // covers the case where it fails immediately. + go removeAfterStart(cmd, cleanup) + return nil } @@ -149,64 +170,101 @@ func (l *Launcher) launchSSHFallback(conn *models.Connection) error { } var cmd *exec.Cmd + cleanup := func() {} // If password is provided, try to use sshpass (if available) if conn.Password != "" { // Check if sshpass is available if _, err := exec.LookPath("sshpass"); err == nil { - // Use sshpass to provide password - sshpassArgs := []string{"-p", conn.Password, "ssh"} - sshpassArgs = append(sshpassArgs, args...) + passwordFile, remove, err := writePasswordFile(conn.Password) + if err != nil { + return fmt.Errorf("failed to prepare password for sshpass: %w", err) + } + cleanup = remove - // Launch in a terminal emulator - cmd = l.launchInTerminal("sshpass", sshpassArgs...) + // sshpass -p would expose the password in the process list. The + // password cannot be passed through the environment either, + // because terminal emulators do not forward it, so the generated + // shell snippet reads the file and deletes it before ssh starts. + cmd = l.launchInTerminal(sshpassCommand(passwordFile, args), nil, conn.Host) } else { // sshpass not available, just use ssh (will prompt for password) - cmd = l.launchInTerminal("ssh", args...) + cmd = l.launchInTerminal(shellCommand("ssh", args...), append([]string{"ssh"}, args...), conn.Host) } } else { - cmd = l.launchInTerminal("ssh", args...) + cmd = l.launchInTerminal(shellCommand("ssh", args...), append([]string{"ssh"}, args...), conn.Host) } if cmd == nil { + cleanup() return fmt.Errorf("failed to create terminal command") } - return cmd.Start() + if err := cmd.Start(); err != nil { + cleanup() + return err + } + + // The shell snippet deletes the file itself; this only covers a terminal + // that never ran it. + time.AfterFunc(staleCleanupDelay, cleanup) + + return nil +} + +// removeAfterStart runs cleanup once the process has exited or after a short +// grace period, whichever comes first. +func removeAfterStart(cmd *exec.Cmd, cleanup func()) { + done := make(chan struct{}) + go func() { + _ = cmd.Wait() + close(done) + }() + + select { + case <-done: + case <-time.After(passwordFileGrace): + } + + cleanup() } -// launchInTerminal launches a command in a terminal emulator -func (l *Launcher) launchInTerminal(command string, args ...string) *exec.Cmd { - // Build the command with proper shell quoting - cmdParts := []string{command} - cmdParts = append(cmdParts, args...) +// shellCommand quotes a command and its arguments for execution by a shell. +func shellCommand(command string, args ...string) string { + parts := append([]string{command}, args...) - // Join command parts, using shell quoting for safety - var quotedParts []string - for _, part := range cmdParts { - // Use single quotes and escape any single quotes in the string - quotedParts = append(quotedParts, fmt.Sprintf("'%s'", strings.ReplaceAll(part, "'", "'\\''"))) + quoted := make([]string, 0, len(parts)) + for _, part := range parts { + quoted = append(quoted, "'"+strings.ReplaceAll(part, "'", `'\''`)+"'") } - fullCmd := strings.Join(quotedParts, " ") + return strings.Join(quoted, " ") +} + +// sshpassCommand builds a shell snippet that reads the password from a file +// into the environment and removes the file before ssh is executed, so the +// secret appears neither in the process list nor on disk for longer than +// necessary. +func sshpassCommand(passwordFile string, sshArgs []string) string { + quotedFile := "'" + strings.ReplaceAll(passwordFile, "'", `'\''`) + "'" + + return fmt.Sprintf( + "SSHPASS=\"$(head -n 1 %s)\"; rm -f %s; export SSHPASS; %s", + quotedFile, + quotedFile, + shellCommand("sshpass", append([]string{"-e", "ssh"}, sshArgs...)...), + ) +} + +// launchInTerminal launches a shell snippet in a terminal emulator. +// +// fullCmd is the snippet to run. argv is the equivalent argument vector and is +// only used on systems where no terminal emulator is available. hostname names +// the host being connected to and is used for the host key hint on Linux. +func (l *Launcher) launchInTerminal(fullCmd string, argv []string, hostname string) *exec.Cmd { // For SSH commands on Linux, wrap with error detection for host key changes var wrappedCmd string - if runtime.GOOS == "linux" && (command == "ssh" || command == "sshpass") { - // Extract hostname from args for ssh-keygen -R - hostname := "" - for i, arg := range args { - if !strings.HasPrefix(arg, "-") && i > 0 { - // This is likely the hostname or user@hostname - parts := strings.Split(arg, "@") - if len(parts) > 1 { - hostname = parts[1] - } else { - hostname = arg - } - break - } - } - + if runtime.GOOS == "linux" && hostname != "" { // Create wrapper that detects host key mismatch and shows helpful message // Use a temp file to capture stderr for error detection wrappedCmd = fmt.Sprintf(` @@ -280,7 +338,19 @@ func (l *Launcher) launchInTerminal(command string, args ...string) *exec.Cmd { // Fallback: try to run without terminal (will need stdin/stdout) fmt.Println("Fallback: Running without terminal") - cmd := exec.Command(command, args...) + + var cmd *exec.Cmd + if runtime.GOOS == "windows" { + // Windows has no POSIX shell to interpret the snippet, and sshpass does + // not exist there, so the argument vector is always safe to run. + if len(argv) == 0 { + return nil + } + cmd = exec.Command(argv[0], argv[1:]...) + } else { + cmd = exec.Command("sh", "-c", wrappedCmd) + } + cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -601,14 +671,11 @@ 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)) + // Write the credential through the Credential Manager API. Using cmdkey + // here would put the password on a command line, where any local process + // could read it. + if err := writeGenericCredential("TERMSRV/"+target, username, conn.Password); err != nil { + return fmt.Errorf("failed to store credential for %s: %w", target, err) } return nil @@ -630,17 +697,9 @@ 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)) - } + // Delete through the Credential Manager API; a missing credential is fine. + if err := deleteGenericCredential("TERMSRV/" + target); err != nil { + return fmt.Errorf("failed to remove credential for %s: %w", target, err) } return nil diff --git a/internal/launcher/password_file.go b/internal/launcher/password_file.go new file mode 100644 index 0000000..e24ccf4 --- /dev/null +++ b/internal/launcher/password_file.go @@ -0,0 +1,104 @@ +package launcher + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "sync" + "time" +) + +const ( + // passwordFilePrefix marks the temporary files holding a password. + passwordFilePrefix = "pw-" + // passwordFileMaxAge is how long a leftover file may survive before it is + // considered stale and removed at start-up. + passwordFileMaxAge = 2 * time.Minute + // passwordFileGrace bounds how long the file may exist after the helper + // program was started, in case that program never exits. + passwordFileGrace = 5 * time.Second + // staleCleanupDelay is the safety net for terminal emulators that never ran + // the generated snippet, which would otherwise leave the file behind. + staleCleanupDelay = 60 * time.Second +) + +// tempDir is the directory used for the short lived files handed to helper +// programs. It is the same directory the generated .rdp files already use. +func tempDir() string { + return filepath.Join(os.TempDir(), "mremotego") +} + +// writePasswordFile stores a password in a private temporary file and returns +// its path together with a cleanup function. +// +// Passing a password through a file rather than the command line keeps it out +// of the process list, where any local user could read it. +// +// os.CreateTemp creates the file with mode 0600, and on Windows the per-user +// temporary directory is already restricted to the user, SYSTEM and +// administrators - the same protection the generated .rdp files rely on. The +// window in which the file exists is short: the helper program reads it while +// starting up and the caller removes it right after. +func writePasswordFile(password string) (path string, cleanup func(), err error) { + dir := tempDir() + if err := os.MkdirAll(dir, 0700); err != nil { + return "", func() {}, fmt.Errorf("failed to create temp directory: %w", err) + } + + file, err := os.CreateTemp(dir, passwordFilePrefix+"*.txt") + if err != nil { + return "", func() {}, fmt.Errorf("failed to create password file: %w", err) + } + path = file.Name() + + // PuTTY and the sshpass wrapper both read the first line of the file. + if _, err := file.WriteString(password + "\n"); err != nil { + file.Close() + os.Remove(path) + return "", func() {}, fmt.Errorf("failed to write password file: %w", err) + } + + if err := file.Close(); err != nil { + os.Remove(path) + return "", func() {}, fmt.Errorf("failed to close password file: %w", err) + } + + var once sync.Once + cleanup = func() { + once.Do(func() { + // Overwrite before unlinking so the content does not linger in + // slack space if the delete fails. + if f, err := os.OpenFile(path, os.O_WRONLY, 0600); err == nil { + _, _ = f.Write(make([]byte, len(password)+1)) + f.Close() + } + os.Remove(path) + }) + } + + return path, cleanup, nil +} + +// cleanupStalePasswordFiles removes password files left behind by an earlier +// run that was killed before it could clean up. +func cleanupStalePasswordFiles() { + entries, err := os.ReadDir(tempDir()) + if err != nil { + return + } + + cutoff := time.Now().Add(-passwordFileMaxAge) + for _, entry := range entries { + if entry.IsDir() || !strings.HasPrefix(entry.Name(), passwordFilePrefix) { + continue + } + + info, err := entry.Info() + if err != nil || info.ModTime().After(cutoff) { + continue + } + + os.Remove(filepath.Join(tempDir(), entry.Name())) + } +} diff --git a/internal/launcher/password_file_test.go b/internal/launcher/password_file_test.go new file mode 100644 index 0000000..61c5a0a --- /dev/null +++ b/internal/launcher/password_file_test.go @@ -0,0 +1,138 @@ +package launcher + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestWritePasswordFile(t *testing.T) { + path, cleanup, err := writePasswordFile("hunter2") + if err != nil { + t.Fatalf("writePasswordFile returned error: %v", err) + } + defer cleanup() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read password file: %v", err) + } + + // PuTTY and the sshpass wrapper read the first line of the file. + if string(data) != "hunter2\n" { + t.Errorf("password file contains %q, want %q", string(data), "hunter2\n") + } +} + +func TestWritePasswordFileCleanupRemovesFile(t *testing.T) { + path, cleanup, err := writePasswordFile("hunter2") + if err != nil { + t.Fatalf("writePasswordFile returned error: %v", err) + } + + cleanup() + + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("password file still exists after cleanup (stat error: %v)", err) + } +} + +func TestWritePasswordFileCleanupIsIdempotent(t *testing.T) { + _, cleanup, err := writePasswordFile("hunter2") + if err != nil { + t.Fatalf("writePasswordFile returned error: %v", err) + } + + cleanup() + cleanup() // must not panic or fail +} + +func TestWritePasswordFileIsUnique(t *testing.T) { + first, cleanupFirst, err := writePasswordFile("a") + if err != nil { + t.Fatalf("writePasswordFile returned error: %v", err) + } + defer cleanupFirst() + + second, cleanupSecond, err := writePasswordFile("b") + if err != nil { + t.Fatalf("writePasswordFile returned error: %v", err) + } + defer cleanupSecond() + + if first == second { + t.Error("two password files got the same path") + } +} + +func TestCleanupStalePasswordFiles(t *testing.T) { + stale := filepath.Join(tempDir(), passwordFilePrefix+"stale-test.txt") + if err := os.MkdirAll(tempDir(), 0700); err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + if err := os.WriteFile(stale, []byte("old\n"), 0600); err != nil { + t.Fatalf("failed to create stale file: %v", err) + } + + old := time.Now().Add(-2 * passwordFileMaxAge) + if err := os.Chtimes(stale, old, old); err != nil { + t.Fatalf("failed to age the stale file: %v", err) + } + + fresh, cleanupFresh, err := writePasswordFile("keep me") + if err != nil { + t.Fatalf("writePasswordFile returned error: %v", err) + } + defer cleanupFresh() + + cleanupStalePasswordFiles() + + if _, err := os.Stat(stale); !os.IsNotExist(err) { + os.Remove(stale) + t.Errorf("stale password file survived cleanup (stat error: %v)", err) + } + if _, err := os.Stat(fresh); err != nil { + t.Errorf("recent password file was removed: %v", err) + } +} + +func TestShellCommandQuotesArguments(t *testing.T) { + got := shellCommand("ssh", "-p", "22", "user@host") + want := "'ssh' '-p' '22' 'user@host'" + + if got != want { + t.Errorf("shellCommand = %q, want %q", got, want) + } +} + +func TestShellCommandEscapesSingleQuotes(t *testing.T) { + got := shellCommand("ssh", "o'brien@host") + + if strings.Contains(got, "o'brien@host") { + t.Errorf("shellCommand left an unescaped quote: %q", got) + } + if !strings.Contains(got, `'\''`) { + t.Errorf("shellCommand = %q, want the single quote shell-escaped", got) + } +} + +// The password must reach sshpass through the environment, never through argv, +// where every local user could read it. +func TestSshpassCommandKeepsPasswordOutOfArgv(t *testing.T) { + got := sshpassCommand("/tmp/mremotego/pw-1.txt", []string{"-p", "22", "user@host"}) + + if !strings.Contains(got, "SSHPASS=") { + t.Errorf("snippet does not set SSHPASS: %q", got) + } + if !strings.Contains(got, "'sshpass' '-e' 'ssh'") { + t.Errorf("snippet does not run sshpass -e ssh: %q", got) + } + if strings.Contains(got, "-p '") && strings.Contains(got, "sshpass' '-p") { + t.Errorf("snippet passes the password on the command line: %q", got) + } + if !strings.Contains(got, "rm -f") { + t.Errorf("snippet does not remove the password file: %q", got) + } +} diff --git a/internal/secrets/bitwarden.go b/internal/secrets/bitwarden.go new file mode 100644 index 0000000..51a9f15 --- /dev/null +++ b/internal/secrets/bitwarden.go @@ -0,0 +1,325 @@ +package secrets + +import ( + "context" + "fmt" + "os/exec" + "strings" + "sync" +) + +// BitwardenProvider resolves "bw://" references through the Bitwarden CLI. +// +// The CLI has no library interface, so the provider runs "bw serve" as a child +// process and talks to its REST API over loopback. The server is started lazily +// on first use, bound to 127.0.0.1 on a random free port, and terminated when +// Close is called. +// +// The child inherits BW_SESSION from the environment. Unlocking therefore +// happens once, in the shell the application was started from, and no master +// password is ever handled by MremoteGO itself. +type BitwardenProvider struct { + bwPath string + enabled bool + + mu sync.Mutex + server *bwServer + client *bwClient + + // newClient creates the transport. Tests replace it to talk to an httptest + // server instead of spawning a real CLI. + newClient func() (*bwClient, *bwServer, error) +} + +var ( + _ Provider = (*BitwardenProvider)(nil) + _ ItemCreator = (*BitwardenProvider)(nil) +) + +// BitwardenItem is a login item as shown in the item picker. +type BitwardenItem struct { + ID string + Name string + Username string + URI string +} + +// NewBitwardenProvider creates a provider backed by the Bitwarden CLI. It only +// looks the binary up; no process is started until a secret is needed. +func NewBitwardenProvider() *BitwardenProvider { + path, err := exec.LookPath("bw") + provider := &BitwardenProvider{ + bwPath: path, + enabled: err == nil, + } + provider.newClient = provider.startServer + return provider +} + +// newBitwardenProviderWithClient builds a provider around an existing client. +// It exists for tests, which must not spawn the real CLI. +func newBitwardenProviderWithClient(client *bwClient) *BitwardenProvider { + provider := &BitwardenProvider{ + bwPath: "bw", + enabled: true, + } + provider.newClient = func() (*bwClient, *bwServer, error) { + return client, nil, nil + } + return provider +} + +// Name returns the human readable provider name. +func (p *BitwardenProvider) Name() string { return "Bitwarden" } + +// Scheme returns the reference scheme handled by this provider. +func (p *BitwardenProvider) Scheme() string { return SchemeBitwarden } + +// IsEnabled reports whether the Bitwarden CLI is installed. +func (p *BitwardenProvider) IsEnabled() bool { return p.enabled } + +// IsReference checks whether a value is a Bitwarden reference. +func (p *BitwardenProvider) IsReference(value string) bool { + return strings.HasPrefix(value, bitwardenPrefix) +} + +// Status returns the vault state: unlocked, locked or unauthenticated. +func (p *BitwardenProvider) Status() (string, error) { + client, err := p.ensureClient() + if err != nil { + return "", err + } + + status, err := client.status(context.Background()) + if err != nil { + return "", err + } + return status.Status, nil +} + +// IsAuthenticated reports whether the vault is unlocked and secrets can be +// read. Note that this may start the helper process, so it should not be called +// on a UI thread. +func (p *BitwardenProvider) IsAuthenticated() bool { + status, err := p.Status() + return err == nil && status == bwStatusUnlocked +} + +// GetAuthenticationInstructions explains how to make the vault available. +func (p *BitwardenProvider) GetAuthenticationInstructions() string { + return `Bitwarden CLI is installed but the vault is not available. + +MremoteGO reads secrets through the Bitwarden CLI, which needs an unlocked +vault. Unlock it once in a terminal and start MremoteGO from that same +terminal, so it inherits the session key. + +PowerShell: + bw config server https://your-server (self-hosted or Vaultwarden only) + bw login (once) + $env:BW_SESSION = bw unlock --raw + .\mremotego.exe + +bash / zsh: + bw config server https://your-server (self-hosted or Vaultwarden only) + bw login (once) + export BW_SESSION="$(bw unlock --raw)" + ./mremotego + +Run "bw status" to confirm the vault reports "unlocked". + +MremoteGO never asks for or stores your master password. It starts +"bw serve" on 127.0.0.1 with a random port and stops it on exit.` +} + +// ResolveSecret reads the field a reference points at. +func (p *BitwardenProvider) ResolveSecret(reference string) (string, error) { + id, field, err := parseBitwardenReference(reference) + if err != nil { + return "", err + } + + client, err := p.ensureClient() + if err != nil { + return "", err + } + + value, err := client.getField(context.Background(), field, id) + if err != nil { + return "", err + } + + if value == "" { + return "", fmt.Errorf("item %s has no %s", id, field) + } + + return value, nil +} + +// CreateItem stores a new login item and returns its "bw://" reference. +func (p *BitwardenProvider) CreateItem(req CreateItemRequest) (string, error) { + if req.Title == "" { + return "", fmt.Errorf("a title is required") + } + + client, err := p.ensureClient() + if err != nil { + return "", err + } + + login := &bwLogin{ + Username: req.Username, + Password: req.Password, + } + if req.URI != "" { + login.URIs = []bwURI{{URI: req.URI}} + } + + created, err := client.createItem(context.Background(), bwItem{ + Type: bwTypeLogin, + Name: req.Title, + Login: login, + }) + if err != nil { + return "", err + } + + return bitwardenReference(created.ID), nil +} + +// ListLoginItems returns the login items of the vault. Cards, identities and +// secure notes are skipped because they carry no connection credentials. +func (p *BitwardenProvider) ListLoginItems(search string) ([]BitwardenItem, error) { + client, err := p.ensureClient() + if err != nil { + return nil, err + } + + items, err := client.listItems(context.Background(), search) + if err != nil { + return nil, err + } + + var logins []BitwardenItem + for _, item := range items { + if item.Type != bwTypeLogin { + continue + } + + entry := BitwardenItem{ID: item.ID, Name: item.Name} + if item.Login != nil { + entry.Username = item.Login.Username + if len(item.Login.URIs) > 0 { + entry.URI = item.Login.URIs[0].URI + } + } + logins = append(logins, entry) + } + + return logins, nil +} + +// Sync pulls the latest vault contents from the server. The CLI serves items +// from a local cache, so items created elsewhere only appear after a sync. +func (p *BitwardenProvider) Sync() error { + client, err := p.ensureClient() + if err != nil { + return err + } + return client.sync(context.Background()) +} + +// Warmup starts the helper process in the background so that a later call from +// the UI thread does not have to wait for the CLI to boot. +func (p *BitwardenProvider) Warmup() { + if !p.enabled { + return + } + go func() { + _, _ = p.ensureClient() + }() +} + +// Close stops the helper process. +func (p *BitwardenProvider) Close() error { + p.mu.Lock() + server, client := p.server, p.client + p.server, p.client = nil, nil + p.mu.Unlock() + + if server != nil { + server.Stop() + } + _ = client + + return nil +} + +// ensureClient returns a client for a running server, starting one if needed. +func (p *BitwardenProvider) ensureClient() (*bwClient, error) { + if !p.enabled { + return nil, fmt.Errorf("Bitwarden CLI (bw) is not installed or not in PATH") + } + + p.mu.Lock() + defer p.mu.Unlock() + + // Restart if a previously started server has died. + if p.client != nil && (p.server == nil || !p.server.Exited()) { + return p.client, nil + } + if p.server != nil { + p.server.Stop() + p.server, p.client = nil, nil + } + + client, server, err := p.newClient() + if err != nil { + return nil, err + } + + p.client, p.server = client, server + return client, nil +} + +// startServer spawns "bw serve" and waits until it answers requests. +func (p *BitwardenProvider) startServer() (*bwClient, *bwServer, error) { + server, err := startBwServe(p.bwPath) + if err != nil { + return nil, nil, err + } + + client := newBwClient(server.baseURL) + if err := waitForReady(context.Background(), client, server.exited); err != nil { + tail := server.StderrTail() + server.Stop() + + // The CLI refuses to start the server at all when no user is logged in, + // and prints the reason on stderr. Report that as the vault state + // rather than as a start-up failure, so the caller can offer the right + // advice. + if stateErr := classifyStartupFailure(tail); stateErr != nil { + return nil, nil, stateErr + } + if tail != "" { + return nil, nil, fmt.Errorf("%w: %s", err, tail) + } + return nil, nil, err + } + + return client, server, nil +} + +// classifyStartupFailure maps the CLI output of a server that refused to start +// onto a vault state error, or returns nil when the output does not name one. +func classifyStartupFailure(output string) error { + lower := strings.ToLower(output) + + switch { + case strings.Contains(lower, "not logged in"): + return ErrNotAuthenticated + case strings.Contains(lower, "vault is locked"), strings.Contains(lower, "vault is locked."): + return ErrVaultLocked + } + + return nil +} diff --git a/internal/secrets/bitwarden_cli_test.go b/internal/secrets/bitwarden_cli_test.go new file mode 100644 index 0000000..d16a140 --- /dev/null +++ b/internal/secrets/bitwarden_cli_test.go @@ -0,0 +1,77 @@ +package secrets + +import ( + "errors" + "os/exec" + "testing" +) + +func TestClassifyStartupFailure(t *testing.T) { + tests := []struct { + output string + want error + }{ + {output: "You are not logged in.", want: ErrNotAuthenticated}, + {output: "Vault is locked.", want: ErrVaultLocked}, + {output: "EADDRINUSE: address already in use", want: nil}, + {output: "", want: nil}, + } + + for _, tt := range tests { + got := classifyStartupFailure(tt.output) + if tt.want == nil { + if got != nil { + t.Errorf("classifyStartupFailure(%q) = %v, want nil", tt.output, got) + } + continue + } + if !errors.Is(got, tt.want) { + t.Errorf("classifyStartupFailure(%q) = %v, want %v", tt.output, got, tt.want) + } + } +} + +// TestRealBwServeLifecycle exercises the actual Bitwarden CLI when it is +// installed. It does not need a logged-in vault: an unauthenticated CLI is a +// valid outcome and is checked for the right error. +func TestRealBwServeLifecycle(t *testing.T) { + if _, err := exec.LookPath("bw"); err != nil { + t.Skip("bw CLI not in PATH") + } + + provider := NewBitwardenProvider() + defer provider.Close() + + if !provider.IsEnabled() { + t.Fatal("provider reports disabled although bw is in PATH") + } + + status, err := provider.Status() + switch { + case errors.Is(err, ErrNotAuthenticated): + // The CLI refuses to serve without a login, which is the expected + // result on a machine that has never run "bw login". + t.Log("bw is not logged in; start-up failure correctly reported") + return + case err != nil: + t.Fatalf("Status returned error: %v", err) + } + + t.Logf("vault status: %q", status) + + provider.mu.Lock() + server := provider.server + provider.mu.Unlock() + + if server == nil { + t.Fatal("no bw serve process was started") + } + t.Logf("bw serve pid=%d port=%d", server.cmd.Process.Pid, server.port) + + if err := provider.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) + } + if !server.Exited() { + t.Error("bw serve is still running after Close") + } +} diff --git a/internal/secrets/bitwarden_client.go b/internal/secrets/bitwarden_client.go new file mode 100644 index 0000000..75b9fc3 --- /dev/null +++ b/internal/secrets/bitwarden_client.go @@ -0,0 +1,256 @@ +package secrets + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "strings" + "time" +) + +// Errors returned by the Bitwarden client that callers may want to react to. +var ( + // ErrVaultLocked is returned when the vault is locked, so no secret can be + // read until the user unlocks it. + ErrVaultLocked = errors.New("bitwarden vault is locked") + // ErrNotAuthenticated is returned when no user is logged in. + ErrNotAuthenticated = errors.New("not logged in to bitwarden") + // ErrNotFound is returned when the requested item does not exist. + ErrNotFound = errors.New("item not found") +) + +// Vault states reported by the Bitwarden CLI. +const ( + bwStatusUnlocked = "unlocked" + bwStatusLocked = "locked" + bwStatusUnauthenticated = "unauthenticated" +) + +// Bitwarden item types; only logins carry credentials. +const bwTypeLogin = 1 + +const ( + bwRequestTimeout = 15 * time.Second + bwSyncTimeout = 60 * time.Second +) + +// bwClient talks to the local REST API exposed by "bw serve". +type bwClient struct { + baseURL string + http *http.Client +} + +func newBwClient(baseURL string) *bwClient { + return &bwClient{ + baseURL: strings.TrimSuffix(baseURL, "/"), + http: &http.Client{Timeout: bwSyncTimeout}, + } +} + +// bwEnvelope is the response wrapper used by every "bw serve" endpoint. +type bwEnvelope struct { + Success bool `json:"success"` + Data json.RawMessage `json:"data"` + Message string `json:"message"` +} + +type bwStatus struct { + ServerURL string `json:"serverUrl"` + UserEmail string `json:"userEmail"` + Status string `json:"status"` +} + +type bwURI struct { + URI string `json:"uri"` + Match *int `json:"match,omitempty"` +} + +type bwLogin struct { + Username string `json:"username,omitempty"` + Password string `json:"password,omitempty"` + Totp string `json:"totp,omitempty"` + URIs []bwURI `json:"uris,omitempty"` +} + +type bwItem struct { + ID string `json:"id,omitempty"` + Type int `json:"type"` + Name string `json:"name"` + Notes string `json:"notes,omitempty"` + Login *bwLogin `json:"login,omitempty"` + FolderID string `json:"folderId,omitempty"` +} + +// bwAPIError carries the message returned by the CLI so that it can be shown +// to the user unchanged. +type bwAPIError struct { + Message string +} + +func (e *bwAPIError) Error() string { return e.Message } + +// do performs a request and unwraps the response envelope. The Origin header is +// deliberately never set: "bw serve" rejects any request that carries one. +func (c *bwClient) do(ctx context.Context, method, path string, body any) (json.RawMessage, error) { + var reader *bytes.Reader + if body != nil { + encoded, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("failed to encode request: %w", err) + } + reader = bytes.NewReader(encoded) + } else { + reader = bytes.NewReader(nil) + } + + req, err := http.NewRequestWithContext(ctx, method, c.baseURL+path, reader) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/json") + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + + resp, err := c.http.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + var envelope bwEnvelope + if err := json.NewDecoder(resp.Body).Decode(&envelope); err != nil { + return nil, fmt.Errorf("unexpected response from bw serve (HTTP %d): %w", resp.StatusCode, err) + } + + if !envelope.Success { + return nil, classifyBwError(envelope.Message, resp.StatusCode) + } + + return envelope.Data, nil +} + +// classifyBwError maps CLI messages onto the sentinel errors above. +func classifyBwError(message string, statusCode int) error { + lower := strings.ToLower(message) + + switch { + case strings.Contains(lower, "vault is locked"): + return fmt.Errorf("%w", ErrVaultLocked) + case strings.Contains(lower, "not logged in"), strings.Contains(lower, "you are not logged in"): + return fmt.Errorf("%w", ErrNotAuthenticated) + case strings.Contains(lower, "not found"), statusCode == http.StatusNotFound: + if message == "" { + return ErrNotFound + } + return fmt.Errorf("%w: %s", ErrNotFound, message) + case message == "": + return fmt.Errorf("bw serve returned HTTP %d", statusCode) + } + + return &bwAPIError{Message: message} +} + +// status reports whether the vault is unlocked, locked or unauthenticated. +func (c *bwClient) status(ctx context.Context) (*bwStatus, error) { + ctx, cancel := context.WithTimeout(ctx, bwRequestTimeout) + defer cancel() + + data, err := c.do(ctx, http.MethodGet, "/status", nil) + if err != nil { + return nil, err + } + + // The payload is wrapped in a template object: + // {"object":"template","template":{"status":"unlocked",...}} + var wrapper struct { + Template bwStatus `json:"template"` + } + if err := json.Unmarshal(data, &wrapper); err != nil { + return nil, fmt.Errorf("failed to parse status: %w", err) + } + + return &wrapper.Template, nil +} + +// getField reads a single field of an item, e.g. its password. +func (c *bwClient) getField(ctx context.Context, field, id string) (string, error) { + ctx, cancel := context.WithTimeout(ctx, bwRequestTimeout) + defer cancel() + + path := "/object/" + url.PathEscape(field) + "/" + url.PathEscape(id) + data, err := c.do(ctx, http.MethodGet, path, nil) + if err != nil { + return "", err + } + + // Scalar fields are returned as {"object":"string","data":""}. + var wrapper struct { + Data string `json:"data"` + } + if err := json.Unmarshal(data, &wrapper); err != nil { + return "", fmt.Errorf("failed to parse %s: %w", field, err) + } + + return wrapper.Data, nil +} + +// listItems returns the vault items, optionally narrowed by a search term. +func (c *bwClient) listItems(ctx context.Context, search string) ([]bwItem, error) { + ctx, cancel := context.WithTimeout(ctx, bwRequestTimeout) + defer cancel() + + path := "/list/object/items" + if search != "" { + path += "?search=" + url.QueryEscape(search) + } + + data, err := c.do(ctx, http.MethodGet, path, nil) + if err != nil { + return nil, err + } + + var wrapper struct { + Data []bwItem `json:"data"` + } + if err := json.Unmarshal(data, &wrapper); err != nil { + return nil, fmt.Errorf("failed to parse item list: %w", err) + } + + return wrapper.Data, nil +} + +// createItem stores a new item and returns it including the assigned id. +func (c *bwClient) createItem(ctx context.Context, item bwItem) (*bwItem, error) { + ctx, cancel := context.WithTimeout(ctx, bwRequestTimeout) + defer cancel() + + data, err := c.do(ctx, http.MethodPost, "/object/item", item) + if err != nil { + return nil, err + } + + var created bwItem + if err := json.Unmarshal(data, &created); err != nil { + return nil, fmt.Errorf("failed to parse created item: %w", err) + } + + if created.ID == "" { + return nil, fmt.Errorf("bitwarden did not return an item id") + } + + return &created, nil +} + +// sync pulls the latest vault state from the server. +func (c *bwClient) sync(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, bwSyncTimeout) + defer cancel() + + _, err := c.do(ctx, http.MethodPost, "/sync", nil) + return err +} diff --git a/internal/secrets/bitwarden_process.go b/internal/secrets/bitwarden_process.go new file mode 100644 index 0000000..7e2baba --- /dev/null +++ b/internal/secrets/bitwarden_process.go @@ -0,0 +1,216 @@ +package secrets + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "os" + "os/exec" + "strconv" + "strings" + "sync" + "time" +) + +const ( + // bwReadyTimeout bounds how long we wait for "bw serve" to accept requests. + // The CLI is a Node application and can take a few seconds to boot. + bwReadyTimeout = 20 * time.Second + // bwReadyPollInterval is how often readiness is probed. + bwReadyPollInterval = 150 * time.Millisecond + // bwStopGrace is how long Stop waits for the process to disappear. + bwStopGrace = 3 * time.Second + // bwStderrLimit caps the amount of stderr kept for diagnostics. + bwStderrLimit = 4096 +) + +// bwServer is a running "bw serve" child process. +type bwServer struct { + cmd *exec.Cmd + port int + baseURL string + stderr *boundedBuffer + exited chan struct{} + + stopOnce sync.Once +} + +// boundedBuffer keeps only the last n bytes written to it. The CLI can be +// chatty and only the tail is needed to explain a failed start. +type boundedBuffer struct { + mu sync.Mutex + limit int + data []byte +} + +func newBoundedBuffer(limit int) *boundedBuffer { + return &boundedBuffer{limit: limit} +} + +func (b *boundedBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + + b.data = append(b.data, p...) + if len(b.data) > b.limit { + b.data = b.data[len(b.data)-b.limit:] + } + return len(p), nil +} + +func (b *boundedBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return strings.TrimSpace(string(b.data)) +} + +// findFreePort asks the operating system for an unused loopback port. +func findFreePort() (int, error) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return 0, err + } + defer listener.Close() + + addr, ok := listener.Addr().(*net.TCPAddr) + if !ok { + return 0, fmt.Errorf("unexpected listener address type %T", listener.Addr()) + } + return addr.Port, nil +} + +// startBwServe launches "bw serve" bound to loopback on a free port. +// +// The child inherits the environment, which is how BW_SESSION reaches it: the +// user unlocks the vault once in their shell and every request made through +// this server is then authorised. +func startBwServe(bwPath string) (*bwServer, error) { + port, err := findFreePort() + if err != nil { + return nil, fmt.Errorf("failed to reserve a local port: %w", err) + } + + args := []string{"serve", "--hostname", "127.0.0.1", "--port", strconv.Itoa(port)} + + var cmd *exec.Cmd + if isBatchFile(bwPath) { + // The npm distribution installs bw as a .cmd shim, which cannot be + // executed directly. + cmd = exec.Command("cmd", append([]string{"/c", bwPath}, args...)...) + } else { + cmd = exec.Command(bwPath, args...) + } + + cmd.Env = os.Environ() + cmd.Stdout = io.Discard + stderr := newBoundedBuffer(bwStderrLimit) + cmd.Stderr = stderr + hideConsoleWindow(cmd) + configureChildProcess(cmd) + + if err := cmd.Start(); err != nil { + return nil, fmt.Errorf("failed to start bw serve: %w", err) + } + + server := &bwServer{ + cmd: cmd, + port: port, + baseURL: fmt.Sprintf("http://127.0.0.1:%d", port), + stderr: stderr, + exited: make(chan struct{}), + } + + // Reap the child and let readiness polling notice an early exit. + go func() { + _ = cmd.Wait() + close(server.exited) + }() + + adoptChildProcess(cmd) + + return server, nil +} + +func isBatchFile(path string) bool { + lower := strings.ToLower(path) + return strings.HasSuffix(lower, ".cmd") || strings.HasSuffix(lower, ".bat") +} + +// Stop terminates the server. It is safe to call more than once. +func (s *bwServer) Stop() { + if s == nil { + return + } + + s.stopOnce.Do(func() { + if s.cmd.Process == nil { + return + } + + stopProcess(s.cmd) + + select { + case <-s.exited: + case <-time.After(bwStopGrace): + _ = s.cmd.Process.Kill() + } + }) +} + +// Exited reports whether the server process has already terminated. +func (s *bwServer) Exited() bool { + select { + case <-s.exited: + return true + default: + return false + } +} + +// StderrTail returns the tail of the stderr produced by the server, so that a +// failed start can be explained to the user. +func (s *bwServer) StderrTail() string { + if s == nil || s.stderr == nil { + return "" + } + return s.stderr.String() +} + +// waitForReady polls the status endpoint until the server answers, the process +// exits or the deadline passes. A locked or unauthenticated vault still counts +// as ready: the server is up, it simply has nothing to hand out yet. +func waitForReady(ctx context.Context, client *bwClient, exited <-chan struct{}) error { + ctx, cancel := context.WithTimeout(ctx, bwReadyTimeout) + defer cancel() + + ticker := time.NewTicker(bwReadyPollInterval) + defer ticker.Stop() + + var lastErr error + for { + _, err := client.status(ctx) + if err == nil || isVaultStateError(err) { + return nil + } + lastErr = err + + select { + case <-exited: + return fmt.Errorf("bw serve exited before it became ready") + case <-ctx.Done(): + if lastErr != nil { + return fmt.Errorf("bw serve did not become ready: %w", lastErr) + } + return fmt.Errorf("bw serve did not become ready within %s", bwReadyTimeout) + case <-ticker.C: + } + } +} + +// isVaultStateError reports whether err means the server works but the vault is +// not usable, as opposed to the server not being up yet. +func isVaultStateError(err error) bool { + return errors.Is(err, ErrVaultLocked) || errors.Is(err, ErrNotAuthenticated) +} diff --git a/internal/secrets/bitwarden_process_linux.go b/internal/secrets/bitwarden_process_linux.go new file mode 100644 index 0000000..c930712 --- /dev/null +++ b/internal/secrets/bitwarden_process_linux.go @@ -0,0 +1,17 @@ +//go:build linux + +package secrets + +import ( + "os/exec" + "syscall" +) + +// setParentDeathSignal makes the kernel send SIGTERM to the child when this +// process exits for any reason. +func setParentDeathSignal(cmd *exec.Cmd) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Pdeathsig = syscall.SIGTERM +} diff --git a/internal/secrets/bitwarden_process_other.go b/internal/secrets/bitwarden_process_other.go new file mode 100644 index 0000000..c4802bf --- /dev/null +++ b/internal/secrets/bitwarden_process_other.go @@ -0,0 +1,9 @@ +//go:build !windows && !linux + +package secrets + +import "os/exec" + +// setParentDeathSignal is not available on this platform. The server is still +// stopped on a clean exit; only a hard crash can leave it running. +func setParentDeathSignal(cmd *exec.Cmd) {} diff --git a/internal/secrets/bitwarden_process_unix.go b/internal/secrets/bitwarden_process_unix.go new file mode 100644 index 0000000..51e80f4 --- /dev/null +++ b/internal/secrets/bitwarden_process_unix.go @@ -0,0 +1,28 @@ +//go:build !windows + +package secrets + +import ( + "os/exec" + "syscall" +) + +// configureChildProcess asks the kernel to signal the child when this process +// dies, so a crash cannot leave an orphaned "bw serve" running. +func configureChildProcess(cmd *exec.Cmd) { + setParentDeathSignal(cmd) +} + +// adoptChildProcess has nothing to do on Unix; the death signal is configured +// before the process starts. +func adoptChildProcess(cmd *exec.Cmd) {} + +// stopProcess asks the child to terminate. +func stopProcess(cmd *exec.Cmd) { + if cmd.Process == nil { + return + } + if err := cmd.Process.Signal(syscall.SIGTERM); err != nil { + _ = cmd.Process.Kill() + } +} diff --git a/internal/secrets/bitwarden_process_windows.go b/internal/secrets/bitwarden_process_windows.go new file mode 100644 index 0000000..a85d420 --- /dev/null +++ b/internal/secrets/bitwarden_process_windows.go @@ -0,0 +1,81 @@ +//go:build windows + +package secrets + +import ( + "os/exec" + "sync" + "unsafe" + + "golang.org/x/sys/windows" +) + +var ( + jobOnce sync.Once + jobHandle windows.Handle +) + +// killOnCloseJob returns a job object configured to kill its members once the +// last handle to it is closed, which happens when this process dies. That way a +// crash cannot leave an orphaned "bw serve" listening with an unlocked vault. +func killOnCloseJob() windows.Handle { + jobOnce.Do(func() { + handle, err := windows.CreateJobObject(nil, nil) + if err != nil { + return + } + + info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{ + BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{ + LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }, + } + if _, err := windows.SetInformationJobObject( + handle, + windows.JobObjectExtendedLimitInformation, + uintptr(unsafe.Pointer(&info)), + uint32(unsafe.Sizeof(info)), + ); err != nil { + _ = windows.CloseHandle(handle) + return + } + + jobHandle = handle + }) + + return jobHandle +} + +// configureChildProcess is a no-op on Windows; the console window is already +// hidden by hideConsoleWindow. +func configureChildProcess(cmd *exec.Cmd) {} + +// adoptChildProcess assigns the started child to the kill-on-close job object. +// Failures are ignored: losing this safety net is not a reason to refuse the +// connection, and Stop still terminates the process on a clean exit. +func adoptChildProcess(cmd *exec.Cmd) { + job := killOnCloseJob() + if job == 0 || cmd.Process == nil { + return + } + + handle, err := windows.OpenProcess( + windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, + false, + uint32(cmd.Process.Pid), + ) + if err != nil { + return + } + defer windows.CloseHandle(handle) + + _ = windows.AssignProcessToJobObject(job, handle) +} + +// stopProcess terminates the child. Windows offers no graceful signal for a +// console-less child process, so it is killed outright. +func stopProcess(cmd *exec.Cmd) { + if cmd.Process != nil { + _ = cmd.Process.Kill() + } +} diff --git a/internal/secrets/bitwarden_reference.go b/internal/secrets/bitwarden_reference.go new file mode 100644 index 0000000..f2bdf09 --- /dev/null +++ b/internal/secrets/bitwarden_reference.go @@ -0,0 +1,62 @@ +package secrets + +import ( + "fmt" + "strings" +) + +const bitwardenPrefix = SchemeBitwarden + "://" + +// Fields of a Bitwarden login item that a reference may point at. +const ( + bitwardenFieldPassword = "password" + bitwardenFieldUsername = "username" + bitwardenFieldTotp = "totp" + bitwardenFieldNotes = "notes" +) + +var bitwardenFields = map[string]bool{ + bitwardenFieldPassword: true, + bitwardenFieldUsername: true, + bitwardenFieldTotp: true, + bitwardenFieldNotes: true, +} + +// parseBitwardenReference splits a reference into the item id and the field to +// read. Accepted formats: +// +// bw:// -> the item's password +// bw:/// -> password, username, totp or notes +func parseBitwardenReference(reference string) (id string, field string, err error) { + if !strings.HasPrefix(reference, bitwardenPrefix) { + return "", "", fmt.Errorf("reference must start with %s", bitwardenPrefix) + } + + rest := strings.TrimPrefix(reference, bitwardenPrefix) + parts := strings.Split(rest, "/") + + switch len(parts) { + case 1: + id, field = parts[0], bitwardenFieldPassword + case 2: + id, field = parts[0], parts[1] + default: + return "", "", fmt.Errorf("reference must be in format %s[/]", bitwardenPrefix) + } + + if id == "" { + return "", "", fmt.Errorf("reference is missing an item id") + } + + if !bitwardenFields[field] { + return "", "", fmt.Errorf("unsupported field %q, expected one of password, username, totp, notes", field) + } + + return id, field, nil +} + +// bitwardenReference builds the reference stored in the configuration for an +// item id. +func bitwardenReference(id string) string { + return bitwardenPrefix + id +} diff --git a/internal/secrets/bitwarden_reference_test.go b/internal/secrets/bitwarden_reference_test.go new file mode 100644 index 0000000..768cded --- /dev/null +++ b/internal/secrets/bitwarden_reference_test.go @@ -0,0 +1,61 @@ +package secrets + +import "testing" + +func TestParseBitwardenReference(t *testing.T) { + tests := []struct { + reference string + wantID string + wantField string + wantErr bool + }{ + {reference: "bw://abc-123", wantID: "abc-123", wantField: "password"}, + {reference: "bw://abc-123/password", wantID: "abc-123", wantField: "password"}, + {reference: "bw://abc-123/username", wantID: "abc-123", wantField: "username"}, + {reference: "bw://abc-123/totp", wantID: "abc-123", wantField: "totp"}, + {reference: "bw://abc-123/notes", wantID: "abc-123", wantField: "notes"}, + {reference: "bw://abc-123/secret", wantErr: true}, + {reference: "bw://abc-123/", wantErr: true}, + {reference: "bw://abc-123/password/extra", wantErr: true}, + {reference: "bw://", wantErr: true}, + {reference: "op://Private/Server/password", wantErr: true}, + {reference: "hunter2", wantErr: true}, + {reference: "", wantErr: true}, + } + + for _, tt := range tests { + id, field, err := parseBitwardenReference(tt.reference) + + if tt.wantErr { + if err == nil { + t.Errorf("parseBitwardenReference(%q) = (%q, %q), want an error", tt.reference, id, field) + } + continue + } + + if err != nil { + t.Errorf("parseBitwardenReference(%q) returned error: %v", tt.reference, err) + continue + } + if id != tt.wantID || field != tt.wantField { + t.Errorf("parseBitwardenReference(%q) = (%q, %q), want (%q, %q)", + tt.reference, id, field, tt.wantID, tt.wantField) + } + } +} + +func TestBitwardenReference(t *testing.T) { + if got := bitwardenReference("abc-123"); got != "bw://abc-123" { + t.Errorf("bitwardenReference = %q, want %q", got, "bw://abc-123") + } +} + +func TestBitwardenReferenceRoundTrip(t *testing.T) { + id, field, err := parseBitwardenReference(bitwardenReference("abc-123")) + if err != nil { + t.Fatalf("round trip returned error: %v", err) + } + if id != "abc-123" || field != bitwardenFieldPassword { + t.Errorf("round trip = (%q, %q), want (abc-123, password)", id, field) + } +} diff --git a/internal/secrets/bitwarden_test.go b/internal/secrets/bitwarden_test.go new file mode 100644 index 0000000..b607b85 --- /dev/null +++ b/internal/secrets/bitwarden_test.go @@ -0,0 +1,477 @@ +package secrets + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// fakeBw is a stand-in for "bw serve" so the tests never touch the real CLI. +type fakeBw struct { + server *httptest.Server + + state string + items map[string]bwItem + lastCreated bwItem + sawOrigin bool + lastSearch string + syncCalls int +} + +func newFakeBw(t *testing.T) *fakeBw { + t.Helper() + + f := &fakeBw{ + state: bwStatusUnlocked, + items: make(map[string]bwItem), + } + + mux := http.NewServeMux() + + mux.HandleFunc("/status", func(w http.ResponseWriter, r *http.Request) { + f.note(r) + writeEnvelope(w, map[string]any{ + "object": "template", + "template": map[string]string{ + "serverUrl": "https://vault.example.com", + "userEmail": "user@example.com", + "status": f.state, + }, + }) + }) + + mux.HandleFunc("/sync", func(w http.ResponseWriter, r *http.Request) { + f.note(r) + if f.locked(w) { + return + } + f.syncCalls++ + writeEnvelope(w, map[string]string{"object": "message", "title": "Syncing complete."}) + }) + + mux.HandleFunc("/list/object/items", func(w http.ResponseWriter, r *http.Request) { + f.note(r) + if f.locked(w) { + return + } + f.lastSearch = r.URL.Query().Get("search") + + list := make([]bwItem, 0, len(f.items)) + for _, item := range f.items { + list = append(list, item) + } + writeEnvelope(w, map[string]any{"object": "list", "data": list}) + }) + + mux.HandleFunc("/object/", func(w http.ResponseWriter, r *http.Request) { + f.note(r) + if f.locked(w) { + return + } + + if r.Method == http.MethodPost { + var item bwItem + if err := json.NewDecoder(r.Body).Decode(&item); err != nil { + writeError(w, http.StatusBadRequest, "bad request body") + return + } + item.ID = "created-id" + f.lastCreated = item + f.items[item.ID] = item + writeEnvelope(w, item) + return + } + + // /object// + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + if len(parts) != 3 { + writeError(w, http.StatusNotFound, "Not found.") + return + } + kind, id := parts[1], parts[2] + + item, ok := f.items[id] + if !ok { + writeError(w, http.StatusNotFound, "Not found.") + return + } + + if kind == "item" { + writeEnvelope(w, item) + return + } + + var value string + if item.Login != nil { + switch kind { + case bitwardenFieldPassword: + value = item.Login.Password + case bitwardenFieldUsername: + value = item.Login.Username + case bitwardenFieldTotp: + value = item.Login.Totp + } + } + if kind == bitwardenFieldNotes { + value = item.Notes + } + + writeEnvelope(w, map[string]string{"object": "string", "data": value}) + }) + + f.server = httptest.NewServer(mux) + t.Cleanup(f.server.Close) + return f +} + +func (f *fakeBw) note(r *http.Request) { + if r.Header.Get("Origin") != "" { + f.sawOrigin = true + } +} + +// locked mimics the CLI, which rejects vault access while locked. +func (f *fakeBw) locked(w http.ResponseWriter) bool { + switch f.state { + case bwStatusLocked: + writeError(w, http.StatusOK, "Vault is locked.") + return true + case bwStatusUnauthenticated: + writeError(w, http.StatusOK, "You are not logged in.") + return true + } + return false +} + +func (f *fakeBw) provider() *BitwardenProvider { + return newBitwardenProviderWithClient(newBwClient(f.server.URL)) +} + +func (f *fakeBw) addLogin(id, name, username, password string) { + f.items[id] = bwItem{ + ID: id, + Type: bwTypeLogin, + Name: name, + Login: &bwLogin{Username: username, Password: password}, + } +} + +func writeEnvelope(w http.ResponseWriter, data any) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"success": true, "data": data}) +} + +func writeError(w http.ResponseWriter, status int, message string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(map[string]any{"success": false, "message": message}) +} + +func TestBitwardenResolvePassword(t *testing.T) { + fake := newFakeBw(t) + fake.addLogin("abc-123", "Web Server", "admin", "hunter2") + + got, err := fake.provider().ResolveSecret("bw://abc-123") + if err != nil { + t.Fatalf("ResolveSecret returned error: %v", err) + } + if got != "hunter2" { + t.Errorf("ResolveSecret = %q, want %q", got, "hunter2") + } +} + +func TestBitwardenResolveExplicitField(t *testing.T) { + fake := newFakeBw(t) + fake.addLogin("abc-123", "Web Server", "admin", "hunter2") + + got, err := fake.provider().ResolveSecret("bw://abc-123/username") + if err != nil { + t.Fatalf("ResolveSecret returned error: %v", err) + } + if got != "admin" { + t.Errorf("ResolveSecret = %q, want %q", got, "admin") + } +} + +func TestBitwardenResolveLockedVault(t *testing.T) { + fake := newFakeBw(t) + fake.addLogin("abc-123", "Web Server", "admin", "hunter2") + fake.state = bwStatusLocked + + _, err := fake.provider().ResolveSecret("bw://abc-123") + if !errors.Is(err, ErrVaultLocked) { + t.Fatalf("ResolveSecret error = %v, want ErrVaultLocked", err) + } +} + +func TestBitwardenResolveUnauthenticated(t *testing.T) { + fake := newFakeBw(t) + fake.state = bwStatusUnauthenticated + + _, err := fake.provider().ResolveSecret("bw://abc-123") + if !errors.Is(err, ErrNotAuthenticated) { + t.Fatalf("ResolveSecret error = %v, want ErrNotAuthenticated", err) + } +} + +func TestBitwardenResolveUnknownItem(t *testing.T) { + fake := newFakeBw(t) + + _, err := fake.provider().ResolveSecret("bw://missing") + if !errors.Is(err, ErrNotFound) { + t.Fatalf("ResolveSecret error = %v, want ErrNotFound", err) + } +} + +func TestBitwardenIsAuthenticated(t *testing.T) { + tests := map[string]bool{ + bwStatusUnlocked: true, + bwStatusLocked: false, + bwStatusUnauthenticated: false, + } + + for state, want := range tests { + fake := newFakeBw(t) + fake.state = state + + if got := fake.provider().IsAuthenticated(); got != want { + t.Errorf("IsAuthenticated with status %q = %v, want %v", state, got, want) + } + } +} + +func TestBitwardenCreateItem(t *testing.T) { + fake := newFakeBw(t) + + reference, err := fake.provider().CreateItem(CreateItemRequest{ + Title: "Web Server", + Username: "admin", + Password: "hunter2", + URI: "ssh://web.example.com", + }) + if err != nil { + t.Fatalf("CreateItem returned error: %v", err) + } + if reference != "bw://created-id" { + t.Errorf("CreateItem = %q, want %q", reference, "bw://created-id") + } + + created := fake.lastCreated + if created.Type != bwTypeLogin { + t.Errorf("created item type = %d, want %d", created.Type, bwTypeLogin) + } + if created.Name != "Web Server" { + t.Errorf("created item name = %q, want %q", created.Name, "Web Server") + } + if created.Login == nil || created.Login.Password != "hunter2" || created.Login.Username != "admin" { + t.Fatalf("created item login = %+v, want admin/hunter2", created.Login) + } + if len(created.Login.URIs) != 1 || created.Login.URIs[0].URI != "ssh://web.example.com" { + t.Errorf("created item URIs = %+v, want one ssh:// entry", created.Login.URIs) + } +} + +func TestBitwardenCreateItemRequiresTitle(t *testing.T) { + fake := newFakeBw(t) + + if _, err := fake.provider().CreateItem(CreateItemRequest{Password: "hunter2"}); err == nil { + t.Error("expected CreateItem to reject an empty title") + } +} + +func TestBitwardenListLoginItemsSkipsOtherTypes(t *testing.T) { + fake := newFakeBw(t) + fake.addLogin("login-1", "Web Server", "admin", "hunter2") + fake.items["note-1"] = bwItem{ID: "note-1", Type: 2, Name: "Secure Note"} + + items, err := fake.provider().ListLoginItems("") + if err != nil { + t.Fatalf("ListLoginItems returned error: %v", err) + } + if len(items) != 1 { + t.Fatalf("ListLoginItems returned %d items, want 1", len(items)) + } + if items[0].ID != "login-1" || items[0].Username != "admin" { + t.Errorf("ListLoginItems = %+v, want the login item", items[0]) + } +} + +func TestBitwardenListLoginItemsPassesSearch(t *testing.T) { + fake := newFakeBw(t) + + if _, err := fake.provider().ListLoginItems("web server"); err != nil { + t.Fatalf("ListLoginItems returned error: %v", err) + } + if fake.lastSearch != "web server" { + t.Errorf("search term = %q, want %q", fake.lastSearch, "web server") + } +} + +func TestBitwardenSync(t *testing.T) { + fake := newFakeBw(t) + + if err := fake.provider().Sync(); err != nil { + t.Fatalf("Sync returned error: %v", err) + } + if fake.syncCalls != 1 { + t.Errorf("sync called %d times, want 1", fake.syncCalls) + } +} + +// bw serve rejects any request carrying an Origin header, so the client must +// never send one. +func TestBitwardenClientSendsNoOriginHeader(t *testing.T) { + fake := newFakeBw(t) + fake.addLogin("abc-123", "Web Server", "admin", "hunter2") + + provider := fake.provider() + if _, err := provider.ResolveSecret("bw://abc-123"); err != nil { + t.Fatalf("ResolveSecret returned error: %v", err) + } + if _, err := provider.ListLoginItems(""); err != nil { + t.Fatalf("ListLoginItems returned error: %v", err) + } + + if fake.sawOrigin { + t.Error("client sent an Origin header, which bw serve would reject") + } +} + +func TestBitwardenDisabledProviderReportsMissingCLI(t *testing.T) { + provider := &BitwardenProvider{enabled: false} + + _, err := provider.ResolveSecret("bw://abc-123") + if err == nil { + t.Fatal("expected an error when the CLI is missing") + } + if !strings.Contains(err.Error(), "not installed") { + t.Errorf("error = %q, want it to mention the missing CLI", err) + } + + if provider.IsAuthenticated() { + t.Error("a disabled provider must not report itself as authenticated") + } +} + +func TestBitwardenIsReference(t *testing.T) { + provider := &BitwardenProvider{} + + tests := map[string]bool{ + "bw://abc": true, + "bw://abc/username": true, + "op://Private/Server/password": false, + "hunter2": false, + "": false, + } + + for value, want := range tests { + if got := provider.IsReference(value); got != want { + t.Errorf("IsReference(%q) = %v, want %v", value, got, want) + } + } +} + +func TestBitwardenResolveRejectsForeignReference(t *testing.T) { + fake := newFakeBw(t) + + if _, err := fake.provider().ResolveSecret("op://Private/Server/password"); err == nil { + t.Error("expected a 1Password reference to be rejected") + } +} + +func TestBitwardenStatusReported(t *testing.T) { + fake := newFakeBw(t) + fake.state = bwStatusLocked + + status, err := fake.provider().Status() + if err != nil { + t.Fatalf("Status returned error: %v", err) + } + if status != bwStatusLocked { + t.Errorf("Status = %q, want %q", status, bwStatusLocked) + } +} + +func TestWaitForReadyAcceptsLockedVault(t *testing.T) { + fake := newFakeBw(t) + fake.state = bwStatusLocked + + // A locked vault still means the server is up and usable once unlocked. + if err := waitForReady(context.Background(), newBwClient(fake.server.URL), make(chan struct{})); err != nil { + t.Errorf("waitForReady returned error for a locked vault: %v", err) + } +} + +func TestWaitForReadyDetectsExit(t *testing.T) { + // A server that never answers, plus an already exited process. + dead := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "boom", http.StatusInternalServerError) + })) + defer dead.Close() + + exited := make(chan struct{}) + close(exited) + + err := waitForReady(context.Background(), newBwClient(dead.URL), exited) + if err == nil { + t.Fatal("expected an error when the process has exited") + } + if !strings.Contains(err.Error(), "exited") { + t.Errorf("error = %q, want it to mention the exit", err) + } +} + +func TestWaitForReadyTimesOut(t *testing.T) { + dead := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "boom", http.StatusInternalServerError) + })) + defer dead.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 400*time.Millisecond) + defer cancel() + + if err := waitForReady(ctx, newBwClient(dead.URL), make(chan struct{})); err == nil { + t.Fatal("expected waitForReady to give up") + } +} + +func TestFindFreePort(t *testing.T) { + port, err := findFreePort() + if err != nil { + t.Fatalf("findFreePort returned error: %v", err) + } + if port <= 0 || port > 65535 { + t.Errorf("findFreePort = %d, want a valid TCP port", port) + } +} + +func TestBoundedBufferKeepsTail(t *testing.T) { + buf := newBoundedBuffer(8) + if _, err := buf.Write([]byte("abcdefghijkl")); err != nil { + t.Fatalf("Write returned error: %v", err) + } + if got := buf.String(); got != "efghijkl" { + t.Errorf("String = %q, want %q", got, "efghijkl") + } +} + +func TestIsBatchFile(t *testing.T) { + tests := map[string]bool{ + `C:\npm\bw.cmd`: true, + `C:\npm\bw.CMD`: true, + `C:\tools\bw.bat`: true, + `C:\tools\bw.exe`: false, + "/usr/bin/bw": false, + } + + for path, want := range tests { + if got := isBatchFile(path); got != want { + t.Errorf("isBatchFile(%q) = %v, want %v", path, got, want) + } + } +} diff --git a/internal/secrets/onepassword.go b/internal/secrets/onepassword.go index 8b1ee06..10af8e4 100644 --- a/internal/secrets/onepassword.go +++ b/internal/secrets/onepassword.go @@ -12,6 +12,17 @@ type OnePasswordProvider struct { enabled bool } +var ( + _ Provider = (*OnePasswordProvider)(nil) + _ ItemCreator = (*OnePasswordProvider)(nil) +) + +// Name returns the human readable provider name. +func (p *OnePasswordProvider) Name() string { return "1Password" } + +// Scheme returns the reference scheme handled by this provider. +func (p *OnePasswordProvider) Scheme() string { return SchemeOnePassword } + // NewOnePasswordProvider creates a new 1Password provider func NewOnePasswordProvider() *OnePasswordProvider { return &OnePasswordProvider{ @@ -193,7 +204,11 @@ func (p *OnePasswordProvider) CheckItemExists(vault, title string) (string, bool // CreateItem creates a new Login item in 1Password // Returns the 1Password reference (op://vault/title/password) -func (p *OnePasswordProvider) CreateItem(vault, title, username, password string) (string, error) { +// The URI field of the request is ignored; 1Password items created here are +// plain login items. +func (p *OnePasswordProvider) CreateItem(req CreateItemRequest) (string, error) { + vault, title, username, password := req.Vault, req.Title, req.Username, req.Password + if !p.enabled { return "", fmt.Errorf("1Password CLI is not available") } diff --git a/internal/secrets/provider.go b/internal/secrets/provider.go new file mode 100644 index 0000000..8c611f4 --- /dev/null +++ b/internal/secrets/provider.go @@ -0,0 +1,76 @@ +package secrets + +import "strings" + +// Scheme identifiers for the supported secret providers. A connection password +// that starts with "://" is a reference and is resolved at launch time +// instead of being stored in the configuration file. +const ( + SchemeOnePassword = "op" + SchemeBitwarden = "bw" +) + +// knownSchemes lists every scheme understood by this package. It is kept as a +// package level variable (rather than derived from a Registry) so that callers +// which must not instantiate providers - such as the crypto package deciding +// whether a value needs encrypting - can still recognise a reference. +var knownSchemes = []string{SchemeOnePassword, SchemeBitwarden} + +// Provider resolves secret references belonging to a single scheme. +type Provider interface { + // Name returns a human readable provider name, e.g. "1Password". + Name() string + + // Scheme returns the URL scheme handled by this provider, e.g. "op". + Scheme() string + + // IsEnabled reports whether the backing tool is available on this machine. + IsEnabled() bool + + // IsAuthenticated reports whether secrets can currently be retrieved. + IsAuthenticated() bool + + // GetAuthenticationInstructions returns user facing text explaining how to + // authenticate when IsAuthenticated returns false. + GetAuthenticationInstructions() string + + // IsReference reports whether value is a reference for this provider. + IsReference(value string) bool + + // ResolveSecret retrieves the secret a reference points at. + ResolveSecret(reference string) (string, error) +} + +// CreateItemRequest is the provider agnostic input for creating a login item. +// Fields that a provider does not support are ignored by that provider. +type CreateItemRequest struct { + Vault string // 1Password vault name; ignored by Bitwarden + Title string + Username string + Password string + URI string // e.g. "ssh://host"; ignored by 1Password +} + +// ItemCreator is implemented by providers able to store new login items. +type ItemCreator interface { + // CreateItem stores a login item and returns a reference to it. + CreateItem(req CreateItemRequest) (reference string, err error) +} + +// KnownSchemes returns the schemes understood by this package. +func KnownSchemes() []string { + out := make([]string, len(knownSchemes)) + copy(out, knownSchemes) + return out +} + +// IsKnownReference reports whether value uses any known scheme prefix. It does +// not require a provider to be installed or authenticated. +func IsKnownReference(value string) bool { + for _, scheme := range knownSchemes { + if strings.HasPrefix(value, scheme+"://") { + return true + } + } + return false +} diff --git a/internal/secrets/registry.go b/internal/secrets/registry.go new file mode 100644 index 0000000..9f09e9f --- /dev/null +++ b/internal/secrets/registry.go @@ -0,0 +1,111 @@ +package secrets + +import ( + "fmt" + "io" + "sync" +) + +// Registry holds the configured secret providers and routes references to the +// provider that understands them. +type Registry struct { + providers []Provider +} + +// NewRegistry creates a registry from the given providers. +func NewRegistry(providers ...Provider) *Registry { + return &Registry{providers: providers} +} + +// Providers returns every registered provider. +func (r *Registry) Providers() []Provider { + return r.providers +} + +// ByScheme returns the provider registered for a scheme such as "op" or "bw". +func (r *Registry) ByScheme(scheme string) (Provider, bool) { + for _, p := range r.providers { + if p.Scheme() == scheme { + return p, true + } + } + return nil, false +} + +// ProviderFor returns the provider that owns the given reference. +func (r *Registry) ProviderFor(value string) (Provider, bool) { + for _, p := range r.providers { + if p.IsReference(value) { + return p, true + } + } + return nil, false +} + +// IsReference reports whether value is a reference for any registered provider. +func (r *Registry) IsReference(value string) bool { + _, ok := r.ProviderFor(value) + return ok +} + +// Resolve resolves a reference. Values that are not references are returned +// unchanged, which lets callers pass plain passwords through. +func (r *Registry) Resolve(value string) (string, error) { + provider, ok := r.ProviderFor(value) + if !ok { + return value, nil + } + + secret, err := provider.ResolveSecret(value) + if err != nil { + return "", fmt.Errorf("%s: %w", provider.Name(), err) + } + return secret, nil +} + +// Close releases resources held by providers, such as helper processes. +func (r *Registry) Close() { + for _, p := range r.providers { + if closer, ok := p.(io.Closer); ok { + _ = closer.Close() + } + } +} + +var ( + defaultOnce sync.Once + defaultMu sync.Mutex + defaultRegistry *Registry +) + +// Default returns the process wide registry. Providers are shared because some +// of them own a helper process (see BitwardenProvider) that must exist only +// once per application run. +func Default() *Registry { + defaultOnce.Do(func() { + registry := NewRegistry( + NewOnePasswordProvider(), + NewBitwardenProvider(), + ) + defaultMu.Lock() + defaultRegistry = registry + defaultMu.Unlock() + }) + + defaultMu.Lock() + defer defaultMu.Unlock() + return defaultRegistry +} + +// Shutdown closes the default registry if it was ever created. It is safe to +// call multiple times and must be called before the process exits so that +// helper processes are terminated. +func Shutdown() { + defaultMu.Lock() + registry := defaultRegistry + defaultMu.Unlock() + + if registry != nil { + registry.Close() + } +} diff --git a/internal/secrets/registry_test.go b/internal/secrets/registry_test.go new file mode 100644 index 0000000..99aa2c8 --- /dev/null +++ b/internal/secrets/registry_test.go @@ -0,0 +1,154 @@ +package secrets + +import ( + "errors" + "strings" + "testing" +) + +// fakeProvider is a Provider that resolves references from a static map. +type fakeProvider struct { + name string + scheme string + enabled bool + authed bool + values map[string]string + resolveErr error +} + +func (f *fakeProvider) Name() string { return f.name } +func (f *fakeProvider) Scheme() string { return f.scheme } +func (f *fakeProvider) IsEnabled() bool { + return f.enabled +} +func (f *fakeProvider) IsAuthenticated() bool { return f.authed } +func (f *fakeProvider) GetAuthenticationInstructions() string { return "sign in to " + f.name } +func (f *fakeProvider) IsReference(value string) bool { + return strings.HasPrefix(value, f.scheme+"://") +} +func (f *fakeProvider) ResolveSecret(reference string) (string, error) { + if f.resolveErr != nil { + return "", f.resolveErr + } + value, ok := f.values[reference] + if !ok { + return "", errors.New("not found") + } + return value, nil +} + +func newTestRegistry() (*Registry, *fakeProvider, *fakeProvider) { + op := &fakeProvider{ + name: "1Password", scheme: SchemeOnePassword, enabled: true, authed: true, + values: map[string]string{"op://Private/Server/password": "op-secret"}, + } + bw := &fakeProvider{ + name: "Bitwarden", scheme: SchemeBitwarden, enabled: true, authed: true, + values: map[string]string{"bw://item-id": "bw-secret"}, + } + return NewRegistry(op, bw), op, bw +} + +func TestRegistryProviderFor(t *testing.T) { + registry, op, bw := newTestRegistry() + + tests := []struct { + value string + want Provider + }{ + {"op://Private/Server/password", op}, + {"bw://item-id", bw}, + {"plain-password", nil}, + {"", nil}, + {"enc:AAAA", nil}, + } + + for _, tt := range tests { + got, ok := registry.ProviderFor(tt.value) + if tt.want == nil { + if ok { + t.Errorf("ProviderFor(%q) = %s, want no provider", tt.value, got.Name()) + } + continue + } + if !ok || got != tt.want { + t.Errorf("ProviderFor(%q) = %v, want %s", tt.value, got, tt.want.Name()) + } + } +} + +func TestRegistryResolve(t *testing.T) { + registry, _, _ := newTestRegistry() + + got, err := registry.Resolve("bw://item-id") + if err != nil { + t.Fatalf("Resolve returned error: %v", err) + } + if got != "bw-secret" { + t.Errorf("Resolve = %q, want %q", got, "bw-secret") + } + + // Plain values pass through untouched. + got, err = registry.Resolve("literal") + if err != nil { + t.Fatalf("Resolve returned error: %v", err) + } + if got != "literal" { + t.Errorf("Resolve = %q, want %q", got, "literal") + } +} + +func TestRegistryResolveErrorNamesProvider(t *testing.T) { + op := &fakeProvider{name: "1Password", scheme: SchemeOnePassword, resolveErr: errors.New("boom")} + registry := NewRegistry(op) + + _, err := registry.Resolve("op://a/b/c") + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), "1Password") { + t.Errorf("error %q does not name the provider", err) + } +} + +func TestRegistryByScheme(t *testing.T) { + registry, _, bw := newTestRegistry() + + got, ok := registry.ByScheme(SchemeBitwarden) + if !ok || got != bw { + t.Errorf("ByScheme(%q) = %v, want Bitwarden provider", SchemeBitwarden, got) + } + + if _, ok := registry.ByScheme("nope"); ok { + t.Error("ByScheme returned a provider for an unknown scheme") + } +} + +func TestIsKnownReference(t *testing.T) { + tests := map[string]bool{ + "op://Private/Server/password": true, + "bw://8f3c": true, + "enc:AAAA": false, + "hunter2": false, + "": false, + "opsomething": false, + } + + for value, want := range tests { + if got := IsKnownReference(value); got != want { + t.Errorf("IsKnownReference(%q) = %v, want %v", value, got, want) + } + } +} + +func TestKnownSchemesIsACopy(t *testing.T) { + schemes := KnownSchemes() + if len(schemes) == 0 { + t.Fatal("expected at least one known scheme") + } + schemes[0] = "mutated" + + if KnownSchemes()[0] == "mutated" { + t.Error("KnownSchemes returned the underlying slice") + } +}