From e8a1d73c2e15ae2f5c6502c8bdae3355d6e0c6f2 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sun, 29 Sep 2024 21:20:45 -0400 Subject: [PATCH 01/13] Fix tests for Ed448 and X448 keys GnuPG uses fingerprints for these keys that are 64 hex bytes, not 40 like for the other algorithms. Fix the tests to account for this. --- splitgpg2/test_server.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/splitgpg2/test_server.py b/splitgpg2/test_server.py index 75a9a77..abdabd7 100644 --- a/splitgpg2/test_server.py +++ b/splitgpg2/test_server.py @@ -171,7 +171,7 @@ def generate_key(self, *, subkey_usage: str = 'encrypt', client: bool = True) -> Tuple[bytes, bytes]: - fpr_re = re.compile(rb'\A[0-9A-F]{40}\Z') + fpr_re = re.compile(rb'\A[0-9A-F]{40}(?:[0-9A-F]{24})?\Z') email = 'a' + str(self.counter) + self.key_uid self.counter += 1 handle = base64.b64encode(os.urandom(32)).decode('ascii', 'strict') @@ -292,6 +292,7 @@ def do_test(ty: str, subkey_ty: str, key_param: Union[str, int], subkey_param: U do_test('ECDSA', 'ECDH', 'NIST P-521', 'NIST P-521') do_test('ECDSA', 'ECDH', 'secp256k1', 'secp256k1') do_test('EDDSA', 'ECDH', 'Ed25519', 'Curve25519') + do_test('EDDSA', 'ECDH', 'Ed448', 'X448') for i in curves: if i.startswith('ed'): From 39ca8cca795d7630167a6600e315c7af95950d9f Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 30 Sep 2024 14:42:38 -0400 Subject: [PATCH 02/13] Do not test Ed448 or X448 unless GnuPG says they are supported Debian does not support these algorithms. --- splitgpg2/test_server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/splitgpg2/test_server.py b/splitgpg2/test_server.py index abdabd7..517c542 100644 --- a/splitgpg2/test_server.py +++ b/splitgpg2/test_server.py @@ -292,7 +292,6 @@ def do_test(ty: str, subkey_ty: str, key_param: Union[str, int], subkey_param: U do_test('ECDSA', 'ECDH', 'NIST P-521', 'NIST P-521') do_test('ECDSA', 'ECDH', 'secp256k1', 'secp256k1') do_test('EDDSA', 'ECDH', 'Ed25519', 'Curve25519') - do_test('EDDSA', 'ECDH', 'Ed448', 'X448') for i in curves: if i.startswith('ed'): From f4bfddba81ec3db689e518b39ab8a2aa80a1ecf4 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sun, 29 Sep 2024 20:27:34 -0400 Subject: [PATCH 03/13] Ignore NOP command This command is used by Sequoia Chameleon. Fixes: QubesOS/qubes-issues#9483. --- splitgpg2/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index 7fc50a7..b9f7e12 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -544,6 +544,7 @@ def default_commands(self) -> Dict[bytes, 'NoneCallback']: b'BYE': self.command_BYE, b'SCD': self.command_SCD, b'READKEY': self.command_READKEY, + b'NOP': self.command_NOP, } @staticmethod @@ -909,6 +910,11 @@ async def command_SETKEYDESC(self, untrusted_args: Optional[bytes]) -> None: # pylint: disable=unused-argument self.fake_respond(b'OK') + async def command_NOP(self, untrusted_args: Optional[bytes]) -> None: + # Ignores all arguments. + # pylint: disable=unused-argument + self.fake_respond(b'OK') + async def command_PKDECRYPT(self, untrusted_args: Optional[bytes]) -> None: if untrusted_args is not None: raise Filtered From 58efb86bbd3dfb2fdddee912b125a4bf74d7043f Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 21 Oct 2024 20:36:39 -0400 Subject: [PATCH 04/13] Return fake 'OK' to setting 'display' option This option makes no sense in the context of split-gpg2 and fails if the gpg-agent-connection is restricted, causing Sequoia Chameleon to disconnect. Return a fake 'OK' response instead of passing the command to the agent. Fixes: QubesOS/qubes-issues#9527 --- splitgpg2/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index b9f7e12..0fb91c4 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -552,7 +552,7 @@ def default_options() -> Dict[bytes, Tuple[OptionHandlingType, Optional[bytes]]] return { b'ttyname': (OptionHandlingType.fake, b'OK'), b'ttytype': (OptionHandlingType.fake, b'OK'), - b'display': (OptionHandlingType.override, b':0'), + b'display': (OptionHandlingType.fake, b'OK'), b'lc-ctype': (OptionHandlingType.fake, b'OK'), b'lc-messages': (OptionHandlingType.fake, b'OK'), b'putenv': (OptionHandlingType.fake, b'OK'), From f388c33c2abf80f4d2daf403b1a07a4cd8fb5a41 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Wed, 23 Oct 2024 16:14:06 -0400 Subject: [PATCH 05/13] Remove OptionHandlingType.override Since 58efb86bbd3d ("Return fake 'OK' to setting 'display' option") it has no users. --- splitgpg2/__init__.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index 0fb91c4..b3bdc14 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -115,7 +115,6 @@ class OptionHandlingType(enum.Enum): # pylint: disable=invalid-name fake = 1 verify = 2 - override = 3 class HashAlgo: @@ -686,12 +685,7 @@ async def command_OPTION(self, untrusted_args: Optional[bytes]) -> None: except KeyError as e: raise Filtered from e - if action == OptionHandlingType.override: - if opts is not None: - option_arg = b'%s=%s' % (name, opts) - else: - option_arg = name - elif action == OptionHandlingType.verify: + if action == OptionHandlingType.verify: if callable(opts): verified = opts(untrusted_value=untrusted_value) elif isinstance(opts, Pattern): From 8efb735e68d166ef656029ec3a4da452b4895c0e Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 21 Oct 2024 20:47:36 -0400 Subject: [PATCH 06/13] Give fake OK response to OPTIONS pinentry-mode=ask pinentry-mode=ask is the default, so this is a no-op. Return OK instead of an error code. Sequoia Chameleon sends pinentry-mode=ask and disconnects when it gets an error. Fixes: QubesOS/qubes-issues#9528 --- splitgpg2/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index b3bdc14..7222e36 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -678,6 +678,11 @@ async def command_OPTION(self, untrusted_args: Optional[bytes]) -> None: if not untrusted_args: raise Filtered + if untrusted_args == b'pinentry-mode=ask': + # This is the default and a no-op + self.fake_respond(b'OK') + return + untrusted_name, untrusted_value = extract_args(untrusted_args, b'=') try: action, opts = self.options[untrusted_name] From 1010282acbf7a7f5530cd5b45178f15acbd47d01 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Tue, 22 Oct 2024 11:08:02 -0400 Subject: [PATCH 07/13] Fix bytes vs str mismatch This bug is old, but it only triggers if there are no UIDs, which is why testing didn't reveal it. I suspect old versions of mypy just did not catch the bug. --- splitgpg2/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index 7222e36..410c075 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -809,7 +809,9 @@ async def setkeydesc(self, keygrip: bytes) -> None: assert key is not None, 'no key?' desc = b'%s\nFingerprint: %s%s' % ( - (b'UID: ' + key.first_uid.split(b'\n')[0]) if key.first_uid is not None else '', + (b'UID: ' + key.first_uid.split(b'\n')[0]) + if key.first_uid is not None + else b'', key.fingerprint, subkey_desc) From 5b8e483c359dfb2330bb8df5da61ba7e781949d4 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 24 Oct 2024 23:04:56 -0400 Subject: [PATCH 08/13] Properly indent a function parameter No functional change intended. --- splitgpg2/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index 410c075..17f09bb 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -1276,7 +1276,7 @@ def validate_keygen_sexp(*, untrusted_sexp: 'SExpr') -> None: untrusted_args=untrusted_args) async def inquire_command_D(self, validate_sexp: 'SExprValidator', *, - untrusted_args: bytes) -> bool: + untrusted_args: bytes) -> bool: # We parse and then reserialize the sexpr. Currently we assume that the # sexpr fits in one assuan line. This line length also implicitly # limits the sexpr sizes. From 329b272e1a727e7e7ee9354a301150ff0ae312ce Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Fri, 25 Oct 2024 16:24:14 -0400 Subject: [PATCH 09/13] Understand Assuan comments Assuan comments start with '#' and must be ignored. Do not send them to the client. --- splitgpg2/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index 17f09bb..a8422fe 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -1055,6 +1055,9 @@ async def handle_agent_response(self, untrusted_line = await self.agent_reader.readline() untrusted_line = untrusted_line.rstrip(b'\n') self.log_io('A >>>', untrusted_line) + if untrusted_line.startswith(b'#'): + # Comment, ignore + return True untrusted_res, untrusted_args = extract_args(untrusted_line) if untrusted_res in (b'D', b'S'): # passthrough to the client From 5890078a1a4bba342d500b1fe90d258233192542 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 24 Oct 2024 23:06:05 -0400 Subject: [PATCH 10/13] Always pass expected_inquires dict This saves some code. --- splitgpg2/__init__.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index a8422fe..f69222b 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -469,7 +469,7 @@ async def connect_agent(self) -> None: self.notify('connected') # wait for agent hello - await self.handle_agent_response() + await self.handle_agent_response({}) def close(self, reason: str, log_level: int = logging.ERROR) -> None: self.log.log(log_level, '%s; Closing!', reason) @@ -1036,8 +1036,7 @@ def agent_write(self, data: bytes) -> None: writer.write(data) async def handle_agent_response(self, - expected_inquires: Optional[Dict[bytes, 'ArgCallback']] = None) \ - -> bool: + expected_inquires: Dict[bytes, 'ArgCallback']) -> bool: """ Receive and handle one agent response. Return whether there are more expected """ assert self.agent_reader is not None @@ -1048,8 +1047,6 @@ async def handle_agent_response(self, while await self.agent_reader.read(1024): pass return False - expected_inquires = (expected_inquires if expected_inquires is not None - else {}) # We generally consider the agent as trusted. But since the client can # determine part of the response we handle this here as untrusted. untrusted_line = await self.agent_reader.readline() From cff7f8eec33c0b221a1f74e39ac5dd15e9fffb08 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Wed, 23 Oct 2024 17:26:31 -0400 Subject: [PATCH 11/13] Clean up input validation - Compile all regular expressions during initialization. - Check for newline injection before sending data to the agent. - Misc cleanups. - Use "parse, don't validate" for commands taking keygrips --- splitgpg2/__init__.py | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index f69222b..51d0544 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -171,7 +171,8 @@ def extract_args(untrusted_line: bytes, sep: bytes = b' ') -> Tuple[bytes, Optio return untrusted_line, None # none of our uses allow 0, so do not allow it -_int_re = re.compile(rb'\A[1-9][0-9]*\Z') +_int_re: re.Pattern[bytes] = re.compile(rb'\A[1-9][0-9]*\Z') +_hash_regex = re.compile(rb'\A[0-9A-F]+\Z') def sanitize_int(untrusted_arg: bytes, min_value: int, max_value: int) -> int: """ @@ -224,6 +225,8 @@ class GpgServer: log: logging.Logger cache_nonce_regex: re.Pattern[bytes] = re.compile(rb'\A[0-9A-F]{24}\Z') + # Any command argument ever sent to the agent should match this pattern. + command_argument_regex: re.Pattern[bytes] = re.compile(rb'\A[0-9A-Za-z_=. -]*\Z') __slots__ = ('verbose_notifications', 'timer_delay', @@ -640,15 +643,15 @@ def fake_respond(self, response: bytes) -> None: @staticmethod def verify_keygrip_arguments(min_count: int, max_count: int, - untrusted_args: Optional[bytes]) -> bytes: + untrusted_args: Optional[bytes]) -> bytes: if untrusted_args is None: raise Filtered - args_regex = re.compile(rb'\A[0-9A-F]{40}( [0-9A-F]{40}){%d,%d}\Z' % - (min_count-1, max_count-1)) - - if args_regex.match(untrusted_args) is None: + if not (min_count <= len(untrusted_args_list) <= max_count): raise Filtered - return untrusted_args + for untrusted_arg in untrusted_args_list: + if len(untrusted_arg) != 40 or not _hash_regex.match(untrusted_arg): + raise Filtered + return b' '.join(untrusted_args_list) def sanitize_key_desc(self, untrusted_args: bytes) -> bytes: untrusted_args = untrusted_args.replace(b'+', b' ') @@ -937,30 +940,27 @@ async def command_SETHASH(self, untrusted_args: Optional[bytes]) -> None: except KeyError as e: raise Filtered from e - if not untrusted_hash: + if len(untrusted_hash) != alg_param.len: raise Filtered - hash_regex = re.compile(rb'\A[0-9A-F]{%d}\Z' % alg_param.len) - if hash_regex.match(untrusted_hash) is None: + if not _hash_regex.match(untrusted_hash): raise Filtered hash_value = untrusted_hash - await self.send_agent_command( - b'SETHASH', b'%d %s' % (alg, hash_value)) + # Hash values and ASCII decimal numbers are safe to pass. + await self.send_agent_command(b'SETHASH', b'%d %s' % (alg, hash_value)) async def command_PKSIGN(self, untrusted_args: Optional[bytes]) -> None: if untrusted_args is not None: if not untrusted_args.startswith(b'-- '): raise Filtered - untrusted_args = untrusted_args[3:] - if self.cache_nonce_regex.match(untrusted_args) is None: + if self.cache_nonce_regex.match(untrusted_args[3:]) is None: raise Filtered - args = b'-- ' + untrusted_args - else: - args = None + args = untrusted_args self.request_timer('PKSIGN') + # String checked to be '-- ' followed by a cache nonce await self.send_agent_command(b'PKSIGN', args) async def command_GETINFO(self, untrusted_args: Optional[bytes]) -> None: @@ -1019,6 +1019,8 @@ async def send_agent_command(self, command: bytes, args: Optional[bytes]) -> Non """ Sends command to local gpg agent and handle the response """ expected_inquires = self.get_inquires_for_command(command) if args: + if not self.command_argument_regex.match(args): + raise AssertionError("BUG: corrupt command about to be sent to agent!") cmd_with_args = command + b' ' + args + b'\n' else: cmd_with_args = command + b'\n' @@ -1355,7 +1357,7 @@ def _parse_sexpr(cls, untrusted_arg: bytes, nesting: int) -> Tuple[List['SExpr'] rest: bytes value: Union[List['SExpr'], bytes] - if untrusted_arg[0] in range(0x30, 0x40): + if 0x30 <= untrusted_arg[0] <= 0x40: length_s, rest = untrusted_arg.split(b':', 1) length = sanitize_int(length_s, 1, len(rest)) value, rest = rest[0:length], rest[length:] From ebf46f98d53a9a5d9bdfc90a21442dfcbfe4a3f2 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 21 Oct 2024 21:00:17 -0400 Subject: [PATCH 12/13] Allow "KEYINFO --list" Currently, this command is blocked. GnuPG detects that the agent connection is restricted and doesn't try to use it, while Sequoia Chameleon does not implement the fallback and is unable to list secret keys or decrypt messages. Furthermore, 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. Fix these problems by allowing this request. The request does not work over a restricted connection, so an unrestricted connection must be used. However, the filtering done by split-gpg2 is far stronger than the access checks in gpg-agent so there is no loss of security. Fixes: QubesOS/qubes-issues#9529 --- splitgpg2/__init__.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index 51d0544..cf07c8a 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -459,7 +459,7 @@ async def connect_agent(self) -> None: if self.allow_keygen: socket_field = b'agent-socket:' else: - socket_field = b'agent-extra-socket:' + socket_field = b'agent-extra-socket' # search for agent-socket:/run/user/1000/gnupg/S.gpg-agent agent_socket_path = [d.split(b':', 1)[1] for d in dirs.splitlines() if d.startswith(socket_field)][0] @@ -643,9 +643,18 @@ def fake_respond(self, response: bytes) -> None: @staticmethod def verify_keygrip_arguments(min_count: int, max_count: int, - untrusted_args: Optional[bytes]) -> bytes: + untrusted_args: Optional[bytes], + allow_list: bool) -> bytes: if untrusted_args is None: raise Filtered + if allow_list and untrusted_args.startswith(b'--list'): + if untrusted_args == b'--list': + return b'--list' + if untrusted_args[6] == 61: # ASCII '=' + # 1000 is the default value used by gpg2 + return b'--list=%d' % sanitize_int(untrusted_args[7:], 1, 1000) + raise Filtered + untrusted_args_list: List[bytes] = untrusted_args.split(b' ') if not (min_count <= len(untrusted_args_list) <= max_count): raise Filtered for untrusted_arg in untrusted_args_list: @@ -728,21 +737,11 @@ async def command_HAVEKEY(self, untrusted_args: Optional[bytes]) -> None: if untrusted_args is None: raise Filtered # upper keygrip limit is arbitary - if untrusted_args.startswith(b'--list'): - if b'=' in untrusted_args: - # 1000 is the default value used by gpg2 - limit = sanitize_int(untrusted_args[len(b'--list='):], 1, 1000) - args = b'--list=%d' % limit - else: - if untrusted_args != b'--list': - raise Filtered - args = b'--list' - else: - args = self.verify_keygrip_arguments(1, 200, untrusted_args) + args = self.verify_keygrip_arguments(1, 200, untrusted_args, True) await self.send_agent_command(b'HAVEKEY', args) async def command_KEYINFO(self, untrusted_args: Optional[bytes]) -> None: - args = self.verify_keygrip_arguments(1, 1, untrusted_args) + args = self.verify_keygrip_arguments(1, 1, untrusted_args, True) await self.send_agent_command(b'KEYINFO', args) async def command_GENKEY(self, untrusted_args: Optional[bytes]) -> None: @@ -782,12 +781,12 @@ async def command_GENKEY(self, untrusted_args: Optional[bytes]) -> None: await self.send_agent_command(b'GENKEY', b' '.join(args)) async def command_SIGKEY(self, untrusted_args: Optional[bytes]) -> None: - args = self.verify_keygrip_arguments(1, 1, untrusted_args) + args = self.verify_keygrip_arguments(1, 1, untrusted_args, False) await self.send_agent_command(b'SIGKEY', args) await self.setkeydesc(args) async def command_SETKEY(self, untrusted_args: Optional[bytes]) -> None: - args = self.verify_keygrip_arguments(1, 1, untrusted_args) + args = self.verify_keygrip_arguments(1, 1, untrusted_args, False) await self.send_agent_command(b'SETKEY', args) await self.setkeydesc(args) @@ -992,7 +991,7 @@ async def command_READKEY(self, untrusted_args: Optional[bytes]) -> None: raise Filtered if untrusted_args.startswith(b'-- '): untrusted_args = untrusted_args[3:] - args = self.verify_keygrip_arguments(1, 1, untrusted_args) + args = self.verify_keygrip_arguments(1, 1, untrusted_args, False) await self.send_agent_command(b'READKEY', b'-- ' + args) From 2129a1287494148e9839e0219e9d191cb6308410 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 24 Oct 2024 23:29:25 -0400 Subject: [PATCH 13/13] Use unrestricted connection for HAVEKEY --list and KEYINFO --list These commands are forbidden over a restricted connection to the agent, but GnuPG wars if they are not present and Sequoia Chameleon requires them. Fortunately, they are trivial to sanitize input for, so there is near-zero risk of an injection vulnerability. Therefore, use a separate unrestricted agent connection for these commands. Also use a separate function to read agent hello messages sent upon connection. --- splitgpg2/__init__.py | 106 +++++++++++++++++++++++++++++------------- 1 file changed, 73 insertions(+), 33 deletions(-) diff --git a/splitgpg2/__init__.py b/splitgpg2/__init__.py index cf07c8a..9264ce2 100755 --- a/splitgpg2/__init__.py +++ b/splitgpg2/__init__.py @@ -218,6 +218,7 @@ class GpgServer: commands: Dict[bytes, 'NoneCallback'] seen_data: bool config_loaded: bool + agent_unrestricted_socket_path: Optional[str] agent_socket_path: Optional[str] agent_reader: Optional[asyncio.StreamReader] agent_writer: Optional[asyncio.StreamWriter] @@ -245,6 +246,7 @@ class GpgServer: 'seen_data', 'config_loaded', 'agent_socket_path', + 'agent_unrestricted_socket_path', 'agent_reader', 'agent_writer', 'source_keyring_dir', @@ -275,6 +277,7 @@ def __init__(self, reader: asyncio.StreamReader, self.log = logging.getLogger('splitgpg2.Server') self.agent_socket_path = None + self.agent_unrestricted_socket_path = None self.agent_reader: Optional[asyncio.StreamReader] = None self.agent_writer: Optional[asyncio.StreamWriter] = None @@ -456,14 +459,20 @@ async def connect_agent(self) -> None: dirs = subprocess.check_output( ['gpgconf', *self.homedir_opts(), '--list-dirs', '-o/dev/stdout']) - if self.allow_keygen: - socket_field = b'agent-socket:' - else: - socket_field = b'agent-extra-socket' + unrestricted_socket_field = b'agent-socket' + socket_field = unrestricted_socket_field if self.allow_keygen else b'agent-extra-socket' # search for agent-socket:/run/user/1000/gnupg/S.gpg-agent - agent_socket_path = [d.split(b':', 1)[1] for d in dirs.splitlines() - if d.startswith(socket_field)][0] - self.agent_socket_path = agent_socket_path.decode() + for d in dirs.splitlines(): + key, value = d.split(b':') + if key == socket_field: + self.agent_socket_path = value.decode("UTF-8", "surrogateescape") + if key == unrestricted_socket_field: + self.agent_unrestricted_socket_path = value.decode("UTF-8", "surrogateescape") + if ((self.agent_unrestricted_socket_path is not None) and + (self.agent_socket_path is not None)): + break + else: + raise RuntimeError("bad output from gpgconf") self.agent_reader, self.agent_writer = await asyncio.open_unix_connection( path=self.agent_socket_path) @@ -472,7 +481,7 @@ async def connect_agent(self) -> None: self.notify('connected') # wait for agent hello - await self.handle_agent_response({}) + self.client_write(await self.read_hello(self.agent_reader)) def close(self, reason: str, log_level: int = logging.ERROR) -> None: self.log.log(log_level, '%s; Closing!', reason) @@ -559,8 +568,8 @@ def default_options() -> Dict[bytes, Tuple[OptionHandlingType, Optional[bytes]]] b'lc-messages': (OptionHandlingType.fake, b'OK'), b'putenv': (OptionHandlingType.fake, b'OK'), b'pinentry-mode': (OptionHandlingType.fake, b'ERR 67108924 Not supported '), - b'allow-pinentry-notify': (OptionHandlingType.verify, None), - b'agent-awareness': (OptionHandlingType.verify, b'2.1.0') + b'allow-pinentry-notify': (OptionHandlingType.fake, b'OK'), + b'agent-awareness': (OptionHandlingType.verify, b'2.1.0'), } @staticmethod @@ -738,11 +747,13 @@ async def command_HAVEKEY(self, untrusted_args: Optional[bytes]) -> None: raise Filtered # upper keygrip limit is arbitary args = self.verify_keygrip_arguments(1, 200, untrusted_args, True) - await self.send_agent_command(b'HAVEKEY', args) + unrestricted = args.startswith(b'--list') and not self.allow_keygen + await self.send_agent_command(b'HAVEKEY', args, unrestricted) async def command_KEYINFO(self, untrusted_args: Optional[bytes]) -> None: args = self.verify_keygrip_arguments(1, 1, untrusted_args, True) - await self.send_agent_command(b'KEYINFO', args) + unrestricted = args.startswith(b'--list') and not self.allow_keygen + await self.send_agent_command(b'KEYINFO', args, unrestricted) async def command_GENKEY(self, untrusted_args: Optional[bytes]) -> None: if not self.allow_keygen: @@ -817,7 +828,8 @@ async def setkeydesc(self, keygrip: bytes) -> None: key.fingerprint, subkey_desc) - self.agent_write(b'SETKEYDESC %s\n' % self.percent_plus_escape(desc)) + assert self.agent_writer is not None, "no writer?" + self.agent_write(b'SETKEYDESC %s\n' % self.percent_plus_escape(desc), self.agent_writer) assert self.agent_reader is not None untrusted_line = await self.agent_reader.readline() @@ -1014,43 +1026,68 @@ def get_inquires_for_command(self, command: bytes) -> Dict[bytes, 'ArgCallback'] } return {} - async def send_agent_command(self, command: bytes, args: Optional[bytes]) -> None: + async def send_agent_command(self, command: bytes, args: Optional[bytes], + unrestricted: bool=False) -> None: """ Sends command to local gpg agent and handle the response """ expected_inquires = self.get_inquires_for_command(command) - if args: - if not self.command_argument_regex.match(args): - raise AssertionError("BUG: corrupt command about to be sent to agent!") - cmd_with_args = command + b' ' + args + b'\n' + assert self.agent_reader is not None, "no reader?" + assert self.agent_writer is not None, "no writer?" + if unrestricted and not self.allow_keygen: + reader, writer = await asyncio.open_unix_connection( + self.agent_unrestricted_socket_path) + await self.read_hello(reader) else: - cmd_with_args = command + b'\n' - self.agent_write(cmd_with_args) - while True: - more_expected = await self.handle_agent_response( - expected_inquires=expected_inquires) - if not more_expected: - break + reader, writer = self.agent_reader, self.agent_writer + try: + if args: + if not self.command_argument_regex.match(args): + raise AssertionError("BUG: corrupt command about to be sent to agent!") + cmd_with_args = command + b' ' + args + b'\n' + else: + cmd_with_args = command + b'\n' + self.agent_write(cmd_with_args, writer) + while True: + more_expected = await self.handle_agent_response(expected_inquires, reader) + if not more_expected: + break + finally: + if reader is not self.agent_reader: + writer.close() - def agent_write(self, data: bytes) -> None: - writer = self.agent_writer + async def read_hello(self, agent_reader: asyncio.StreamReader) -> bytes: + while True: + line = await agent_reader.readline() + if not line.endswith(b'\n'): + raise ProtocolError("premature EOF from agent connection") + if b'\n' in line[:-1]: + raise ProtocolError("newline in readline() result???") + if line.startswith(b'#'): + continue + if line == b'OK' or line.startswith(b'OK '): + return line + raise ProtocolError("agent responded with something other than 'OK'" + " to initial connection") + + def agent_write(self, data: bytes, writer: asyncio.StreamWriter) -> None: assert writer is not None, 'agent_write called with no agent writer?' self.log_io('A <<<', data) writer.write(data) async def handle_agent_response(self, - expected_inquires: Dict[bytes, 'ArgCallback']) -> bool: + expected_inquires: Dict[bytes, 'ArgCallback'], + agent_reader: asyncio.StreamReader) -> bool: """ Receive and handle one agent response. Return whether there are more expected """ - assert self.agent_reader is not None assert self.client_writer is not None if self.client_writer.is_closing(): # If something went wrong, agent might send back junk. # Discard all remaining data from agent and return. - while await self.agent_reader.read(1024): + while await agent_reader.read(1024): pass return False # We generally consider the agent as trusted. But since the client can # determine part of the response we handle this here as untrusted. - untrusted_line = await self.agent_reader.readline() + untrusted_line = await agent_reader.readline() untrusted_line = untrusted_line.rstrip(b'\n') self.log_io('A >>>', untrusted_line) if untrusted_line.startswith(b'#'): @@ -1291,7 +1328,9 @@ async def inquire_command_D(self, validate_sexp: 'SExprValidator', *, raise Filtered from e args = untrusted_sexp - self.agent_write(b'D ' + self.escape_D(self.serialize_sexpr(args)) + b'\n') + assert self.agent_writer is not None, "no writer?" + self.agent_write(b'D ' + self.escape_D(self.serialize_sexpr(args)) + b'\n', + self.agent_writer) self.seen_data = True return True @@ -1389,7 +1428,8 @@ def serialize_item(item: 'SExpr') -> bytes: async def inquire_command_END(self, *, untrusted_args: bytes) -> bool: if untrusted_args: raise Filtered('unexpected arguments to END') - self.agent_write(b'END\n') + assert self.agent_writer is not None, "no writer?" + self.agent_write(b'END\n', self.agent_writer) return False # endregion