Merge #2027

2027: Encode only the public key information r=jaredkerim a=glasserc

Refs #1890. Fixes #2021.

In #2009 I hacked up generation of a public key by serializing a complete certificate. However, `fastecdsa` expects only a public key, which is just one part of the certificate. That the code in #2009 ever worked at all is accidental and almost miraculous. It seems that `fastecdsa` doesn't really do full parsing of the ASN.1 in its input -- it just kind of does a "good enough" job (probably for speed?) and plucks the last vaguely-parametric-looking things out of the file. This means that depending on the other contents of the certificate, it might actually grab the correct `x` and `y` and function correctly. But it can also grab other bit strings and fail catastrophically.

I was able to reproduce this failure by running the contract tests against stage. This patch fixes the tests. However, we've come to this point because I committed code that merely "seemed to work", so I wanted to be much more thorough in my work this time. The cert that we have is an X.509 certificate, described in [RFC 5280](https://tools.ietf.org/html/rfc5280). RFC 7468 [explains](https://tools.ietf.org/html/rfc7468#section-13) that a PEM-encoded public key corresponds to the subject public key information field from RFC 5280, so retrieving this field from the cert and serializing it is correct.

Just to confirm, we can also see that `fastecdsa` specifies in https://github.com/AntonKueltz/fastecdsa/blob/master/fastecdsa/encoding/pem.py#L109 that it accepts elliptic-curve key data in  [RFC 5480](https://tools.ietf.org/html/rfc5480) format. This RFC is the one that adds a format for encoding elliptic-curve keys to RFC 3279, which is the RFC referred to by RFC 5280 as defining the methods for encoding public key materials in X.509 certificates. So again, the public key data from the certificate should be the format that this library expects.

/cc @mozbhearsum @jvehent

Co-authored-by: Ethan Glasser-Camp <ethan@betacantrips.com>

normandy/recipes/signing.py

@@ -104,7 +104,7 @@ def verify_signature_x5u(data, signature, x5u):
     an exception explaining why.
     """
     cert = verify_x5u(x5u)
-    encoded = der_encode(cert)
+    encoded = der_encode(cert["tbsCertificate"]["subjectPublicKeyInfo"])
     cert_b64 = base64.b64encode(encoded).decode()
     return verify_signature_pubkey(data, signature, cert_b64)
 

normandy/recipes/tests/test_signing.py

@@ -173,10 +173,14 @@ def test_happy_path(self, mocker):
 
         mock_der_encode.return_value = cert_contents
 
+        public_key = "public_key"
+        cert = {"tbsCertificate": {"subjectPublicKeyInfo": public_key}}
+        mock_verify_x5u.return_value = cert
+
         ret = signing.verify_signature_x5u(data, signature, x5u)
 
         mock_verify_x5u.assert_called_with(x5u)
-        mock_der_encode.assert_called_with(mock_verify_x5u.return_value)
+        mock_der_encode.assert_called_with(public_key)
         mock_verify_signature_pubkey.assert_called_with(data, signature, encoded_cert_contents)
 
         assert ret == mock_verify_signature_pubkey.return_value