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

x/crypto/ssh/agent: keyring does not honor "ConfirmBeforeUse" #47533

Open
guahki opened this issue Aug 4, 2021 · 1 comment
Open

x/crypto/ssh/agent: keyring does not honor "ConfirmBeforeUse" #47533

guahki opened this issue Aug 4, 2021 · 1 comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@guahki
Copy link

guahki commented Aug 4, 2021

What version of Go are you using (go version)?

I was using the newest release of https://github.com/buptczq/WinCryptSSHAgent

Does this issue reproduce with the latest release?

As far as my code analysis goes: yes

What operating system and processor architecture are you using (go env)?

Windows 10, x64

What did you do?

TL;DR: realizing, the -c-flag of ssh-add is ignored by https://github.com/buptczq/WinCryptSSHAgent, while -t isn't.

for more details, please see buptczq/WinCryptSSHAgent#55

After opening the issue there, I further investigated and understood, the problem is in the library. Especially in the keyring-implementation of the ssh agent: https://github.com/golang/crypto/blob/a769d52b0f97a420f3dcafc17f8b3384217859a2/ssh/agent/keyring.go.

While https://github.com/golang/crypto/blob/a769d52b0f97a420f3dcafc17f8b3384217859a2/ssh/agent/keyring.go#L147-L148 says "Note that any constraints given are ignored.", the time constraint really isn't, as appropriate handling was added in golang/crypto@8e06e8d. However, the claim "and will ask the user to confirm a signing operation if ConfirmBeforeUse is set." in the commit description is not true: as far as I understand the code, no sch handling was/is included in the file.

What did you expect to see?

I expected, that either both flags are ignored/not supported (as common on windows in Pageant as well as Microsoft OpenSSH) or both flags are supported. At best of course the latest.

What did you see instead?

As described above: ssh-add -t is handled correctly, while ssh-add -c isn't.

@gopherbot gopherbot added this to the Unreleased milestone Aug 4, 2021
@seankhliao seankhliao changed the title x/crypto: ssh agent keyring does not honor "ConfirmBeforeUse" x/crypto/ssh/agent: keyring does not honor "ConfirmBeforeUse" Aug 5, 2021
@seankhliao
Copy link
Member

Looks like it was removed during review in https://golang.org/cl/28956 since that implementation relied on an external program, but the commit message and doc comment weren't updated.

@seankhliao seankhliao added Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants