Techdebt: Remove ECDSA dependency (#7356)

2024-02-18 15:30:33 +00:00 · 2024-02-18 15:30:33 +00:00 · a54cb2937f
commit a54cb2937f
parent 3f65f94675
6 changed files with 146 additions and 54 deletions
--- a/docs/requirements.txt
+++ b/docs/requirements.txt
@ -6,4 +6,4 @@ readthedocs-sphinx-search
 docker
 openapi_spec_validator
 PyYAML>=5.1
-python-jose[cryptography]>=3.1.0,<4.0.0
+joserfc>=0.9.0
--- a/moto/cognitoidp/models.py
+++ b/moto/cognitoidp/models.py
@ -8,7 +8,7 @@ import typing
 from collections import OrderedDict
 from typing import Any, Dict, List, Optional, Set, Tuple

-from jose import jws
+from joserfc import jwk, jwt

 from moto.core.base_backend import BackendDict, BaseBackend
 from moto.core.common_models import BaseModel
@ -444,7 +444,7 @@ class CognitoIdpUserPool(BaseModel):
        with open(
            os.path.join(os.path.dirname(__file__), "resources/jwks-private.json")
        ) as f:
-            self.json_web_key = json.loads(f.read())
+            self.json_web_key = jwk.RSAKey.import_key(json.loads(f.read()))

    @property
    def backend(self) -> "CognitoIdpBackend":
@ -543,10 +543,10 @@ class CognitoIdpUserPool(BaseModel):
            "username" if token_use == "access" else "cognito:username": username,
        }
        payload.update(extra_data or {})
-        headers = {"kid": "dummy"}  # KID as present in jwks-public.json
+        headers = {"kid": "dummy", "alg": "RS256"}  # KID as present in jwks-public.json

        return (
-            jws.sign(payload, self.json_web_key, headers, algorithm="RS256"),
+            jwt.encode(headers, payload, self.json_web_key),
            expires_in,
        )

--- a/moto/ec2/utils.py
+++ b/moto/ec2/utils.py
@ -669,31 +669,66 @@ def generate_instance_identity_document(instance: Any) -> Dict[str, Any]:
    return document


+def _convert_rfc4716(data: bytes) -> bytes:
+    """Convert an RFC 4716 public key to OpenSSH authorized_keys format"""
+
+    # Normalize line endings and join continuation lines
+    data_normalized = data.replace(b"\r\n", b"\n").replace(b"\r", b"\n")
+    data_joined = data_normalized.replace(b"\\\n", b"")
+    lines = data_joined.splitlines()
+
+    # Trim header and footer
+    if lines[0] != b"---- BEGIN SSH2 PUBLIC KEY ----":
+        raise ValueError("Invalid RFC4716 header line")
+    if lines[-1] != b"---- END SSH2 PUBLIC KEY ----":
+        raise ValueError("Invalid RFC4716 footer line")
+    lines = lines[1:-1]
+
+    # Leading lines containing a colon are headers
+    headers = {}
+    num_header_lines = 0
+    for line in lines:
+        if b":" not in line:
+            break
+        num_header_lines += 1
+        header_name, header_value = line.split(b": ")
+        headers[header_name.lower()] = header_value
+
+    # Remaining lines are key data
+    data_lines = lines[num_header_lines:]
+    b64_key = b"".join(data_lines)
+
+    # Extract the algo name from the binary packet
+    packet = base64.b64decode(b64_key)
+    alg_len = int.from_bytes(packet[:4], "big")
+    alg = packet[4 : 4 + alg_len]
+
+    result_parts = [alg, b64_key]
+    if b"comment" in headers:
+        result_parts.append(headers[b"comment"])
+    return b" ".join(result_parts)
+
+
 def public_key_parse(
    key_material: Union[str, bytes]
 ) -> Union[RSAPublicKey, Ed25519PublicKey]:
-    # These imports take ~.5s; let's keep them local
-    import sshpubkeys.exceptions
-    from sshpubkeys.keys import SSHKey
-
    try:
-        if not isinstance(key_material, bytes):
+        if isinstance(key_material, str):
            key_material = key_material.encode("ascii")
+        key_material = base64.b64decode(key_material)

-        decoded_key = base64.b64decode(key_material)
-        public_key = SSHKey(decoded_key.decode("ascii"))
-    except (sshpubkeys.exceptions.InvalidKeyException, UnicodeDecodeError):
+        if key_material.startswith(b"---- BEGIN SSH2 PUBLIC KEY ----"):
+            # cryptography doesn't parse RFC4716 key format, so we have to convert it first
+            key_material = _convert_rfc4716(key_material)
+
+        public_key = serialization.load_ssh_public_key(key_material)
+
+        if not isinstance(public_key, (RSAPublicKey, Ed25519PublicKey)):
+            raise ValueError("bad key")
+    except UnicodeDecodeError:
        raise ValueError("bad key")

-    if public_key.rsa:
-        return public_key.rsa
-
-    # `cryptography` currently does not support RSA RFC4716/SSH2 format, otherwise we could get rid of `sshpubkeys` and
-    # simply use `load_ssh_public_key()`
-    if public_key.key_type == b"ssh-ed25519":
-        return serialization.load_ssh_public_key(decoded_key)  # type: ignore[return-value]
-
-    raise ValueError("bad key")
+    return public_key


 def public_key_fingerprint(public_key: Union[RSAPublicKey, Ed25519PublicKey]) -> str:
--- a/setup.cfg
+++ b/setup.cfg
@ -44,13 +44,11 @@ moto = py.typed

 [options.extras_require]
 all =
-    python-jose[cryptography]>=3.1.0,<4.0.0
-    ecdsa!=0.15
+    joserfc>=0.9.0
    docker>=3.0.0
    graphql-core
    PyYAML>=5.1
    cfn-lint>=0.40.0
-    sshpubkeys>=3.1.0
    openapi-spec-validator>=0.5.0
    pyparsing>=3.0.7
    jsondiff>=1.1.2
@ -59,13 +57,11 @@ all =
    setuptools
    multipart
 proxy =
-    python-jose[cryptography]>=3.1.0,<4.0.0
-    ecdsa!=0.15
+    joserfc>=0.9.0
    docker>=2.5.1
    graphql-core
    PyYAML>=5.1
    cfn-lint>=0.40.0
-    sshpubkeys>=3.1.0
    openapi-spec-validator>=0.5.0
    pyparsing>=3.0.7
    jsondiff>=1.1.2
@ -74,13 +70,11 @@ proxy =
    setuptools
    multipart
 server =
-    python-jose[cryptography]>=3.1.0,<4.0.0
-    ecdsa!=0.15
+    joserfc>=0.9.0
    docker>=3.0.0
    graphql-core
    PyYAML>=5.1
    cfn-lint>=0.40.0
-    sshpubkeys>=3.1.0
    openapi-spec-validator>=0.5.0
    pyparsing>=3.0.7
    jsondiff>=1.1.2
@ -94,10 +88,9 @@ acmpca =
 amp =
 apigateway =
    PyYAML>=5.1
-    python-jose[cryptography]>=3.1.0,<4.0.0
-    ecdsa!=0.15
+    joserfc>=0.9.0
    openapi-spec-validator>=0.5.0
-apigatewayv2 = 
+apigatewayv2 =
    PyYAML>=5.1
    openapi-spec-validator>=0.5.0
 applicationautoscaling =
@ -112,13 +105,11 @@ batch_simple =
 budgets =
 ce =
 cloudformation =
-    python-jose[cryptography]>=3.1.0,<4.0.0
-    ecdsa!=0.15
+    joserfc>=0.9.0
    docker>=3.0.0
    graphql-core
    PyYAML>=5.1
    cfn-lint>=0.40.0
-    sshpubkeys>=3.1.0
    openapi-spec-validator>=0.5.0
    pyparsing>=3.0.7
    jsondiff>=1.1.2
@ -133,8 +124,7 @@ codecommit =
 codepipeline =
 cognitoidentity =
 cognitoidp =
-    python-jose[cryptography]>=3.1.0,<4.0.0
-    ecdsa!=0.15
+    joserfc>=0.9.0
 comprehend =
 config =
 databrew =
@ -150,7 +140,7 @@ dynamodbstreams =
    docker>=3.0.0
    py-partiql-parser==0.5.1
 ebs =
-ec2 = sshpubkeys>=3.1.0
+ec2 =
 ec2instanceconnect =
 ecr =
 ecs =
@ -204,8 +194,7 @@ redshiftdata =
 rekognition =
 resourcegroups =
 resourcegroupstaggingapi =
-    python-jose[cryptography]>=3.1.0,<4.0.0
-    ecdsa!=0.15
+    joserfc>=0.9.0
    docker>=3.0.0
    graphql-core
    PyYAML>=5.1
--- a/tests/test_cognitoidp/test_cognitoidp.py
+++ b/tests/test_cognitoidp/test_cognitoidp.py
@ -13,13 +13,17 @@ import boto3
 import pytest
 import requests
 from botocore.exceptions import ClientError, ParamValidationError
-from jose import jws, jwt
+from joserfc import jwk, jws, jwt

 import moto.cognitoidp.models
-from moto import mock_aws, settings
+from moto import cognitoidp, mock_aws, settings
 from moto.cognitoidp.utils import create_id
 from moto.core import DEFAULT_ACCOUNT_ID as ACCOUNT_ID
 from moto.core import set_initial_no_auth_action_count
+from moto.utilities.utils import load_resource
+
+private_key = load_resource(cognitoidp.__name__, "resources/jwks-private.json")
+PUBLIC_KEY = jwk.RSAKey.import_key(private_key)


@mock_aws
@ -1543,7 +1547,8 @@ def test_group_in_access_token():
        ChallengeResponses={"USERNAME": username, "NEW_PASSWORD": new_password},
    )

-    claims = jwt.get_unverified_claims(result["AuthenticationResult"]["AccessToken"])
+    payload = jwt.decode(result["AuthenticationResult"]["AccessToken"], PUBLIC_KEY)
+    claims = payload.claims
    assert claims["cognito:groups"] == [group_name]


@ -1604,7 +1609,8 @@ def test_other_attributes_in_id_token():
        ChallengeResponses={"USERNAME": username, "NEW_PASSWORD": new_password},
    )

-    claims = jwt.get_unverified_claims(result["AuthenticationResult"]["IdToken"])
+    payload = jwt.decode(result["AuthenticationResult"]["IdToken"], PUBLIC_KEY)
+    claims = payload.claims
    assert claims["cognito:groups"] == [group_name]
    assert claims["custom:myattr"] == "some val"

@ -2957,7 +2963,7 @@ def test_token_legitimacy():

    path = "../../moto/cognitoidp/resources/jwks-public.json"
    with open(os.path.join(os.path.dirname(__file__), path)) as f:
-        json_web_key = json.loads(f.read())["keys"][0]
+        json_web_key = jwk.RSAKey.import_key(json.loads(f.read())["keys"][0])

    for auth_flow in ["ADMIN_NO_SRP_AUTH", "ADMIN_USER_PASSWORD_AUTH"]:
        outputs = authentication_flow(conn, auth_flow)
@ -2968,14 +2974,14 @@ def test_token_legitimacy():
        issuer = (
            f"https://cognito-idp.us-west-2.amazonaws.com/{outputs['user_pool_id']}"
        )
-        id_claims = json.loads(jws.verify(id_token, json_web_key, "RS256"))
+        id_claims = jwt.decode(id_token, json_web_key, ["RS256"]).claims
        assert id_claims["iss"] == issuer
        assert id_claims["aud"] == client_id
        assert id_claims["token_use"] == "id"
        assert id_claims["cognito:username"] == username
        for k, v in outputs["additional_fields"].items():
            assert id_claims[k] == v
-        access_claims = json.loads(jws.verify(access_token, json_web_key, "RS256"))
+        access_claims = jwt.decode(access_token, json_web_key, ["RS256"]).claims
        assert access_claims["iss"] == issuer
        assert access_claims["client_id"] == client_id
        assert access_claims["token_use"] == "access"
@ -4938,8 +4944,10 @@ if not settings.TEST_SERVER_MODE:

 def verify_kid_header(token):
    """Verifies the kid-header is corresponds with the public key"""
-    headers = jwt.get_unverified_headers(token)
-    kid = headers["kid"]
+    if isinstance(token, str):
+        token = token.encode("ascii")
+    sig = jws.extract_compact(token)
+    kid = sig.headers()["kid"]

    key_index = -1
    keys = fetch_public_keys()
--- a/tests/test_ec2/test_key_pairs.py
+++ b/tests/test_ec2/test_key_pairs.py
@ -27,7 +27,7 @@ A3t8mL7r91aM5q6QOQm219lctFM8O7HRJnDgmhGpnjRwE1LyKktWTbgFZ4SNWU2X\
 qusUO07jKuSxzPumXBeU+JEtx0J1tqZwJlpGt2R+0qN7nKnPl2+hx \
 moto@github.com"""

-RSA_PUBLIC_KEY_RFC4716 = b"""\
+RSA_PUBLIC_KEY_RFC4716_1 = b"""\
 ---- BEGIN SSH2 PUBLIC KEY ----
 AAAAB3NzaC1yc2EAAAADAQABAAABAQDusXfgTE4eBP50NglSzCSEGnIL6+cr6m3H6cZANO
 Q+P1o/W4BdtcAL3sor4iGi7SOeJgo8kweyMQrhrt6HaKGgromRiz37LQx4YIAcBi4Zd023
@ -38,6 +38,44 @@ wJlpGt2R+0qN7nKnPl2+hx
 ---- END SSH2 PUBLIC KEY ----
 """

+RSA_PUBLIC_KEY_RFC4716_2 = b"""\
+---- BEGIN SSH2 PUBLIC KEY ----
+cOmmENt: moto@github.com
+AAAAB3NzaC1yc2EAAAADAQABAAABAQDusXfgTE4eBP50NglSzCSEGnIL6+cr6m3H6cZANO
+Q+P1o/W4BdtcAL3sor4iGi7SOeJgo8kweyMQrhrt6HaKGgromRiz37LQx4YIAcBi4Zd023
+mO/V7Rc2Chh18mWgLSmA6ng+j37ip6452zxtv0jHAz9pJolbKBpJzbZlPN45ZCTk9ck0fS
+VHRl6VRSSPQcpqi65XpRf+35zNOCGCc1mAOOTmw59Q2a6A3t8mL7r91aM5q6QOQm219lct
+FM8O7HRJnDgmhGpnjRwE1LyKktWTbgFZ4SNWU2XqusUO07jKuSxzPumXBeU+JEtx0J1tqZ
+wJlpGt2R+0qN7nKnPl2+hx
+---- END SSH2 PUBLIC KEY ----
+"""
+
+RSA_PUBLIC_KEY_RFC4716_3 = b"""\
+---- BEGIN SSH2 PUBLIC KEY ----
+Comment: "1024-bit RSA, converted from OpenSSH by me@example.com"
+x-command: /home/me/bin/lock-in-guest.sh
+AAAAB3NzaC1yc2EAAAADAQABAAABAQDusXfgTE4eBP50NglSzCSEGnIL6+cr6m3H6cZANO
+Q+P1o/W4BdtcAL3sor4iGi7SOeJgo8kweyMQrhrt6HaKGgromRiz37LQx4YIAcBi4Zd023
+mO/V7Rc2Chh18mWgLSmA6ng+j37ip6452zxtv0jHAz9pJolbKBpJzbZlPN45ZCTk9ck0fS
+VHRl6VRSSPQcpqi65XpRf+35zNOCGCc1mAOOTmw59Q2a6A3t8mL7r91aM5q6QOQm219lct
+FM8O7HRJnDgmhGpnjRwE1LyKktWTbgFZ4SNWU2XqusUO07jKuSxzPumXBeU+JEtx0J1tqZ
+wJlpGt2R+0qN7nKnPl2+hx
+---- END SSH2 PUBLIC KEY ----
+"""
+
+RSA_PUBLIC_KEY_RFC4716_4 = b"""\
+---- BEGIN SSH2 PUBLIC KEY ----
+Comment: This is my public key for use on \
+servers which I don't like.
+AAAAB3NzaC1yc2EAAAADAQABAAABAQDusXfgTE4eBP50NglSzCSEGnIL6+cr6m3H6cZANO
+Q+P1o/W4BdtcAL3sor4iGi7SOeJgo8kweyMQrhrt6HaKGgromRiz37LQx4YIAcBi4Zd023
+mO/V7Rc2Chh18mWgLSmA6ng+j37ip6452zxtv0jHAz9pJolbKBpJzbZlPN45ZCTk9ck0fS
+VHRl6VRSSPQcpqi65XpRf+35zNOCGCc1mAOOTmw59Q2a6A3t8mL7r91aM5q6QOQm219lct
+FM8O7HRJnDgmhGpnjRwE1LyKktWTbgFZ4SNWU2XqusUO07jKuSxzPumXBeU+JEtx0J1tqZ
+wJlpGt2R+0qN7nKnPl2+hx
+---- END SSH2 PUBLIC KEY ----
+"""
+
 RSA_PUBLIC_KEY_FINGERPRINT = "6a:49:07:1c:7e:bd:d2:bd:96:25:fe:b5:74:83:ae:fd"

 DSA_PUBLIC_KEY_OPENSSH = b"""ssh-dss \
@ -157,10 +195,20 @@ def test_key_pairs_delete_exist_boto3():
    "public_key,fingerprint",
    [
        (RSA_PUBLIC_KEY_OPENSSH, RSA_PUBLIC_KEY_FINGERPRINT),
-        (RSA_PUBLIC_KEY_RFC4716, RSA_PUBLIC_KEY_FINGERPRINT),
+        (RSA_PUBLIC_KEY_RFC4716_1, RSA_PUBLIC_KEY_FINGERPRINT),
+        (RSA_PUBLIC_KEY_RFC4716_2, RSA_PUBLIC_KEY_FINGERPRINT),
+        (RSA_PUBLIC_KEY_RFC4716_3, RSA_PUBLIC_KEY_FINGERPRINT),
+        (RSA_PUBLIC_KEY_RFC4716_4, RSA_PUBLIC_KEY_FINGERPRINT),
        (ED25519_PUBLIC_KEY_OPENSSH, ED25519_PUBLIC_KEY_FINGERPRINT),
    ],
-    ids=["rsa-openssh", "rsa-rfc4716", "ed25519"],
+    ids=[
+        "rsa-openssh",
+        "rsa-rfc4716-1",
+        "rsa-rfc4716-2",
+        "rsa-rfc4716-3",
+        "rsa-rfc4716-4",
+        "ed25519",
+    ],
 )
 def test_key_pairs_import_boto3(public_key, fingerprint):
    client = boto3.client("ec2", "us-west-1")
@ -188,6 +236,18 @@ def test_key_pairs_import_boto3(public_key, fingerprint):
    assert kp1["KeyName"] in all_names


+@mock_aws
+def test_key_pairs_import_invalid_key():
+    client = boto3.client("ec2", "us-west-1")
+
+    with pytest.raises(ClientError) as exc:
+        client.import_key_pair(
+            KeyName="sth", PublicKeyMaterial="---- BEGIN SSH2 PUBLIC KEY ----\nsth"
+        )
+    err = exc.value.response["Error"]
+    assert err["Code"] == "InvalidKeyPair.Format"
+
+
@mock_aws
 def test_key_pairs_import_exist_boto3():
    client = boto3.client("ec2", "us-west-1")