[tor-commits] [stem/master] Rewrite ONION_CLIENT_AUTH_VIEW parsing

commit 6af714a4bdae173f39ea4a9ba002dc3ddb60bef6 Author: Damian Johnson atagar@torproject.org Date: Wed Aug 5 17:39:41 2020 -0700

Rewrite ONION_CLIENT_AUTH_VIEW parsing

Mig5's parser was a fine proof of concept but stem parses everything within the spec. Our list_hidden_service_auth() method now returns either a credential or dictionary of credentials based on if we're requesting a single service or everything. --- docs/change_log.rst | 1 + stem/control.py | 87 +++++++++++++++++++++++--------------- stem/interpreter/settings.cfg | 2 +- stem/response/__init__.py | 14 ------ stem/response/onion_client_auth.py | 86 +++++++++++++++++++------------------ test/integ/control/controller.py | 49 +++++++++++++++------ 6 files changed, 136 insertions(+), 103 deletions(-)

diff --git a/docs/change_log.rst b/docs/change_log.rst index d1d04c87..f76b933f 100644 --- a/docs/change_log.rst +++ b/docs/change_log.rst @@ -49,6 +49,7 @@ The following are only available within Stem's `git repository * **Controller**

* Socket based control connections often raised BrokenPipeError when closed + * Added :func:`~stem.control.Controller.add_hidden_service_auth`, :func:`~stem.control.Controller.remove_hidden_service_auth`, and :func:`~stem.control.Controller.list_hidden_service_auth` to the :class:`~stem.control.Controller`

* **Descriptors**

diff --git a/stem/control.py b/stem/control.py index 61c4a277..d77eb311 100644 --- a/stem/control.py +++ b/stem/control.py @@ -457,6 +457,18 @@ class CreateHiddenServiceOutput(collections.namedtuple('CreateHiddenServiceOutpu """

+class HiddenServiceCredential(collections.namedtuple('HiddenServiceCredential', ['service_id', 'private_key', 'key_type', 'client_name', 'flags'])): + """ + Credentials for authenticating with a v3 hidden service. + + :var str service_id: hidden service id without its '.onion' suffix + :var str private_key: base64 encoded credential + :var str key_type: encryption type being used + :var str client_name: optional nickname for our client + :var list flags: flags associated with this credential + """ + + def with_default(yields: bool = False) -> Callable: """ Provides a decorator to support having a default value. This should be @@ -3083,76 +3095,83 @@ class Controller(BaseController): else: raise stem.ProtocolError('DEL_ONION returned unexpected response code: %s' % response.code)

- async def add_hidden_service_auth(self, service_id: str, private_key_blob: str, key_type: str = 'x25519', client_name: Optional[str] = None, permanent: Optional[bool] = False) -> stem.response.onion_client_auth.OnionClientAuthAddResponse: + async def add_hidden_service_auth(self, service_id: str, private_key: str, key_type: str = 'x25519', client_name: Optional[str] = None, write: bool = False) -> None: """ Authenticate with a v3 hidden service.

+ .. versionadded:: 2.0.0 + :param service_id: hidden service address without the '.onion' suffix - :param key_type: the type of private key in use. x25519 is the only one supported right now - :param private_key_blob: base64 encoding of x25519 private key + :param private_key: base64 encoded private key to authenticate with + :param key_type: type of private key in use :param client_name: optional nickname for this client - :param permanent: optionally flag that this client's credentials should be stored in the filesystem. - If this is not set, the client's credentials are epheremal and stored in memory. - - :returns: **True* if the client authentication was added or replaced, **False** if it - was rejected by the Tor controller + :param write: persists these credentials to disk if **True**, only resides + in memory otherwise

:raises: :class:`stem.ControllerError` if the call fails """

- request = 'ONION_CLIENT_AUTH_ADD %s %s:%s' % (service_id, key_type, private_key_blob) + request = 'ONION_CLIENT_AUTH_ADD %s %s:%s' % (service_id, key_type, private_key)

if client_name: request += ' ClientName=%s' % client_name

- flags = [] - - if permanent: - flags.append('Permanent') - - if flags: - request += ' Flags=%s' % ','.join(flags) + if write: + request += ' Flags=Permanent'

- response = stem.response._convert_to_onion_client_auth_add(stem.response._convert_to_onion_client_auth_add(await self.msg(request))) + response = stem.response._convert_to_single_line(await self.msg(request))

- return response + if not response.is_ok(): + raise stem.ProtocolError("ONION_CLIENT_AUTH_ADD response didn't have an OK status: %s" % response.message)

- async def remove_hidden_service_auth(self, service_id: str) -> stem.response.onion_client_auth.OnionClientAuthRemoveResponse: + async def remove_hidden_service_auth(self, service_id: str) -> None: """ Revoke authentication with a v3 hidden service.

- :param service_id: hidden service address without the '.onion' suffix + .. versionadded:: 2.0.0

- :returns: **True* if the client authentication was removed, (or if no such - service ID existed), **False** if it was rejected by the Tor controller + :param service_id: hidden service address without the '.onion' suffix

:raises: :class:`stem.ControllerError` if the call fails """

- request = 'ONION_CLIENT_AUTH_REMOVE %s' % service_id + response = stem.response._convert_to_single_line(await self.msg('ONION_CLIENT_AUTH_REMOVE %s' % service_id))

- response = stem.response._convert_to_onion_client_auth_remove(stem.response._convert_to_onion_client_auth_remove(await self.msg(request))) - - return response + if not response.is_ok(): + raise stem.ProtocolError("ONION_CLIENT_AUTH_REMOVE response didn't have an OK status: %s" % response.message)

- async def list_hidden_service_auth(self, service_id: str) -> stem.response.onion_client_auth.OnionClientAuthViewResponse: + async def list_hidden_service_auth(self, service_id: Optional[str] = None) -> Union['stem.control.HiddenServiceCredential', Dict[str, 'stem.control.HiddenServiceCredential']]: """ View Client Authentication for a v3 onion service.

- :param service_id: hidden service address without the '.onion' suffix + .. versionadded:: 2.0.0 + + :param service_id: restrict query to this service, otherwise provides all + credentials

- :returns: :class:`~stem.response.onion_client_auth.OnionClientAuthViewResponse` with the - client_auth_credential if there were credentials to view, **True** if the service ID - was valid but no credentials existed, **False** if the service ID was invalid + :returns: single :class:`~stem.control.HiddenServiceCredential` if called + with a **service_id**, otherwise this provides a **dict** mapping hidden + service ids with their credential

:raises: :class:`stem.ControllerError` if the call fails """

- request = 'ONION_CLIENT_AUTH_VIEW %s' % service_id + request = 'ONION_CLIENT_AUTH_VIEW'

- response = stem.response._convert_to_onion_client_auth_view(stem.response._convert_to_onion_client_auth_view(await self.msg(request))) + if service_id: + request += ' %s' % service_id

- return response + response = stem.response._convert_to_onion_client_auth_view(await self.msg(request)) + + if service_id: + if service_id not in response.credentials: + raise stem.ProtocolError('ONION_CLIENT_AUTH_VIEW failed to provide a credential for %s: %s' % (service_id, response)) + elif len(response.credentials) != 1: + raise stem.ProtocolError('ONION_CLIENT_AUTH_VIEW should only contain credentials for %s but had %s' % (service_id, ', '.join(response.credentials.keys()))) + + return response.credentials[service_id] + else: + return response.credentials

async def add_event_listener(self, listener: Callable[[stem.response.events.Event], Union[None, Awaitable[None]]], *events: 'stem.control.EventType') -> None: """ diff --git a/stem/interpreter/settings.cfg b/stem/interpreter/settings.cfg index a373e617..fe853616 100644 --- a/stem/interpreter/settings.cfg +++ b/stem/interpreter/settings.cfg @@ -271,7 +271,7 @@ help.description.del_onion |Delete a hidden service that was created with ADD_ONION.

help.description.onion_client_auth -|Add, remove or view Client Authentication for a v3 onion service. +|Add, remove or list authentication credentials for a v3 hidden service.

help.description.hsfetch |Retrieves the descriptor for a hidden service. This is an asynchronous diff --git a/stem/response/__init__.py b/stem/response/__init__.py index eb6231bd..5749fbcb 100644 --- a/stem/response/__init__.py +++ b/stem/response/__init__.py @@ -73,8 +73,6 @@ def convert(response_type: str, message: 'stem.response.ControlMessage', **kwarg **GETCONF** :class:`stem.response.getconf.GetConfResponse` **GETINFO** :class:`stem.response.getinfo.GetInfoResponse` **MAPADDRESS** :class:`stem.response.mapaddress.MapAddressResponse` - **ONION_CLIENT_AUTH_ADD** :class:`stem.response.onion_client_auth.OnionClientAuthAddResponse` - **ONION_CLIENT_AUTH_REMOVE** :class:`stem.response.onion_client_auth.OnionClientAuthRemoveResponse` **ONION_CLIENT_AUTH_VIEW** :class:`stem.response.onion_client_auth.OnionClientAuthViewResponse` **PROTOCOLINFO** :class:`stem.response.protocolinfo.ProtocolInfoResponse` **SINGLELINE** :class:`stem.response.SingleLineResponse` @@ -118,8 +116,6 @@ def convert(response_type: str, message: 'stem.response.ControlMessage', **kwarg 'GETCONF': stem.response.getconf.GetConfResponse, 'GETINFO': stem.response.getinfo.GetInfoResponse, 'MAPADDRESS': stem.response.mapaddress.MapAddressResponse, - 'ONION_CLIENT_AUTH_ADD': stem.response.onion_client_auth.OnionClientAuthAddResponse, - 'ONION_CLIENT_AUTH_REMOVE': stem.response.onion_client_auth.OnionClientAuthRemoveResponse, 'ONION_CLIENT_AUTH_VIEW': stem.response.onion_client_auth.OnionClientAuthViewResponse, 'PROTOCOLINFO': stem.response.protocolinfo.ProtocolInfoResponse, 'SINGLELINE': SingleLineResponse, @@ -162,16 +158,6 @@ def _convert_to_add_onion(message: 'stem.response.ControlMessage', **kwargs: Any return message # type: ignore

-def _convert_to_onion_client_auth_add(message: 'stem.response.ControlMessage', **kwargs: Any) -> 'stem.response.onion_client_auth.OnionClientAuthAddResponse': - stem.response.convert('ONION_CLIENT_AUTH_ADD', message) - return message # type: ignore - - -def _convert_to_onion_client_auth_remove(message: 'stem.response.ControlMessage', **kwargs: Any) -> 'stem.response.onion_client_auth.OnionClientAuthRemoveResponse': - stem.response.convert('ONION_CLIENT_AUTH_REMOVE', message) - return message # type: ignore - - def _convert_to_onion_client_auth_view(message: 'stem.response.ControlMessage', **kwargs: Any) -> 'stem.response.onion_client_auth.OnionClientAuthViewResponse': stem.response.convert('ONION_CLIENT_AUTH_VIEW', message) return message # type: ignore diff --git a/stem/response/onion_client_auth.py b/stem/response/onion_client_auth.py index c21f9158..8735808c 100644 --- a/stem/response/onion_client_auth.py +++ b/stem/response/onion_client_auth.py @@ -1,61 +1,65 @@ -# Copyright 2015-2020, Damian Johnson and The Tor Project +# Copyright 2020, Damian Johnson and The Tor Project # See LICENSE for licensing information

+import stem.control import stem.response -from stem.util import log

-class OnionClientAuthAddResponse(stem.response.ControlMessage): +class OnionClientAuthViewResponse(stem.response.ControlMessage): """ - ONION_CLIENT_AUTH_ADD response. + ONION_CLIENT_AUTH_VIEW response. + + :var str requested: queried hidden service id, this is None if all + credentials were requested + :var dict credentials: mapping of hidden service ids to their + :class:`~stem.control.HiddenServiceCredential` """

def _parse_message(self) -> None: - # ONION_CLIENT_AUTH_ADD responds with: - # '250 OK', - # '251 Client for onion existed and replaced', - # '252 Registered client and decrypted desc', - # '512 Invalid v3 address [service id]', - # '553 Unable to store creds for [service id]' + # Example: + # 250-ONION_CLIENT_AUTH_VIEW yvhz3ofkv7gwf5hpzqvhonpr3gbax2cc7dee3xcnt7dmtlx2gu7vyvid + # 250-CLIENT yvhz3ofkv7gwf5hpzqvhonpr3gbax2cc7dee3xcnt7dmtlx2gu7vyvid x25519:FCV0c0ELDKKDpSFgVIB8Yow8Evj5iD+GoiTtK878NkQ= + # 250 OK + + self.requested = None + self.credentials = {}

if not self.is_ok(): - raise stem.ProtocolError("ONION_CLIENT_AUTH_ADD response didn't have an OK status: %s" % self) + raise stem.ProtocolError("ONION_CLIENT_AUTH_VIEW response didn't have an OK status: %s" % self)

+ # first line optionally contains the service id this request was for

-class OnionClientAuthRemoveResponse(stem.response.ControlMessage): - """ - ONION_CLIENT_AUTH_REMOVE response. - """ + first_line = list(self)[0]

- def _parse_message(self) -> None: - # ONION_CLIENT_AUTH_REMOVE responds with: - # '250 OK', - # '251 No credentials for [service id]', - # '512 Invalid v3 address [service id]' + if not first_line.startswith('ONION_CLIENT_AUTH_VIEW'): + raise stem.ProtocolError("Response should begin with 'ONION_CLIENT_AUTH_VIEW': %s" % self) + elif ' ' in first_line: + self.requested = first_line.split(' ')[1]

- if not self.is_ok(): - raise stem.ProtocolError("ONION_CLIENT_AUTH_REMOVE response didn't have an OK status: %s" % self) + for credential_line in list(self)[1:-1]: + attributes = credential_line.split(' ')

+ if len(attributes) < 3: + raise stem.ProtocolError('ONION_CLIENT_AUTH_VIEW lines must contain an address and credential: %s' % self) + elif attributes[0] != 'CLIENT': + raise stem.ProtocolError("ONION_CLIENT_AUTH_VIEW lines should begin with 'CLIENT': %s" % self) + elif ':' not in attributes[2]: + raise stem.ProtocolError("ONION_CLIENT_AUTH_VIEW credentials must be of the form 'encryption_type:key': %s" % self)

-class OnionClientAuthViewResponse(stem.response.ControlMessage): - """ - ONION_CLIENT_AUTH_VIEW response. - """ + service_id = attributes[1] + key_type, private_key = attributes[2].split(':', 1) + client_name = None + flags = []

- def _parse_message(self) -> None: - # ONION_CLIENT_AUTH_VIEW responds with: - # '250 OK' if there was Client Auth for this service or if the service is a valid address, - # ''512 Invalid v3 address [service id]' + for attr in attributes[2:]: + if '=' not in attr: + raise stem.ProtocolError("'%s' expected to be a 'key=value' mapping: %s" % (attr, self))

- self.client_auth_credential = None + key, value = attr.split('=', 1)

- if not self.is_ok(): - raise stem.ProtocolError("ONION_CLIENT_AUTH_VIEW response didn't have an OK status: %s" % self) - else: - for line in list(self): - if line.startswith('CLIENT'): - key, value = line.split(' ', 1) - log.debug(key) - log.debug(value) - - self.client_auth_credential = value + if key == 'ClientName': + client_name = value + elif key == 'Flags': + flags = value.split(',') + + self.credentials[service_id] = stem.control.HiddenServiceCredential(service_id, private_key, key_type, client_name, flags) diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py index 3840a1f6..25566229 100644 --- a/test/integ/control/controller.py +++ b/test/integ/control/controller.py @@ -29,6 +29,9 @@ from stem.control import EventType, Listener, State from stem.exit_policy import ExitPolicy from stem.util.test_tools import async_test

+SERVICE_ID = 'yvhz3ofkv7gwf5hpzqvhonpr3gbax2cc7dee3xcnt7dmtlx2gu7vyvid' +PRIVATE_KEY = 'FCV0c0ELDKKDpSFgVIB8Yow8Evj5iD+GoiTtK878NkQ=' + # Router status entry for a relay with a nickname other than 'Unnamed'. This is # used for a few tests that need to look up a relay.

@@ -1604,37 +1607,55 @@ class TestController(unittest.TestCase):

@test.require.controller @async_test - async def test_hidden_service_v3_authentication(self): + async def test_hidden_service_auth(self): """ Exercises adding, viewing and removing authentication credentials for a v3 service. """

async with await test.runner.get_runner().get_tor_controller() as controller: - service_id = 'yvhz3ofkv7gwf5hpzqvhonpr3gbax2cc7dee3xcnt7dmtlx2gu7vyvid' - private_key = 'FCV0c0ELDKKDpSFgVIB8Yow8Evj5iD+GoiTtK878NkQ=' - # register authentication credentials

- await controller.add_hidden_service_auth(service_id, private_key) + await controller.add_hidden_service_auth(SERVICE_ID, PRIVATE_KEY, client_name = 'StemInteg') + + credential = await controller.list_hidden_service_auth(SERVICE_ID)

- response = await controller.list_hidden_service_auth(service_id) - self.assertEqual('%s x25519:%s' % (service_id, private_key), response.client_auth_credential) + self.assertEqual(SERVICE_ID, credential.service_id) + self.assertEqual(PRIVATE_KEY, credential.private_key) + self.assertEqual('x25519', credential.key_type) + self.assertEqual([], credential.flags) + + # TODO: We should assert our client_name's value... + # + # self.assertEqual('StemInteg', credential.client_name) + # + # ... but that's broken within tor... + # + # https://gitlab.torproject.org/tpo/core/tor/-/issues/40089

# deregister authentication credentials

- await controller.remove_hidden_service_auth(service_id) + await controller.remove_hidden_service_auth(SERVICE_ID) + self.assertEqual({}, await controller.list_hidden_service_auth())

- response = await controller.list_hidden_service_auth(service_id) - self.assertEqual(None, response.client_auth_credential) + # TODO: We should add a persistance test (calling with 'write = True') + # but that doesn't look to be working... + # + # https://gitlab.torproject.org/tpo/core/tor/-/issues/40090

- # exercise with an invalid address + @test.require.controller + @async_test + async def test_hidden_service_auth_invalid(self): + """ + Exercises hidden service authentication with invalid data. + """

+ async with await test.runner.get_runner().get_tor_controller() as controller: invalid_service_id = 'xxxxxxxxyvhz3ofkv7gwf5hpzqvhonpr3gbax2cc7dee3xcnt7dmtlx2gu7vyvid' exc_msg = "%%s response didn't have an OK status: Invalid v3 address "%s"" % invalid_service_id

with self.assertRaisesWith(stem.ProtocolError, exc_msg % 'ONION_CLIENT_AUTH_ADD'): - await controller.add_hidden_service_auth(invalid_service_id, private_key) + await controller.add_hidden_service_auth(invalid_service_id, PRIVATE_KEY)

with self.assertRaisesWith(stem.ProtocolError, exc_msg % 'ONION_CLIENT_AUTH_REMOVE'): await controller.remove_hidden_service_auth(invalid_service_id) @@ -1642,10 +1663,12 @@ class TestController(unittest.TestCase): with self.assertRaisesWith(stem.ProtocolError, exc_msg % 'ONION_CLIENT_AUTH_VIEW'): await controller.list_hidden_service_auth(invalid_service_id)

+ invalid_key = 'XXXXXXXXXFCV0c0ELDKKDpSFgVIB8Yow8Evj5iD+GoiTtK878NkQ=' + # register with an invalid key

with self.assertRaisesWith(stem.ProtocolError, "ONION_CLIENT_AUTH_ADD response didn't have an OK status: Failed to decode x25519 private key"): - await controller.add_hidden_service_auth(service_id, 'XXXXXXXXXFCV0c0ELDKKDpSFgVIB8Yow8Evj5iD+GoiTtK878NkQ=') + await controller.add_hidden_service_auth(SERVICE_ID, invalid_key)

async def _get_router_status_entry(self, controller): """