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

feat: Support YubiKeys in KeePassXC open mode #3911

Merged
merged 1 commit into from
Aug 27, 2024
Merged

feat: Support YubiKeys in KeePassXC open mode #3911

merged 1 commit into from
Aug 27, 2024

Conversation

twpayne
Copy link
Owner

@twpayne twpayne commented Aug 20, 2024

Fixes #3910.

@mihakrumpestar would you be able to test this? tl;dr replace keepassxc.args with keepassxc.openArgs in your config file and it should work.

It's not easy for me to write a test for it because I don't use KeePassXC, don't have a YubiKey, and the "open" mode has two processes communicating with each other through a terminal.

Copy link
Collaborator

@halostatue halostatue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me, but I have the same problem as you for testing.

@mihakrumpestar
Copy link

mihakrumpestar commented Aug 20, 2024

Thanks for the quick response.

I tested it and the arguments and the command are in correct order, but unfortunately chezmoi does not wait for confirmation (user has to physically touch the device/key) to confirm authentication.

Here is the debug output:

time=2024-08-21T00:13:30.841+02:00 level=INFO msg=Start cmd="/home/<user>/.local/share/devbox/global/default/.devbox/nix/profile/default/bin/keepassxc-cli open --no-password --yubikey 1:2343434 /home/<user>/Documents/digital-identity/KeePass.kdbx" start=2024-08-21T00:13:30.841+02:00 err=<nil>
chezmoi: .config/rclone/rclone.conf: template: dot_config/rclone/rclone.conf.tmpl:2:3: executing "dot_config/rclone/rclone.conf.tmpl" at <keepassxcAttribute "rclone-config" "rclone.conf">: error calling keepassxcAttribute: Please present or touch your YubiKey to continue.

Steps: Immediately after the message, chezmoi exits, while the keys confirmation light is on (meaning it is waiting for confirmation).
This does work when running the above command ("keepassxc-cli") standalone.

Edit: It did wait for the confirmation in using non "open" mode in prev version.

Edit 2: Here is the output for the standard show using the new build (is the same as before):

time=2024-08-21T00:26:47.827+02:00 level=INFO msg=ReadFile component=system name=/home/<user>miha/repos/personal/dotfiles/home/dot_config/rclone/rclone.conf.tmpl size=98 data="{{ if not (env \"ENCRYPTED\") -}}\n{{ keepassxcAttribute \"rclone-co..."
Hardware key error: operation would block

On Hardware key error: operation would block it was waiting for my confirmation with chezmoi.

@twpayne
Copy link
Owner Author

twpayne commented Aug 20, 2024

I tested it and the arguments and the command are in correct order, but unfortunately chezmoi does not wait for confirmation (user has to physically touch the device/key) to confirm authentication.

Any clue as to what options or output KeePassXC gives before it can accept further commands? The relevant code is at

// Create the console if it is not already created.
if c.Keepassxc.console == nil {
// Create the console.
console, err := expect.NewConsole()
if err != nil {
return nil, err
}
// Start the keepassxc-cli open command.
cmdArgs := slices.Clone(c.Keepassxc.Args)
cmdArgs = append(cmdArgs, "open")
cmdArgs = append(cmdArgs, c.Keepassxc.OpenArgs...)
cmdArgs = append(cmdArgs, c.Keepassxc.Database.String())
cmd := exec.Command(c.Keepassxc.Command, cmdArgs...) //nolint:gosec
env := os.Environ()
// Ensure prompt is in English.
env = append(env, "LANGUAGE=en")
// Reduce injection of terminal control characters.
env = slices.DeleteFunc(env, func(s string) bool {
return strings.HasPrefix(s, "TERM=")
})
cmd.Env = env
cmd.Stdin = console.Tty()
cmd.Stdout = console.Tty()
cmd.Stderr = console.Tty()
if err := chezmoilog.LogCmdStart(c.logger, cmd); err != nil {
return nil, err
}
if c.Keepassxc.Prompt {
// Expect the password prompt, e.g. "Enter password to unlock $HOME/Passwords.kdbx: ".
enterPasswordToUnlockPrompt, err := console.Expect(
expect.Regexp(keepassxcEnterPasswordToUnlockDatabaseRx),
expect.Regexp(keepassxcAnyResponseRx),
)
if err != nil {
return nil, err
}
if !keepassxcEnterPasswordToUnlockDatabaseRx.MatchString(enterPasswordToUnlockPrompt) {
return nil, errors.New(strings.TrimSpace(enterPasswordToUnlockPrompt))
}
// Read the password from the user, if necessary.
var password string
if c.Keepassxc.password != "" {
password = c.Keepassxc.password
} else {
password, err = c.readPassword(enterPasswordToUnlockPrompt)
if err != nil {
return nil, err
}
}
// Send the password.
if _, err := console.SendLine(password); err != nil {
return nil, err
}
// Wait for the end of the password prompt.
if _, err := console.ExpectString("\r\n"); err != nil {
return nil, err
}
}
// Read the prompt, e.g "Passwords> ", so we can expect it later.
output, err := console.Expect(
expect.Regexp(keepassxcPromptRx),
expect.Regexp(keepassxcAnyResponseRx),
)
if err != nil {
return nil, err
}
if !keepassxcPromptRx.MatchString(output) {
return nil, errors.New(strings.TrimSpace(output))
}
c.Keepassxc.cmd = cmd
c.Keepassxc.console = console
c.Keepassxc.prompt = keepassxcPromptRx.FindString(output)
}
.

Edit: link to code in this PR.

@mihakrumpestar
Copy link

Any clue as to what options or output KeePassXC gives before it can accept further commands? The relevant code is at

When doing both show and open in standalone keepassxc-cli, both show Please present or touch your YubiKey to continue..

So, how it gets to Hardware key error: operation would block is a mystery.

As for the code, I don't have any clues. I did look up, and it seems it is not that trivial, this thread (last comment in particular) might give some hints keepassxreboot/keepassxc#8969. Unfortunately, I am not that familiar with bash scripting to give an answer.

@mihakrumpestar
Copy link

mihakrumpestar commented Aug 20, 2024

I did test right now with the template command:

  • Regular mode:

    keepassxc:
      database: "~/Documents/digital-identity/KeePass.kdbx"
      args: ["--no-password", "--yubikey", "1:1097617"]
      prompt: false
    $ /usr/local/bin/chezmoi execute-template '{{ keepassxcAttribute "git-config" "gitconfig" }}' --verbose
    Hardware key error: operation would block
    [url "ssh://git@...

    Works as expected (block execution and then unlocks database).

  • Open mode:

    keepassxc:
      database: "~/Documents/digital-identity/KeePass.kdbx"
      openArgs: ["--no-password", "--yubikey", "1:1097617"]
      mode: "open"
      prompt: false
    $ /usr/local/bin/chezmoi execute-template '{{ keepassxcAttribute "git-config" "gitconfig" }}' --verbose
    chezmoi: template: arg1:1:3: executing "arg1" at <keepassxcAttribute "git-config" "gitconfig">: error calling keepassxcAttribute: Please present or touch your YubiKey to continue.

    Does not block execution and immediately exits with YubiKey waiting for confirmation.

    Now, based on this error calling keepassxcAttribute: Please present or touch your YubiKey to continue. it treats this wait as error already.

@twpayne
Copy link
Owner Author

twpayne commented Aug 25, 2024

Thanks for the testing!

Note that chezmoi already uses expect to talk to keepassxc-cli in open mode. This means that chezmoi provides a virtual interactive terminal to keepassxc-cli.

The reason for this error message:

error calling keepassxcAttribute: Please present or touch your YubiKey to continue.

is that chezmoi was expecting an Enter password to unlock... message from keepassxc-cli, but received the Please present or touch... message instead.

I've pushed an second commit to PR that makes chezmoi expect the Please present or touch... message, and also improved the error messages that chezmoi returns when it encounters something that it doesn't expect.

Would you be able to test this?

I don't have a recent YubiKey or use KeePassXC, so this might take a few iterations to get right with you doing the testing and me writing the code. I hope you're OK with that - I'd like to get this working :)

@twpayne
Copy link
Owner Author

twpayne commented Aug 25, 2024

Minor update: I found a YubiKey 5C NFC in my box of stuff and have set up KeePassXC to work with this. So, I can now test locally.

@mihakrumpestar
Copy link

mihakrumpestar commented Aug 26, 2024

I was trying to debug using latest commit:

/usr/local/bin/chezmoi --version
chezmoi version dev, commit 2b2a25d3b242b13b7245dc48d524489ca4d0130d, built at 2024-08-25T22:20:42Z

# I added some modifications on how the outputs are here/usr/local/bin/chezmoi execute-template '{{ keepassxcAttribute "git-config" "gitconfig" }}' --verbose
matched keepassxcPleasePresentOrTouchYourYubiKeyToContinueRx
not matched keepassxcPromptRx
chezmoi: template: arg1:1:3: executing "arg1" at <keepassxcAttribute "git-config" "gitconfig">: error calling keepassxcAttribute: "\r\n": unexpected prompt

The error error calling keepassxcAttribute happens on this line. I used strconv.Quote(output) to print escaped characters so it is properly readable in cmd.

I am not sure if we can change keepassxcPromptRx to just match any character.

Edit:

If I change to keepassxcPromptRx = regexp.MustCompile(`^.*>?\`) to account for these characters it does block and wait for prompt, but after confirmation, there is no output (KeePassXC entry output is missing). Execution probably proceeded further (while waiting for confirmation) and tried to get data from KeePassXC without it being unlocked yet.

@twpayne
Copy link
Owner Author

twpayne commented Aug 26, 2024

Thanks again for the testing!

I've updated this PR with new code which works locally for me.

My test config contains:

[keepassxc]
        database = "/home/twp/Passwords.kdbx"
        openArgs = ["--no-password", "--yubikey", "2:18281566"]
        mode = "open"

Example run:

$ chezmoi execute-template '{{ keepassxc "chezmoi test" | toJson }}'
Please present or touch your YubiKey to continue.
{"Notes":"","Password":"'rW*b25hhsP91a.*bX#C","Tags":"","Title":"chezmoi test","URL":"","UserName":"","Uuid":"{2d7c6567-1f44-4139-bafb-5f886b2e08b7}"}%                       

Would you be able to test this version?

@mihakrumpestar
Copy link

Tested and can confirm that it works. And is much faster than non open mode when deploying all the dotfiles.

Would it be possible to have just args instead of having additional openArgs? It seems redundant and definitely confusing, since in the keepassxc-cli, args and openArgs are just args.

@twpayne
Copy link
Owner Author

twpayne commented Aug 27, 2024

Would it be possible to have just args instead of having additional openArgs? It seems redundant and definitely confusing, since in the keepassxc-cli, args and openArgs are just args.

Yes, this makes sense because keepassxc-cli doesn't seem to need/want any arguments before open. I've updated the PR. Could you test this because I will not be able to test this until much later today.

@mihakrumpestar
Copy link

Tested and works as before. Thank you.

@twpayne twpayne changed the title feat: Add keepassxc.openArgs config var for keepassxc-cli in open mode eat: Support YubiKeys in KeePassXC open mode Aug 27, 2024
@twpayne twpayne changed the title eat: Support YubiKeys in KeePassXC open mode feat: Support YubiKeys in KeePassXC open mode Aug 27, 2024
@twpayne
Copy link
Owner Author

twpayne commented Aug 27, 2024

Great! I'll merge once the CI checks are complete. I'll likely do the next release of chezmoi towards the end of the week (the release cycle is roughly every two weeks).

@twpayne twpayne merged commit 7aee332 into master Aug 27, 2024
23 checks passed
@twpayne twpayne deleted the fix-3910 branch August 27, 2024 09:02
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KeepassXC mode: "open" does not work with args ["--yubikey", "1:2343434"]
3 participants