Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide passwords from MacOS clipboard history #1816

Closed
rianadon opened this issue Feb 26, 2021 · 8 comments · Fixed by #2065
Closed

Hide passwords from MacOS clipboard history #1816

rianadon opened this issue Feb 26, 2021 · 8 comments · Fixed by #2065
Assignees
Labels
feature Enhancements and new features
Milestone

Comments

@rianadon
Copy link
Contributor

Passwords I copy appear in Alfred's clipboard history, and I imagine they would appear in other MacOS clipboard history managers like Maccy as well.

The standard seems to be that apps can set org.nspasteboard.ConcealedType when copying (which also requires using the Mac API). From an issue on bitwarden's tracker, I found a link to this PR on keepassxc which implemented the flag: https://github.com/keepassxreboot/keepassxc/pull/1202/files.

I know gopass uses attoto's module for clipboard access - so it might be worth adding the option upstream. However the module is made tiny by plugging directly to pbcopy, so using the clipboard api instead would be adding a lot of code; it might be worth adding mac-specific clipboard implementation here. Either way, I'm happy to code up a draft.

The alternative would be something like this open PR on the alfred integration that uses Alfred to do the copying, but this only fixes the issue for those who use Alfred to copy passwords.

@AnomalRoil
Copy link
Member

If you're willing to send a PR, we would be fine with it, IMO.

Just note that we have a native Go only policy. You can take a look at our contribution guidelines and note we are using Go 1.16.

@dominikschulz dominikschulz added the feature Enhancements and new features label Feb 26, 2021
@dominikschulz
Copy link
Member

It seems like the content hints are only available through the ObjectiveC bindings?
The problem is that this would require CGO and we can't support that without major changes to our release process.

Maybe at some point we need an OSX-specific helper binary with it's own release process?

@rianadon
Copy link
Contributor Author

Yes it would eventually have to go through ObjectiveC bindings; I was thinking of using MacDriver as a layer between, which I don't think would require changes to the release process but is yet another module to pull in.

But then I started thinking there must be a way to accomplish this from the command line, then realized applescript has access to Objective C apis so hacked together this command (where the last argument to osascript is the password):

osascript \
  -e 'use framework "Foundation"' \
  -e "on run argv" \
  -e "set type to current application's NSPasteboardTypeString" \
  -e "set pb to current application's NSPasteboard's generalPasteboard()" \
  -e "pb's clearContents()" \
  -e "pb's setString:\"\" forType:\"org.nspasteboard.ConcealedType\"" \
  -e "pb's setString:(item 1 of argv) forType:type" \
  -e "end run" \
  secretpassword

Disclaimer: I've only tested this on Big Sur, and have no idea how robust of a solution this is.

This approach is closer to how gopass spawns pbcopy right now, though this shell script is 50 times as long.

@dominikschulz
Copy link
Member

Your osascript approach looks good (but I'm not very familiar with osascript) and we could easily switch to that.

The MacDriver module seems to require ObjectiveC bindings and since we build with CGO_ENABLED=false (because goreleaser doesn't support it / can't support it) it wouldn't compile a release with any MacDriver dependency.

I think we could either use the osascript approach or consider having our own Mac-only helper.

@dominikschulz
Copy link
Member

@rianadon If you could add a little bit of documentation to your osascript snippet I could quickly produce a PR to use that instead of attoto/clipboard on MacOS (or feel free to send a PR yourself, if you want).

@rianadon
Copy link
Contributor Author

I wasn't sure how to shift things around to make a special case for copying on MacOS, so I'd appreciate if you could make the PR! I'll send a commented snippet for the command.

Before I do that I'm going to see if I can give the password to osascript in some way other than a command line argument, as the password might show up in ps for the brief moment the script is run.

@rianadon
Copy link
Contributor Author

rianadon commented Mar 1, 2021

Thank you for your guidance on the issue @dominikschulz! I made two snippet versions: one launches osascript with the script as arguments (which means each line of the script is prefixed with -e) and another which passes the script through standard input. In both cases, I pass in the password through file descriptor 3, which is a hack used to get around Applescript's limitation that it cannot read stdin. That way the password isn't leaked to ps like in my first snippet 😁

I don't think there are any important distinctions between the two methods other than that the former saves us from having to deal with stdin, while the latter doesn't have -e on every line of the script.

Script passed through arguments

args := []string{
	// The Foundation library has the Objective C symbols for pasteboard
	"-e", `use framework "Foundation"`,
	// Need to use scripting additions for access to "do shell script"
	"-e", "use scripting additions",
	// type = a reference to the ObjC constant NSPasteboardTypeString
	// which is needed to indentify clioboard contents as text
	"-e", "set type to current application's NSPasteboardTypeString",
	// pb = a reference to the system's pasteboard
	"-e", "set pb to current application's NSPasteboard's generalPasteboard()",
	// Must clear contents before adding a new item to pasteboard
	"-e", "pb's clearContents()",
	// Set the flag ConcealedType so clipboard history mangers don't record the password.
	// The first argument can by anything, but an empty string will do fine.
	"-e", `pb's setString:"" forType:"org.nspasteboard.ConcealedType"`,
	// AppleScript cannot read from stdin, so pipe fd#3 to stdin of cat and read the output.
	// This output is put in the clipboard, setting type = string type
	"-e", `pb's setString:(do shell script "cat 0<&3") forType:type`,
}
cmd := exec.Command("osascript", args...)
r, w, errpipe := os.Pipe()
cmd.ExtraFiles = []*os.File{r} // Receiving end of pipes is connected to fd#3
go func() {
	defer w.Close()
	io.WriteString(w, password) // Write the password to fd#3
}()
out, errcmd := cmd.Output()
// osascript should print true (return value of the last setString call) on success
if errpipe != nil || errcmd != nil || string(out) != "true\n" {
	// Fallback to using attoto's pbcopy
}

Script passed through stdin

script := strings.Join([]string{
	// The Foundation library has the Objective C symbols for pasteboard
	`use framework "Foundation"`,
	// Need to use scripting additions for access to "do shell script"
	"use scripting additions",
	// type = a reference to the ObjC constant NSPasteboardTypeString
	// which is needed to indentify clioboard contents as text
	"set type to current application's NSPasteboardTypeString",
	// pb = a reference to the system's pasteboard
	"set pb to current application's NSPasteboard's generalPasteboard()",
	// Must clear contents before adding a new item to pasteboard
	"pb's clearContents()",
	// Set the flag ConcealedType so clipboard history mangers don't record the password.
	// The first argument can by anything, but an empty string will do fine.
	`pb's setString:"" forType:"org.nspasteboard.ConcealedType"`,
	// AppleScript cannot read from stdin, so pipe fd#3 to stdin of cat and read the output.
	// This output is put in the clipboard, setting type = string type
	`pb's setString:(do shell script "cat 0<&3") forType:type`,
}, "\n")
cmd := exec.Command("osascript")
r, w, errpipe := os.Pipe() // Receiving end of pipes is connected to fd#3
stdin, errstdin := cmd.StdinPipe()
cmd.ExtraFiles = []*os.File{r}
go func() {
	defer stdin.Close()
	defer w.Close()
	// Write the script to stdin and the password to the pipe, which is passed to fd #3
	io.WriteString(stdin, script)
	io.WriteString(w, password)
}()
out, errcmd := cmd.Output()
// osascript should print true (return value of the last setString call) on success
if errpipe != nil || errcmd != nil || errstdin != nil || string(out) != "true\n" {
	// Fallback to using attoto's pbcopy
}

@dominikschulz
Copy link
Member

Wow, that looks nice. I'll need a bit to digest that a figure out which one I like better, but then I'll create a PR for that.

@dominikschulz dominikschulz self-assigned this Mar 1, 2021
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Dec 24, 2021
This commit finally incorporates the changes suggested by rianadon in

Additionally it switches the unclip checksum from SHA256 to Argon2id.
Although the risk is close to zero (i.e. if someone can access our
process env we're doomed anyway) but there is just no reason to not
do it properly.

RELEASE_NOTES=[ENHANCEMENT] Hide password on MacOS clipboards

Fixes gopasspw#1816

Signed-off-by: Dominik Schulz <[email protected]>
@dominikschulz dominikschulz added this to the 1.14.0 milestone Dec 24, 2021
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Dec 24, 2021
This commit finally incorporates the changes suggested by rianadon in

Additionally it switches the unclip checksum from SHA256 to Argon2id.
Although the risk is close to zero (i.e. if someone can access our
process env we're doomed anyway) but there is just no reason to not
do it properly.

RELEASE_NOTES=[ENHANCEMENT] Hide password on MacOS clipboards

Fixes gopasspw#1816

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Dec 28, 2021
This commit finally incorporates the changes suggested by rianadon in

Additionally it switches the unclip checksum from SHA256 to Argon2id.
Although the risk is close to zero (i.e. if someone can access our
process env we're doomed anyway) but there is just no reason to not
do it properly.

RELEASE_NOTES=[ENHANCEMENT] Hide password on MacOS clipboards

Fixes gopasspw#1816

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Dec 28, 2021
This commit finally incorporates the changes suggested by rianadon in

Additionally it switches the unclip checksum from SHA256 to Argon2id.
Although the risk is close to zero (i.e. if someone can access our
process env we're doomed anyway) but there is just no reason to not
do it properly.

RELEASE_NOTES=[ENHANCEMENT] Hide password on MacOS clipboards

Fixes gopasspw#1816

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit that referenced this issue Dec 28, 2021
This commit finally incorporates the changes suggested by rianadon in

Additionally it switches the unclip checksum from SHA256 to Argon2id.
Although the risk is close to zero (i.e. if someone can access our
process env we're doomed anyway) but there is just no reason to not
do it properly.

RELEASE_NOTES=[ENHANCEMENT] Hide password on MacOS clipboards

Fixes #1816

Signed-off-by: Dominik Schulz <[email protected]>
kpitt pushed a commit to kpitt/gopass that referenced this issue Jul 21, 2022
This commit finally incorporates the changes suggested by rianadon in

Additionally it switches the unclip checksum from SHA256 to Argon2id.
Although the risk is close to zero (i.e. if someone can access our
process env we're doomed anyway) but there is just no reason to not
do it properly.

RELEASE_NOTES=[ENHANCEMENT] Hide password on MacOS clipboards

Fixes gopasspw#1816

Signed-off-by: Dominik Schulz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Enhancements and new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants