Skip to content

Commit 8173a79

Browse files
committed
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 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.
1 parent 7145fa0 commit 8173a79

1 file changed

Lines changed: 72 additions & 30 deletions

File tree

splitgpg2/__init__.py

Lines changed: 72 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ class GpgServer:
218218
commands: Dict[bytes, 'NoneCallback']
219219
seen_data: bool
220220
config_loaded: bool
221+
agent_unrestricted_socket_path: Optional[str]
221222
agent_socket_path: Optional[str]
222223
agent_reader: Optional[asyncio.StreamReader]
223224
agent_writer: Optional[asyncio.StreamWriter]
@@ -245,6 +246,7 @@ class GpgServer:
245246
'seen_data',
246247
'config_loaded',
247248
'agent_socket_path',
249+
'agent_unrestricted_socket_path',
248250
'agent_reader',
249251
'agent_writer',
250252
'source_keyring_dir',
@@ -275,6 +277,7 @@ def __init__(self, reader: asyncio.StreamReader,
275277

276278
self.log = logging.getLogger('splitgpg2.Server')
277279
self.agent_socket_path = None
280+
self.agent_unrestricted_socket_path = None
278281
self.agent_reader: Optional[asyncio.StreamReader] = None
279282
self.agent_writer: Optional[asyncio.StreamWriter] = None
280283

@@ -463,11 +466,20 @@ async def connect_agent(self) -> None:
463466
# a message.
464467
# The filtering done by split-gpg2 is far stronger than anything the agent does
465468
# internally.
466-
socket_field = b'agent-socket:'
469+
unrestricted_socket_field = b'agent-socket'
470+
socket_field = unrestricted_socket_field if self.allow_keygen else b'agent-extra-socket'
467471
# search for agent-socket:/run/user/1000/gnupg/S.gpg-agent
468-
agent_socket_path = [d.split(b':', 1)[1] for d in dirs.splitlines()
469-
if d.startswith(socket_field)][0]
470-
self.agent_socket_path = agent_socket_path.decode()
472+
for d in dirs.splitlines():
473+
key, value = d.split(b':')
474+
if key == socket_field:
475+
self.agent_socket_path = value.decode("UTF-8", "surrogateescape")
476+
if key == unrestricted_socket_field:
477+
self.agent_unrestricted_socket_path = value.decode("UTF-8", "surrogateescape")
478+
if ((self.agent_unrestricted_socket_path is not None) and
479+
(self.agent_socket_path is not None)):
480+
break
481+
else:
482+
raise RuntimeError("bad output from gpgconf")
471483

472484
self.agent_reader, self.agent_writer = await asyncio.open_unix_connection(
473485
path=self.agent_socket_path)
@@ -476,7 +488,7 @@ async def connect_agent(self) -> None:
476488
self.notify('connected')
477489

478490
# wait for agent hello
479-
await self.handle_agent_response({})
491+
self.client_write(await self.read_hello(self.agent_reader))
480492

481493
def close(self, reason: str, log_level: int = logging.ERROR) -> None:
482494
self.log.log(log_level, '%s; Closing!', reason)
@@ -563,8 +575,8 @@ def default_options() -> Dict[bytes, Tuple[OptionHandlingType, Optional[bytes]]]
563575
b'lc-messages': (OptionHandlingType.fake, b'OK'),
564576
b'putenv': (OptionHandlingType.fake, b'OK'),
565577
b'pinentry-mode': (OptionHandlingType.fake, b'ERR 67108924 Not supported <GPG Agent>'),
566-
b'allow-pinentry-notify': (OptionHandlingType.verify, None),
567-
b'agent-awareness': (OptionHandlingType.verify, b'2.1.0')
578+
b'allow-pinentry-notify': (OptionHandlingType.fake, b'OK'),
579+
b'agent-awareness': (OptionHandlingType.verify, b'2.1.0'),
568580
}
569581

570582
@staticmethod
@@ -744,11 +756,13 @@ async def command_HAVEKEY(self, untrusted_args: Optional[bytes]) -> None:
744756
raise Filtered
745757
# upper keygrip limit is arbitary
746758
args = self.verify_keygrip_arguments(1, 200, untrusted_args, True)
747-
await self.send_agent_command(b'HAVEKEY', args)
759+
unrestricted = args.startswith(b'--list') and not self.allow_keygen
760+
await self.send_agent_command(b'HAVEKEY', args, unrestricted)
748761

749762
async def command_KEYINFO(self, untrusted_args: Optional[bytes]) -> None:
750763
args = self.verify_keygrip_arguments(1, 1, untrusted_args, True)
751-
await self.send_agent_command(b'KEYINFO', args)
764+
unrestricted = args.startswith(b'--list') and not self.allow_keygen
765+
await self.send_agent_command(b'KEYINFO', args, unrestricted)
752766

753767
async def command_GENKEY(self, untrusted_args: Optional[bytes]) -> None:
754768
if not self.allow_keygen:
@@ -823,7 +837,8 @@ async def setkeydesc(self, keygrip: bytes) -> None:
823837
key.fingerprint,
824838
subkey_desc)
825839

826-
self.agent_write(b'SETKEYDESC %s\n' % self.percent_plus_escape(desc))
840+
assert self.agent_writer is not None, "no writer?"
841+
self.agent_write(b'SETKEYDESC %s\n' % self.percent_plus_escape(desc), self.agent_writer)
827842

828843
assert self.agent_reader is not None
829844
untrusted_line = await self.agent_reader.readline()
@@ -1020,43 +1035,67 @@ def get_inquires_for_command(self, command: bytes) -> Dict[bytes, 'ArgCallback']
10201035
}
10211036
return {}
10221037

1023-
async def send_agent_command(self, command: bytes, args: Optional[bytes]) -> None:
1038+
async def send_agent_command(self, command: bytes, args: Optional[bytes],
1039+
unrestricted: bool=False) -> None:
10241040
""" Sends command to local gpg agent and handle the response """
10251041
expected_inquires = self.get_inquires_for_command(command)
1026-
if args:
1027-
if not self.command_argument_regex.match(args):
1028-
raise AssertionError("BUG: corrupt command about to be sent to agent!")
1029-
cmd_with_args = command + b' ' + args + b'\n'
1042+
assert self.agent_reader is not None, "no reader?"
1043+
assert self.agent_writer is not None, "no writer?"
1044+
if unrestricted and not self.allow_keygen:
1045+
reader, writer = await asyncio.open_unix_connection(
1046+
self.agent_unrestricted_socket_path)
1047+
await self.read_hello(reader)
10301048
else:
1031-
cmd_with_args = command + b'\n'
1032-
self.agent_write(cmd_with_args)
1033-
while True:
1034-
more_expected = await self.handle_agent_response(
1035-
expected_inquires=expected_inquires)
1036-
if not more_expected:
1037-
break
1049+
reader, writer = self.agent_reader, self.agent_writer
1050+
try:
1051+
if args:
1052+
if not self.command_argument_regex.match(args):
1053+
raise AssertionError("BUG: corrupt command about to be sent to agent!")
1054+
cmd_with_args = command + b' ' + args + b'\n'
1055+
else:
1056+
cmd_with_args = command + b'\n'
1057+
self.agent_write(cmd_with_args, writer)
1058+
while True:
1059+
more_expected = await self.handle_agent_response(expected_inquires, reader)
1060+
if not more_expected:
1061+
break
1062+
finally:
1063+
if reader is not self.agent_reader:
1064+
writer.close()
10381065

1039-
def agent_write(self, data: bytes) -> None:
1040-
writer = self.agent_writer
1066+
async def read_hello(self, agent_reader: asyncio.StreamReader) -> bytes:
1067+
while True:
1068+
line = await agent_reader.readline()
1069+
if not line.endswith(b'\n'):
1070+
raise ProtocolError("premature EOF from agent connection")
1071+
if b'\n' in line[:-1]:
1072+
raise ProtocolError("newline in readline() result???")
1073+
if line.startswith(b'#'):
1074+
continue
1075+
if line == b'OK' or line.startswith(b'OK '):
1076+
return line
1077+
raise ProtocolError("agent responded with something other than 'OK' to initial connection")
1078+
1079+
def agent_write(self, data: bytes, writer: asyncio.StreamWriter) -> None:
10411080
assert writer is not None, 'agent_write called with no agent writer?'
10421081
self.log_io('A <<<', data)
10431082
writer.write(data)
10441083

10451084
async def handle_agent_response(self,
1046-
expected_inquires: Dict[bytes, 'ArgCallback']) -> bool:
1085+
expected_inquires: Dict[bytes, 'ArgCallback'],
1086+
agent_reader: asyncio.StreamReader) -> bool:
10471087
""" Receive and handle one agent response. Return whether there are
10481088
more expected """
1049-
assert self.agent_reader is not None
10501089
assert self.client_writer is not None
10511090
if self.client_writer.is_closing():
10521091
# If something went wrong, agent might send back junk.
10531092
# Discard all remaining data from agent and return.
1054-
while await self.agent_reader.read(1024):
1093+
while await agent_reader.read(1024):
10551094
pass
10561095
return False
10571096
# We generally consider the agent as trusted. But since the client can
10581097
# determine part of the response we handle this here as untrusted.
1059-
untrusted_line = await self.agent_reader.readline()
1098+
untrusted_line = await agent_reader.readline()
10601099
untrusted_line = untrusted_line.rstrip(b'\n')
10611100
self.log_io('A >>>', untrusted_line)
10621101
if untrusted_line.startswith(b'#'):
@@ -1297,7 +1336,9 @@ async def inquire_command_D(self, validate_sexp: 'SExprValidator', *,
12971336
raise Filtered from e
12981337
args = untrusted_sexp
12991338

1300-
self.agent_write(b'D ' + self.escape_D(self.serialize_sexpr(args)) + b'\n')
1339+
assert self.agent_writer is not None, "no writer?"
1340+
self.agent_write(b'D ' + self.escape_D(self.serialize_sexpr(args)) + b'\n',
1341+
self.agent_writer)
13011342
self.seen_data = True
13021343
return True
13031344

@@ -1395,7 +1436,8 @@ def serialize_item(item: 'SExpr') -> bytes:
13951436
async def inquire_command_END(self, *, untrusted_args: bytes) -> bool:
13961437
if untrusted_args:
13971438
raise Filtered('unexpected arguments to END')
1398-
self.agent_write(b'END\n')
1439+
assert self.agent_writer is not None, "no writer?"
1440+
self.agent_write(b'END\n', self.agent_writer)
13991441
return False
14001442

14011443
# endregion

0 commit comments

Comments
 (0)