feat: Implement builtin mode for keepassxc #4149

bakito · 2024-12-20T19:31:16Z

Implements: #4148

twpayne

Thanks for this!

Instead of a new set of template functions, I think this different "mode" for opening KeepassXC databases. Currently chemzoi supports "cache-password" and "open" modes. I think this should be a new "builtin" mode.

What do you think?

.gitignore

go.mod

bakito · 2024-12-20T21:50:23Z

Thanks for this!

Instead of a new set of template functions, I think this different "mode" for opening KeepassXC databases. Currently chemzoi supports "cache-password" and "open" modes. I think this should be a new "builtin" mode.

What do you think?

Sounds reasonable.
I modified the implementation and added the functions for the new mode.

bakito · 2024-12-20T21:50:56Z

internal/cmd/keepassxctemplatefuncs.go

@@ -100,6 +113,18 @@ func (c *Config) keepassxcTemplateFunc(entry string) map[string]string {
 }

 func (c *Config) keepassxcAttributeTemplateFunc(entry, attribute string) string {
+	if c.Keepassxc.Mode == keepassxcModeBuiltin {
+		// builtin stores attributes in cache
+		if data, ok := c.Keepassxc.cache[entry]; ok {


As the library handles default values the same as attributes, both functions use the same cache in builtin mode.
I don't think that i makes sense to keep separate caches in this case.

bakito · 2024-12-20T21:53:11Z

internal/cmd/keepassxctemplatefuncs.go

@@ -329,3 +354,82 @@ func (c *Config) keepassxcClose() error {
 	}
 	return nil
 }
+
+// keepassxcBuiltinExtractValues extract builtin values
+func (c *Config) keepassxcBuiltinExtractValues(


This implementation reads all values from the keepass database to reduce access to the file itself.
If this might blow the memory I can change the implementation to only cache the requested values.

We don't know how large any given keepass database will be, so the memory is potentially uncontrollable this way. More problematic, though, is that this decrypts the entire database and puts it in memory. chezmoi doesn't do anything specific to harden its memory accesses (I’m not sure what one could do with Go), so this represents a much larger attack surface on a running instance of chezmoi.

If it isn't too much additional work, I think that only entries requested should be cached because to do otherwise would change chezmoi's security profile.

@halostatue @twpayne I've modified the code to only cache required entries

twpayne

Thank you, this is looking excellent, and I will do a full review after the weekend.

Please also check the contributing guide for low-hanging fruit.

But, thank you, this PR is excellent.

bakito · 2024-12-21T06:26:58Z

internal/cmd/keepassxctemplatefuncs_test.go

@@ -117,6 +117,7 @@ func TestKeepassxcTemplateFuncs(t *testing.T) {
 	for _, mode := range []keepassxcMode{
 		keepassxcModeCachePassword,
 		keepassxcModeOpen,
+		keepassxcModeBuiltin,


I'm having some issues on running this test.
When only keepassxcModeBuiltin mode is enabled, the test is running.
But when the other 2 are also enabled (or only the 2 other) the test fails.

I can't figure out the issue here. Might be some precondition in my environment.

Enter password to encrypt database (optional): Repeat password: Successfully created new database. Enter password to unlock /tmp/TestKeepassxcTemplateFuncs1798468072/001/KeePassXC Passwords.kdbx: Successfully added group test group. Enter password to unlock /tmp/TestKeepassxcTemplateFuncs1798468072/001/KeePassXC Passwords.kdbx: Enter password for new entry: Successfully added entry test entry. Enter password to unlock /tmp/TestKeepassxcTemplateFuncs1798468072/001/KeePassXC Passwords.kdbx: Successfully imported attachment /tmp/TestKeepassxcTemplateFuncs1798468072/001/import file as test / attachment name to entry test group/test entry. error: Permission denied --- FAIL: TestKeepassxcTemplateFuncs (0.37s) --- FAIL: TestKeepassxcTemplateFuncs/cache-password (0.01s) --- FAIL: TestKeepassxcTemplateFuncs/cache-password/correct_password (0.01s) panic: /home/bakito/bin/keepassxc-cli show '/tmp/TestKeepassxcTemplateFuncs1798468072/001/KeePassXC Passwords.kdbx' --quiet --show-protected 'test group/test entry': exit status 1 [recovered] panic: /home/bakito/bin/keepassxc-cli show '/tmp/TestKeepassxcTemplateFuncs1798468072/001/KeePassXC Passwords.kdbx' --quiet --show-protected 'test group/test entry': exit status 1 goroutine 415 [running]: testing.tRunner.func1.2({0x17a2560, 0xc006f6e370}) /home/bakito/.gimme/versions/go1.23.4.linux.amd64/src/testing/testing.go:1632 +0x230 testing.tRunner.func1() /home/bakito/.gimme/versions/go1.23.4.linux.amd64/src/testing/testing.go:1635 +0x35e panic({0x17a2560?, 0xc006f6e370?}) /home/bakito/.gimme/versions/go1.23.4.linux.amd64/src/runtime/panic.go:785 +0x132 github.com/twpayne/chezmoi/v2/internal/cmd.mustValue[...](...) /home/bakito/go/src/github.com/bakito/chezmoi/internal/cmd/cmd.go:254 github.com/twpayne/chezmoi/v2/internal/cmd.(*Config).keepassxcTemplateFunc(0xc0073aa008, {0xc008243e18, 0x15}) /home/bakito/go/src/github.com/bakito/chezmoi/internal/cmd/keepassxctemplatefuncs.go:103 +0x24a github.com/twpayne/chezmoi/v2/internal/cmd.TestKeepassxcTemplateFuncs.func1.1(0xc0072c3380) /home/bakito/go/src/github.com/bakito/chezmoi/internal/cmd/keepassxctemplatefuncs_test.go:130 +0x1fa testing.tRunner(0xc0072c3380, 0xc00737b170) /home/bakito/.gimme/versions/go1.23.4.linux.amd64/src/testing/testing.go:1690 +0xf4 created by testing.(*T).Run in goroutine 414 /home/bakito/.gimme/versions/go1.23.4.linux.amd64/src/testing/testing.go:1743 +0x390

I think you've fixed this, right? I don't see this when I run the tests now.

It seems only to be an issue on my machine. As the pre-existing tests do not run.
If the, run all on your side, that's fine for me.

halostatue · 2024-12-23T21:12:28Z

assets/chezmoi.io/docs/reference/templates/keepassxc-functions/index.md

@@ -18,3 +18,6 @@ cache it for the duration of chezmoi's execution. Setting `keepassxc.mode` to
 `open` will tell chezmoi to instead open KeePassXC's console with `keepassxc-cli
 open` followed by `keepassxc.args`. chezmoi will use this console to request
 values from KeePassXC.
+When setting the `keepassxc.mode` to `builtin`, chezmoi uses a builtin library to
+access a keepassxc database. This mode becomes handy if the keepassxc-cli is not
+available.


When setting `keepassxc.mode` to `builtin`, chezmoi uses a builtin library to access a keepassxc database, which can be handy if `keepassxc-cli` is not available. Some features available in KeePassXC such as Yubikey-enhanced encryption, may not be available with builtin support.

Note the extra blank line at the top.

When setting `keepassxc.mode` to `builtin`, chezmoi uses a builtin library to access a keepassxc database, which can be handy if `keepassxc-cli` is not available. Some features available in KeePassXC such as Yubikey-enhanced encryption, may not be available with builtin support.

Note the extra blank line at the top.

Not sure what you mean wit that. What should be changed?

In your original text, you had two paragraphs right next to each other. They should be separated by a blank line.

With the rest of it, I am suggesting a change.

halostatue · 2024-12-23T21:17:33Z

internal/cmd/keepassxctemplatefuncs.go

@@ -329,3 +354,82 @@ func (c *Config) keepassxcClose() error {
 	}
 	return nil
 }
+
+// keepassxcBuiltinExtractValues extract builtin values
+func (c *Config) keepassxcBuiltinExtractValues(


We don't know how large any given keepass database will be, so the memory is potentially uncontrollable this way. More problematic, though, is that this decrypts the entire database and puts it in memory. chezmoi doesn't do anything specific to harden its memory accesses (I’m not sure what one could do with Go), so this represents a much larger attack surface on a running instance of chezmoi.

If it isn't too much additional work, I think that only entries requested should be cached because to do otherwise would change chezmoi's security profile.

twpayne · 2024-12-24T15:35:13Z

Thank you, this is looking excellent. Once you've made the small changes to the documentation, this should be ready to merge.

Signed-off-by: bakito <github@bakito.ch>

bakito mentioned this pull request Dec 20, 2024

Keepass support via go library #4148

Open

twpayne reviewed Dec 20, 2024

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

bakito commented Dec 20, 2024

View reviewed changes

bakito force-pushed the keepass branch 3 times, most recently from 5396dc8 to f42961b Compare December 20, 2024 22:20

twpayne reviewed Dec 20, 2024

View reviewed changes

bakito force-pushed the keepass branch from f42961b to f07e2a8 Compare December 21, 2024 06:05

bakito changed the title ~~[WIP] implement keepasslib as alternative to keepassxc~~ feat: Implement builtin mode for keepassxc Dec 21, 2024

bakito force-pushed the keepass branch from f07e2a8 to 53aa05d Compare December 21, 2024 06:22

bakito commented Dec 21, 2024

View reviewed changes

bakito marked this pull request as ready for review December 21, 2024 06:27

bakito force-pushed the keepass branch 3 times, most recently from 5e0b26c to a5f1457 Compare December 23, 2024 20:45

halostatue requested changes Dec 23, 2024

View reviewed changes

feat: Implement builtin mode for keepassxc

71adf5d

Signed-off-by: bakito <github@bakito.ch>

bakito force-pushed the keepass branch from a5f1457 to 71adf5d Compare December 28, 2024 07:42

cache only required entries

1531b03

Signed-off-by: bakito <github@bakito.ch>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement builtin mode for keepassxc #4149

feat: Implement builtin mode for keepassxc #4149

bakito commented Dec 20, 2024 •

edited

Loading

twpayne left a comment

bakito commented Dec 20, 2024

bakito Dec 20, 2024

bakito Dec 20, 2024

halostatue Dec 23, 2024

bakito Dec 28, 2024

twpayne left a comment

bakito Dec 21, 2024

twpayne Dec 24, 2024

bakito Dec 28, 2024 •

edited

Loading

halostatue Dec 23, 2024

bakito Dec 23, 2024

halostatue Dec 23, 2024

halostatue Dec 23, 2024

twpayne commented Dec 24, 2024

feat: Implement builtin mode for keepassxc #4149

Are you sure you want to change the base?

feat: Implement builtin mode for keepassxc #4149

Conversation

bakito commented Dec 20, 2024 • edited Loading

twpayne left a comment

Choose a reason for hiding this comment

bakito commented Dec 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twpayne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bakito Dec 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twpayne commented Dec 24, 2024

bakito commented Dec 20, 2024 •

edited

Loading

bakito Dec 28, 2024 •

edited

Loading