launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26037
[Merge] ~cjwatson/launchpad:signing-service-add-authorization into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:signing-service-add-authorization into launchpad:master.
Commit message:
Add addAuthorization methods for signing service
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/396511
This is the client-side counterpart to https://code.launchpad.net/~cjwatson/lp-signing/+git/lp-signing/+merge/396383. While we don't yet use it in Launchpad, it's useful for the client interface to be complete, and it makes it easier to make manual key authorization changes.
I also included some preliminary refactoring to reduce duplication in SigningServiceClient.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:signing-service-add-authorization into launchpad:master.
diff --git a/lib/lp/services/signing/interfaces/signingkey.py b/lib/lp/services/signing/interfaces/signingkey.py
index dadabd6..8548fa8 100644
--- a/lib/lp/services/signing/interfaces/signingkey.py
+++ b/lib/lp/services/signing/interfaces/signingkey.py
@@ -57,6 +57,12 @@ class ISigningKey(Interface):
keys, and `SigningMode.DETACHED` for other key types.
"""
+ def addAuthorization(client_name):
+ """Authorize another client to use this key.
+
+ :param client_name: The name of the client to authorize.
+ """
+
class ISigningKeySet(Interface):
"""Interface to deal with the collection of signing keys
diff --git a/lib/lp/services/signing/interfaces/signingserviceclient.py b/lib/lp/services/signing/interfaces/signingserviceclient.py
index 26dd69a..d1e6e5b 100644
--- a/lib/lp/services/signing/interfaces/signingserviceclient.py
+++ b/lib/lp/services/signing/interfaces/signingserviceclient.py
@@ -59,4 +59,13 @@ class ISigningServiceClient(Interface):
:param public_key: The public key content, in bytes.
:param description: The description of this key.
:param created_at: datetime of when the key was created.
+ :return: A dict with 'fingerprint'
+ """
+
+ def addAuthorization(key_type, fingerprint, client_name):
+ """Authorize another client to use a key.
+
+ :param key_type: One of `SigningKeyType`.
+ :param fingerprint: The fingerprint of the signing key.
+ :param client_name: The name of the client to authorize.
"""
diff --git a/lib/lp/services/signing/model/signingkey.py b/lib/lp/services/signing/model/signingkey.py
index 5dd21f5..ee77613 100644
--- a/lib/lp/services/signing/model/signingkey.py
+++ b/lib/lp/services/signing/model/signingkey.py
@@ -140,6 +140,12 @@ class SigningKey(StormBase):
self.key_type, self.fingerprint, message_name, message, mode)
return signed['signed-message']
+ def addAuthorization(self, client_name):
+ """See `ISigningKey`."""
+ signing_service = getUtility(ISigningServiceClient)
+ signing_service.addAuthorization(
+ self.key_type, self.fingerprint, client_name)
+
@implementer(IArchiveSigningKey)
class ArchiveSigningKey(StormBase):
diff --git a/lib/lp/services/signing/proxy.py b/lib/lp/services/signing/proxy.py
index 390cca9..8d78e65 100644
--- a/lib/lp/services/signing/proxy.py
+++ b/lib/lp/services/signing/proxy.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Proxy calls to lp-signing service"""
@@ -66,39 +66,48 @@ class SigningServiceClient:
return json.loads(box.decrypt(
response.content, response_nonce, encoder=Base64Encoder))
- def _requestJson(self, path, method="GET", **kwargs):
+ def _requestJson(self, path, method="GET", encrypt=False, **kwargs):
"""Helper method to do an HTTP request and get back a json from the
signing service, raising exception if status code != 2xx.
:param path: The endpoint path
:param method: The HTTP method to be used (GET, POST, etc)
- :param needs_resp_nonce: Indicates if the endpoint requires us to
- include a X-Response-Nonce, and returns back an encrypted
- response JSON.
+ :param encrypt: If True, make an encrypted and authenticated
+ request.
"""
- headers = kwargs.get("headers", {})
+ kwargs = dict(kwargs)
timeline = get_request_timeline(get_current_browser_request())
- redacted_kwargs = dict(kwargs)
- if "X-Client-Public-Key" in headers and "data" in redacted_kwargs:
- # The data will be encrypted, and possibly also very large.
- del redacted_kwargs["data"]
+ if encrypt:
+ nonce = self.getNonce()
+ response_nonce = self._makeResponseNonce()
+ headers = kwargs.setdefault("headers", {})
+ headers.update(self._getAuthHeaders(nonce, response_nonce))
+ if "data" in kwargs:
+ data = kwargs.pop("data")
+ elif "json" in kwargs:
+ data = json.dumps(kwargs.pop("json")).encode("UTF-8")
+ else:
+ data = b""
+ # The data will be encrypted, so shouldn't be exposed to OOPSes.
+ # It may also be very large.
+ redacted_kwargs = dict(kwargs)
+ # Stuff the encrypted data back into the arguments.
+ kwargs["data"] = self._encryptPayload(nonce, data)
+ else:
+ redacted_kwargs = kwargs
action = timeline.start(
"services-signing-proxy-%s" % method, "%s %s" %
(path, json.dumps(redacted_kwargs)))
- response_nonce = None
- if "X-Response-Nonce" in headers:
- response_nonce = base64.b64decode(headers["X-Response-Nonce"])
-
try:
url = self.getUrl(path)
response = urlfetch(url, method=method.lower(), **kwargs)
response.raise_for_status()
- if response_nonce is None:
- return response.json()
- else:
+ if encrypt:
return self._decryptResponseJson(response, response_nonce)
+ else:
+ return response.json()
finally:
action.finish()
@@ -155,22 +164,18 @@ class SigningServiceClient:
if length is None:
raise ValueError("SigningKeyType.OPENPGP requires length")
- nonce = self.getNonce()
- response_nonce = self._makeResponseNonce()
- raw_payload = {
+ payload = {
"key-type": key_type.name,
"description": description,
}
if key_type == SigningKeyType.OPENPGP:
- raw_payload.update({
+ payload.update({
"openpgp-key-algorithm": openpgp_key_algorithm.name,
"length": length,
})
- payload = json.dumps(raw_payload).encode("UTF-8")
+
ret = self._requestJson(
- "/generate", "POST",
- headers=self._getAuthHeaders(nonce, response_nonce),
- data=self._encryptPayload(nonce, payload))
+ "/generate", "POST", encrypt=True, json=payload)
return {
"fingerprint": ret["fingerprint"],
"public-key": base64.b64decode(ret["public-key"])}
@@ -184,41 +189,44 @@ class SigningServiceClient:
if key_type not in SigningKeyType.items:
raise ValueError("%s is not a valid key type" % key_type)
- nonce = self.getNonce()
- response_nonce = self._makeResponseNonce()
- payload = json.dumps({
+ payload = {
"key-type": key_type.name,
"fingerprint": fingerprint,
"message-name": message_name,
"message": base64.b64encode(message).decode("UTF-8"),
"mode": mode.name,
- }).encode("UTF-8")
- data = self._requestJson(
- "/sign", "POST",
- headers=self._getAuthHeaders(nonce, response_nonce),
- data=self._encryptPayload(nonce, payload))
+ }
+ ret = self._requestJson("/sign", "POST", encrypt=True, json=payload)
return {
- 'public-key': base64.b64decode(data['public-key']),
- 'signed-message': base64.b64decode(data['signed-message'])}
+ "public-key": base64.b64decode(ret["public-key"]),
+ "signed-message": base64.b64decode(ret["signed-message"])}
def inject(self, key_type, private_key, public_key, description,
created_at):
if key_type not in SigningKeyType.items:
raise ValueError("%s is not a valid key type" % key_type)
- nonce = self.getNonce()
- response_nonce = self._makeResponseNonce()
- payload = json.dumps({
+ payload = {
"key-type": key_type.name,
"private-key": base64.b64encode(private_key).decode("UTF-8"),
"public-key": base64.b64encode(public_key).decode("UTF-8"),
"created-at": created_at.isoformat(),
"description": description,
- }).encode("UTF-8")
+ }
+
+ ret = self._requestJson("/inject", "POST", encrypt=True, json=payload)
+ return {"fingerprint": ret["fingerprint"]}
+
+ def addAuthorization(self, key_type, fingerprint, client_name):
+ if key_type not in SigningKeyType.items:
+ raise ValueError("%s is not a valid key type" % key_type)
+
+ payload = {
+ "key-type": key_type.name,
+ "fingerprint": fingerprint,
+ "client-name": client_name,
+ }
- data = self._requestJson(
- "/inject", "POST",
- headers=self._getAuthHeaders(nonce, response_nonce),
- data=self._encryptPayload(nonce, payload))
- return {"fingerprint": data["fingerprint"]}
+ self._requestJson(
+ "/authorizations/add", "POST", encrypt=True, json=payload)
diff --git a/lib/lp/services/signing/tests/test_proxy.py b/lib/lp/services/signing/tests/test_proxy.py
index d9c760d..f50aa8c 100644
--- a/lib/lp/services/signing/tests/test_proxy.py
+++ b/lib/lp/services/signing/tests/test_proxy.py
@@ -160,6 +160,11 @@ class SigningServiceResponseFactory:
}, nonce=self.response_nonce),
status=200)
+ responses.add(
+ responses.POST, self.getUrl("/authorizations/add"),
+ body=self._encryptPayload({}, nonce=self.response_nonce),
+ status=200)
+
call_counts = {'/sign': 0}
def sign_callback(request):
@@ -563,3 +568,68 @@ class SigningServiceProxyTest(TestCaseWithFactory, TestWithFixtures):
'shrug', bytes(private_key), bytes(public_key),
"This is a test key injected.", datetime.now())
self.assertEqual(0, len(responses.calls))
+
+ @responses.activate
+ def test_addAuthorization_invalid_key_type(self):
+ signing = getUtility(ISigningServiceClient)
+ self.assertRaises(
+ ValueError, signing.addAuthorization,
+ "shrug", "fingerprint", "test-client")
+ self.assertEqual(0, len(responses.calls))
+
+ @responses.activate
+ def test_addAuthorization(self):
+ # Replace GET /service-key response by our mock.
+ resp_factory = self.response_factory
+ resp_factory.addResponses(self)
+
+ fingerprint = self.factory.getUniqueHexString(40).upper()
+ key_type = SigningKeyType.KMOD
+ client_name = "test-client"
+
+ signing = getUtility(ISigningServiceClient)
+ self.assertIsNone(
+ signing.addAuthorization(key_type, fingerprint, client_name))
+
+ self.assertEqual(3, len(responses.calls))
+ # expected order of HTTP calls
+ http_nonce, http_service_key, http_add_authorization = responses.calls
+
+ self.assertEqual("POST", http_nonce.request.method)
+ self.assertEqual(
+ self.response_factory.getUrl("/nonce"), http_nonce.request.url)
+
+ self.assertEqual("GET", http_service_key.request.method)
+ self.assertEqual(
+ self.response_factory.getUrl("/service-key"),
+ http_service_key.request.url)
+ self.assertThat(http_add_authorization.request.headers, ContainsDict({
+ "Content-Type": Equals("application/x-boxed-json"),
+ "X-Client-Public-Key": Equals(config.signing.client_public_key),
+ "X-Nonce": Equals(self.response_factory.b64_nonce),
+ "X-Response-Nonce": Equals(
+ self.response_factory.b64_response_nonce),
+ }))
+ self.assertThat(
+ http_add_authorization.request.body,
+ AfterPreprocessing(
+ self.response_factory._decryptPayload,
+ MatchesDict({
+ "key-type": Equals("KMOD"),
+ "fingerprint": Equals(fingerprint),
+ "client-name": Equals(client_name),
+ })))
+
+ self.assertTimeline([
+ ("POST", "/nonce", {}),
+ ("GET", "/service-key", {}),
+ ("POST", "/authorizations/add", {
+ "headers": {
+ "Content-Type": "application/x-boxed-json",
+ "X-Client-Public-Key": config.signing.client_public_key,
+ "X-Nonce": self.response_factory.b64_nonce,
+ "X-Response-Nonce":
+ self.response_factory.b64_response_nonce,
+ },
+ }),
+ ])
diff --git a/lib/lp/services/signing/tests/test_signingkey.py b/lib/lp/services/signing/tests/test_signingkey.py
index 0259946..00714f9 100644
--- a/lib/lp/services/signing/tests/test_signingkey.py
+++ b/lib/lp/services/signing/tests/test_signingkey.py
@@ -217,6 +217,29 @@ class TestSigningKey(TestCaseWithFactory, TestWithFixtures):
"mode": Equals("CLEAR"),
}))))
+ @responses.activate
+ def test_addAuthorization(self):
+ self.signing_service.addResponses(self)
+
+ s = SigningKey(
+ SigningKeyType.UEFI, u"a fingerprint",
+ bytes(self.signing_service.generated_public_key),
+ description=u"This is my key!")
+ self.assertIsNone(s.addAuthorization(u"another-client"))
+
+ self.assertEqual(3, len(responses.calls))
+ self.assertThat(
+ responses.calls[2].request,
+ MatchesStructure(
+ url=Equals(self.signing_service.getUrl("/authorizations/add")),
+ body=AfterPreprocessing(
+ self.signing_service._decryptPayload,
+ MatchesDict({
+ "key-type": Equals("UEFI"),
+ "fingerprint": Equals(u"a fingerprint"),
+ "client-name": Equals(u"another-client"),
+ }))))
+
class TestArchiveSigningKey(TestCaseWithFactory):
layer = ZopelessDatabaseLayer
