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.

Sign up for GitHub

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

Jump to bottom

split-gpg2 should accept (and ignore) NOP command #9483

Open
DemiMarie opened this issue Sep 30, 2024 · 12 comments · May be fixed by QubesOS/qubes-app-linux-split-gpg2#17
Open

split-gpg2 should accept (and ignore) NOP command #9483

DemiMarie opened this issue Sep 30, 2024 · 12 comments · May be fixed by QubesOS/qubes-app-linux-split-gpg2#17
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: split-gpg2 split-gpg version 2 diagnosed Technical diagnosis has been performed (see issue comments). more info needed Requires more information before a diagnosis can be made or action can be taken. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. waiting for upstream This issue is waiting for something from an upstream project to arrive in Qubes. Remove when closed.

Comments

@DemiMarie
Copy link

How to file a helpful issue

Qubes OS release

R4.2

Brief summary

split-gpg2 does not work with Sequoia Chameleon.

Steps to reproduce

Use split-gpg2 with Sequoia Chameleon.

Expected behavior

Works

Actual behavior

Fails. Part of this (not supporting NOP) is fixed by QubesOS/qubes-app-linux-split-gpg2#17 but the failure mode is now “unusable secret key” instead of the connection being dropped.
@DemiMarie DemiMarie added T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. C: split-gpg2 split-gpg version 2 labels Sep 30, 2024
@andrewdavidwong andrewdavidwong added needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. affects-4.2 This issue affects Qubes OS 4.2. labels Sep 30, 2024
@h01ger
Copy link

h01ger commented Sep 30, 2024

after I shared this issue on on #sequoia on irc.ofct.net, upstream developer neal said:

| h01ger: ideally the chameleon should be fixed rather than client software
or if the client software is going to be fixed, perhaps it should use a completely different interface

@h01ger
Copy link

h01ger commented Sep 30, 2024

filed upstream as https://gitlab.com/sequoia-pgp/sequoia-chameleon-gnupg/-/issues/96 now.

@h01ger
Copy link

h01ger commented Sep 30, 2024 •
edited
Loading

upstream asks for more info as grepping GnuPG's source revealed that GnuPG used to have a nop action, but it was removed in 1998...

@andrewdavidwong andrewdavidwong added waiting for upstream This issue is waiting for something from an upstream project to arrive in Qubes. Remove when closed. more info needed Requires more information before a diagnosis can be made or action can be taken. diagnosed Technical diagnosis has been performed (see issue comments). and removed needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. labels Oct 1, 2024
@DemiMarie
Copy link
Author

I created a separate test qube, with worthless keys, so I could enable debug logging. This revealed the following:

  • KEYINFO takes --list, but split-gpg2 wrongly rejected this, breaking --list-secret-keys. It does not work after the fix either, but returns no keys instead of a "command filtered out" error.
  • Sequoia Chameleon sends OPTION pinentry-mode=ask which split-gpg2 should accept (it’s a no-op) but instead returns an error on.
  • If key generation is not enabled, which is the default behavior, split-gpg2 connects to the restricted agent socket, rather than the non-restricted one.
  • Sequoia Chameleon doesn’t check for a restricted socket, sends commands that gpg-agent rejects, and disconnects when it receives an error.

DemiMarie added a commit to DemiMarie/qubes-app-linux-split-gpg2 that referenced this issue Oct 22, 2024
@DemiMarie
Never use a restricted agent connection
a03a211 
Enumerating secret keys with "KEYINFO --list" does not work over a
restricted connection.  As a result, gpg prints
"gpg: problem with fast path key listing: Forbidden - ignored", which
Mutt interprets as a prompt the user must respond to.  This causes the
user to need to press enter twice to send a signed email.  Sequoia
Chameleon does not implement the fallback and is unable to list secret
keys or decrypt messages.  The filtering done by split-gpg2 is far
stronger than what gpg-agent does, so there is no loss of security.

Fixes: QubesOS/qubes-issues#9483
@DemiMarie DemiMarie changed the title split-gpg2 does not work with Sequoia Chameleon split-gpg2 should accept (and ignore) NOP command Oct 22, 2024
This was referenced Oct 22, 2024
Open
Open
DemiMarie added a commit to DemiMarie/qubes-app-linux-split-gpg2 that referenced this issue Oct 22, 2024
@DemiMarie
Ignore NOP command
f4bfddb 
This command is used by Sequoia Chameleon.

Fixes: QubesOS/qubes-issues#9483.
@marmarek
Copy link
Member

If key generation is not enabled, which is the default behavior, split-gpg2 connects to the restricted agent socket, rather than the non-restricted one.

That's extra defense that @HW42 added, and it IMO makes sense. Maybe make it configurable, so user can still use this extra filtering with compatible applications?

@DemiMarie DemiMarie linked a pull request Oct 22, 2024 that will close this issue
Open
@DemiMarie
Copy link
Author

If key generation is not enabled, which is the default behavior, split-gpg2 connects to the restricted agent socket, rather than the non-restricted one.

That's extra defense that @HW42 added, and it IMO makes sense. Maybe make it configurable, so user can still use this extra filtering with compatible applications?

I’d prefer for the default connection to be unrestricted, mostly because I expect gpg-sq to be much safer than gpg and I don’t want to discourage users from switching to it.

@marmarek
Copy link
Member

I’d prefer for the default connection to be unrestricted, mostly because I expect gpg-sq to be much safer than gpg and I don’t want to discourage users from switching to it.

From split-gpg2 perspective, it's more important what is used as the backend (gpg-agent side), not the frontend. And here IIUC sequoia can't help (yet?).
That said, even with the unrestricted socket it's probably safe enough. @HW42 what do you think?

@h01ger
Copy link

h01ger commented Oct 23, 2024 via email

@marmarek
Copy link
Member

marmarek commented Oct 23, 2024 •
edited
Loading

AFAIK sequoia does not implement gpg-agent itself (it can just talk to the GnuPG's gpg-agent).

@HW42
Copy link

HW42 commented Oct 23, 2024

From split-gpg2 perspective, it's more important what is used as the backend (gpg-agent side), not the frontend. And here IIUC sequoia can't help (yet?).
That said, even with the unrestricted socket it's probably safe enough. @HW42 what do you think?

Unless the functionality is really needed I would prefer to use the restricted socket. The protocol filtering approach has the risk of the filter and gpg interpreting things differently. In such case using the restricted socket has the big advantages that you would need another bug in gpg (that's also accessible via the filter) to be able to extract a secret.

Why does sequoia need the unrestricted socket?

Even with a agent implementation by sequoia this would only address memory safety issues, not necessarily other bugs. OTOH depending on how nicely their code is to adopt, it might be easiert to implement a restricted agent with their code, thereby avoiding the protocol confusion problem altogether.

@DemiMarie
Copy link
Author

From split-gpg2 perspective, it's more important what is used as the backend (gpg-agent side), not the frontend. And here IIUC sequoia can't help (yet?).
That said, even with the unrestricted socket it's probably safe enough. @HW42 what do you think?

Unless the functionality is really needed I would prefer to use the restricted socket. The protocol filtering approach has the risk of the filter and gpg interpreting things differently. In such case using the restricted socket has the big advantages that you would need another bug in gpg (that's also accessible via the filter) to be able to extract a secret.

I just looked through the code to see if there were any such cases. I believe there are none (which is good), but it wasn’t trivial to check this (which is bad).

Why does sequoia need the unrestricted socket?

Sequoia Chameleon only knows how to list secret keys via KEYINFO --list, not to query each secret key separately with the version of KEYINFO taking a keygrip. The restricted socket connection doesn’t allow this. I flied an issue with Sequoia Chameleon: https://gitlab.com/sequoia-pgp/sequoia-chameleon-gnupg/-/issues/102

Even with a agent implementation by sequoia this would only address memory safety issues, not necessarily other bugs. OTOH depending on how nicely their code is to adopt, it might be easiert to implement a restricted agent with their code, thereby avoiding the protocol confusion problem altogether.

In the long term I agree. In the short term, I wonder if a workaround could be used: make a separate unrestricted agent connection and send KEYINFO --list there. This command is sufficiently trivial that it should not be possible to mess it up.

@HW42
Copy link

HW42 commented Oct 23, 2024 •
edited
Loading

I flied an issue with Sequoia Chameleon: https://gitlab.com/sequoia-pgp/sequoia-chameleon-gnupg/-/issues/102

Thanks. Given that in the code comment you mentioned that the warning message from gpg confuses for example mutt it might be worth taking it upstream. Both for the specific case, but also maybe it would make sense to reevaluate if it's worth blocking this command.

In the short term, I wonder if a workaround could be used: make a separate unrestricted agent connection and send KEYINFO --list there. This command is sufficiently trivial that it should not be possible to mess it up.

That's an interesting idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: split-gpg2 split-gpg version 2 diagnosed Technical diagnosis has been performed (see issue comments). more info needed Requires more information before a diagnosis can be made or action can be taken. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. waiting for upstream This issue is waiting for something from an upstream project to arrive in Qubes. Remove when closed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix use with Sequoia Chameleon DemiMarie/qubes-app-linux-split-gpg2
5 participants
@marmarek @h01ger @DemiMarie @andrewdavidwong @HW42