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

Check if server is nil before passing pointer to go-keyring #322

Closed
wants to merge 1 commit into from

Conversation

Laevos
Copy link
Contributor

@Laevos Laevos commented Jan 29, 2024

I'm running into an issue that appears to be related to zalando/go-keyring#88, which I describe in zalando/go-keyring#88 (comment). The gist of the issue is that an incompatibility between go-keyring and KeepassXC causes a segfault in the SetServerPassword method as the ServerConfig isn't properly initialized.

2024/01/29 03:11:41 error getting password from keyring: org.freedesktop.Secret.Error.IsLocked
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x5e33a2]

goroutine 79 [running]:
github.com/zalando/go-keyring/secret_service.(*SecretService).handlePrompt(0xc0091ef050, {0xc000462380?, 0x2c?})
	github.com/zalando/[email protected]/secret_service/secret_service.go:197 +0x122
github.com/zalando/go-keyring/secret_service.(*SecretService).CreateItem(0xd69990?, {0xe6cdb0, 0xc0089842d0}, {0xc009084780, 0x43}, 0xc00a337620, {{0xc008e7d1d0, 0x41}, {0x1b1af00, 0x0, ...}, ...})
	github.com/zalando/[email protected]/secret_service/secret_service.go:175 +0x370
github.com/zalando/go-keyring.secretServiceProvider.Set({}, {0xd1127f, 0xa}, {0xc0089b61b0, 0x24}, {0xc008402c80, 0x40})
	github.com/zalando/[email protected]/keyring_unix.go:43 +0x3be
github.com/zalando/go-keyring.Set(...)
	github.com/zalando/[email protected]/keyring.go:27
github.com/dweymouth/supersonic/backend.(*ServerManager).SetServerPassword(0xc0000fb900, 0xc008813eb0?, {0xc008402c80, 0x40})
	github.com/dweymouth/supersonic/backend/servermanager.go:159 +0xb6
github.com/dweymouth/supersonic/ui/controller.(*Controller).trySetPasswordAndConnectToServer(0xc000582300, 0x8?, {0xc008402c80, 0x40})
	github.com/dweymouth/supersonic/ui/controller/controller.go:544 +0x3c
github.com/dweymouth/supersonic/ui/controller.(*Controller).PromptForLoginAndConnect.func1.1()
	github.com/dweymouth/supersonic/ui/controller/controller.go:376 +0x178
created by github.com/dweymouth/supersonic/ui/controller.(*Controller).PromptForLoginAndConnect.func1 in goroutine 84
	github.com/dweymouth/supersonic/ui/controller/controller.go:368 +0x105

As there's currently no ETA for the upstream issue being fixed, I propose the following solution to prevent a segfault in the event that this or any other issue crops up which causes the ServerConfig to be nil. Since trySetPasswordAndConnectToServer() can gracefully handle an error in SetServerPassword(), this seems like it should be a safe fix since we can just store the password in-memory.

@Laevos Laevos force-pushed the check-server-pointer branch from cee6f01 to 932b889 Compare January 29, 2024 12:14
@Laevos
Copy link
Contributor Author

Laevos commented Jan 29, 2024

Ah, just realized this doesn't fix the issue; it's not that the ServerConfig is nil, otherwise we wouldn't even get into the keyring-go code before we hit a nil dereference, so unfortunately the fix seems like it'll have to be upstream, apologies 😔

@Laevos Laevos closed this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant