diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/certificate.py b/sdk/identity/azure-identity/azure/identity/_credentials/certificate.py index bec722cf8e19..20de6f5a79d5 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/certificate.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/certificate.py @@ -6,7 +6,8 @@ from typing import TYPE_CHECKING from cryptography import x509 -from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey from cryptography.hazmat.backends import default_backend import six @@ -20,6 +21,8 @@ class CertificateCredential(ClientCredentialBase): """Authenticates as a service principal using a certificate. + The certificate must have an RSA private key, because this credential signs assertions using RS256. + :param str tenant_id: ID of the service principal's tenant. Also called its 'directory' ID. :param str client_id: the service principal's client ID :param str certificate_path: path to a PEM-encoded certificate file including the private key. If not provided, @@ -73,11 +76,15 @@ def get_client_credential(certificate_path, password=None, certificate_bytes=Non with open(certificate_path, "rb") as f: certificate_bytes = f.read() elif not certificate_bytes: - raise ValueError('This credential requires a value for "certificate_path" or "certificate_bytes"') + raise ValueError('CertificateCredential requires a value for "certificate_path" or "certificate_bytes"') if isinstance(password, six.text_type): password = password.encode(encoding="utf-8") + private_key = serialization.load_pem_private_key(certificate_bytes, password=password, backend=default_backend()) + if not isinstance(private_key, RSAPrivateKey): + raise ValueError("CertificateCredential requires an RSA private key because it uses RS256 for signing") + cert = x509.load_pem_x509_certificate(certificate_bytes, default_backend()) fingerprint = cert.fingerprint(hashes.SHA1()) # nosec diff --git a/sdk/identity/azure-identity/azure/identity/_internal/aadclient_certificate.py b/sdk/identity/azure-identity/azure/identity/_internal/aadclient_certificate.py index 271fcd42dcbc..2b7e09ce453f 100644 --- a/sdk/identity/azure-identity/azure/identity/_internal/aadclient_certificate.py +++ b/sdk/identity/azure-identity/azure/identity/_internal/aadclient_certificate.py @@ -8,6 +8,7 @@ from cryptography import x509 from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import padding +from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey from cryptography.hazmat.backends import default_backend import six @@ -19,14 +20,18 @@ class AadClientCertificate(object): """Wraps 'cryptography' to provide the crypto operations AadClient requires for certificate authentication. - :param bytes pem_bytes: bytes of a a PEM-encoded certificate including the private key + :param bytes pem_bytes: bytes of a a PEM-encoded certificate including the (RSA) private key :param bytes password: (optional) the certificate's password """ def __init__(self, pem_bytes, password=None): # type: (bytes, Optional[bytes]) -> None + private_key = serialization.load_pem_private_key(pem_bytes, password=password, backend=default_backend()) + if not isinstance(private_key, RSAPrivateKey): + raise ValueError("CertificateCredential requires an RSA private key because it uses RS256 for signing") + self._private_key = private_key + cert = x509.load_pem_x509_certificate(pem_bytes, default_backend()) fingerprint = cert.fingerprint(hashes.SHA1()) # nosec - self._private_key = serialization.load_pem_private_key(pem_bytes, password=password, backend=default_backend()) self._thumbprint = six.ensure_str(base64.urlsafe_b64encode(fingerprint), encoding="utf-8") @property diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/certificate.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/certificate.py index ef56142a76b5..6400fd793408 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/certificate.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/certificate.py @@ -20,6 +20,8 @@ class CertificateCredential(AsyncContextManager): """Authenticates as a service principal using a certificate. + The certificate must have an RSA private key, because this credential signs assertions using RS256. + :param str tenant_id: ID of the service principal's tenant. Also called its 'directory' ID. :param str client_id: the service principal's client ID :param str certificate_path: path to a PEM-encoded certificate file including the private key. If not provided, diff --git a/sdk/identity/azure-identity/tests/ec-certificate.pem b/sdk/identity/azure-identity/tests/ec-certificate.pem new file mode 100644 index 000000000000..096766daa1e6 --- /dev/null +++ b/sdk/identity/azure-identity/tests/ec-certificate.pem @@ -0,0 +1,35 @@ +-----BEGIN EC PARAMETERS----- +BgUrgQQAIw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MIHcAgEBBEIAlHYv1Pgfi7hayYhJ94kM0iGEoIJEUdK3232zHHxk0f6orHITIk7b +FL90O65CnzZq5rellop1nnD5xGkDyZuqhEGgBwYFK4EEACOhgYkDgYYABACe9HjT +X0bOakJC9SjxO503vCFf1p9r+dcAXo3YGzjeZNPM7qTa51FTXcOYA/4F5gmrkBT6 +ltbcdMJ6AK8bEdUsgAAIkCJjGXLTctcsKHtRphvlMwQTsY3jqkIPv4tymuYjiGCz +V7HlqmFWgYmDXWCVttSNO5Y64uFCu43iaIdkBG7k1Q== +-----END EC PRIVATE KEY----- +-----BEGIN CERTIFICATE----- +MIIEKjCCA4ugAwIBAgIUbkwHWkE2h061m7DrAx8tGB2B6jgwCgYIKoZIzj0EAwIw +RTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGElu +dGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMTAyMTAxODQyNDFaFw0yMzAyMTAx +ODQyNDFaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYD +VQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggJdMIIB0AYHKoZIzj0CATCC +AcMCAQEwTQYHKoZIzj0BAQJCAf////////////////////////////////////// +////////////////////////////////////////////////MIGfBEIB//////// +//////////////////////////////////////////////////////////////// +//////////////wEQgBRlT65YY4cmh+SmiGgtoVA7qLacluZsxXzuLSJkY7xCeFW +GTlR7H6TexZSwL07sb8HNXPfiD0sNPHvRR/Ua1A/AAMVANCeiAApHLhTlsxnFzky +hKqg2mS6BIGFBADGhY4GtwQE6c2ePstmI5W0QpxkgTkFP7Uh+CivYGtNPbqhS153 +7+dZKP4dwSei/6jeM0izwYVqQpv5fn4xwuW9ZgEYOSlqeJo7wARcil+0LH0b2Zj1 +RElXm0RoF6+9Fyc+ZiyX7nKZXvQmQMVQuQE/rQdhNTxwhqJywkCIvpR2n9FmUAJC +Af//////////////////////////////////////////+lGGh4O/L5Zrf8wBSPcJ +pdA7tcm4iZxHrrtvtx6ROGQJAgEBA4GGAAQAOzfbl+0kLDR6w5YXd+hacJYaEbN9 +/QM1DynqYnT4grHLlUG4rmhBNIdu8k77W3mIxvLZY6el1W51yMsArkgeAFwAQWyj +8DTgL4TypCOTegpMl2Mbqcv3A5YYRQ5OkLyZ4ByFdCczqPzcVHHsHcvhAU7hRhPi +Cx6Wf52S6phPShODJlCjUzBRMB0GA1UdDgQWBBSlAcE4Q+qIg5if/3kImBxvCVem +lDAfBgNVHSMEGDAWgBSlAcE4Q+qIg5if/3kImBxvCVemlDAPBgNVHRMBAf8EBTAD +AQH/MAoGCCqGSM49BAMCA4GMADCBiAJCAT2LDMYvahFyVEl60JSl+idJP3nKYUPg +VaAOf75T8HZk6JyYuelpUe79pJr7Wf2msocQLiEfSJk+ziGeokaikzKjAkIBcuN3 +j019ge/uMKqcNn35Y5S5e4rHL7DjUQm0EsJs26/4k30TXlVrK7NOlmawKCcNVIyS ++OhpUmHRHPuOxiYkMJw= +-----END CERTIFICATE----- diff --git a/sdk/identity/azure-identity/tests/test_certificate_credential.py b/sdk/identity/azure-identity/tests/test_certificate_credential.py index f2f756faf24d..0837fd822090 100644 --- a/sdk/identity/azure-identity/tests/test_certificate_credential.py +++ b/sdk/identity/azure-identity/tests/test_certificate_credential.py @@ -42,6 +42,16 @@ (CERT_WITH_PASSWORD_PATH, CERT_PASSWORD.encode("utf-8")), ) +EC_CERT_PATH = os.path.join(os.path.dirname(__file__), "ec-certificate.pem") + + +def test_non_rsa_key(): + """The credential should raise ValueError when given a cert without an RSA private key""" + with pytest.raises(ValueError, match=".*RS256.*"): + CertificateCredential("tenant-id", "client-id", EC_CERT_PATH) + with pytest.raises(ValueError, match=".*RS256.*"): + CertificateCredential("tenant-id", "client-id", certificate_bytes=open(EC_CERT_PATH, "rb").read()) + def test_tenant_id_validation(): """The credential should raise ValueError when given an invalid tenant_id""" diff --git a/sdk/identity/azure-identity/tests/test_certificate_credential_async.py b/sdk/identity/azure-identity/tests/test_certificate_credential_async.py index 61d0ca14ce3c..0432e6c898d0 100644 --- a/sdk/identity/azure-identity/tests/test_certificate_credential_async.py +++ b/sdk/identity/azure-identity/tests/test_certificate_credential_async.py @@ -15,7 +15,15 @@ from helpers import build_aad_response, urlsafeb64_decode, mock_response, Request from helpers_async import async_validating_transport, AsyncMockTransport -from test_certificate_credential import BOTH_CERTS, CERT_PATH, validate_jwt +from test_certificate_credential import BOTH_CERTS, CERT_PATH, EC_CERT_PATH, validate_jwt + + +def test_non_rsa_key(): + """The credential should raise ValueError when given a cert without an RSA private key""" + with pytest.raises(ValueError, match=".*RS256.*"): + CertificateCredential("tenant-id", "client-id", EC_CERT_PATH) + with pytest.raises(ValueError, match=".*RS256.*"): + CertificateCredential("tenant-id", "client-id", certificate_bytes=open(EC_CERT_PATH, "rb").read()) def test_tenant_id_validation():