morepath / more.jwtauth

JWT Authentication integration for Morepath
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Dependency problem on OpenSSL version via Cryptography package. #3

Closed taschini closed 8 years ago

taschini commented 8 years ago

It might happen that during the automatic installation of more.jwauth , e.g. via Buildout, the version of the OpenSSL library linked to the Cryptography package is not recent enough. When that happens, Cryptography disables some encryption algorithms, and this typically causes problems at run-time.

While one might think of how to fix this non-Python dependency in Buildout, I think that at least more.jwauth should detect this situation as early as possible and raise a helpful warning directing people to the instructions to manually install Cryptography on the appropriate OS.

When I ran the test-suite with a Cryptography package that was thus restricted, I obtained the following traceback, which could be useful as a source of inspiration on how to implement this safety check:

________________________________ test_encode_decode_with_ps384 _________________________________

    def test_encode_decode_with_ps384():
        identity_policy = JWTIdentityPolicy(
            algorithm='PS384',
            private_key=get_data('tests/keys/testkey_rsa'),
            public_key=get_data('tests/keys/testkey_rsa.pub')
        )
        claims_set = {
            'sub': 'user'
        }
>       token = identity_policy.encode_jwt(claims_set)

more/jwtauth/tests/test_jwtauth.py:122: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
more/jwtauth/main.py:229: in encode_jwt
    token = jwt.encode(claims_set, key, algorithm)
jwt/api_jwt.py:56: in encode
    json_payload, key, algorithm, headers, json_encoder
jwt/api_jws.py:98: in encode
    signature = alg_obj.sign(signing_input, key)
jwt/algorithms.py:268: in sign
    self.hash_alg()
cryptography/hazmat/backends/openssl/rsa.py:531: in signer
    return _RSASignatureContext(self._backend, self, padding, algorithm)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <cryptography.hazmat.backends.openssl.rsa._RSASignatureContext object at 0x1042b0850>
backend = <cryptography.hazmat.backends.openssl.backend.Backend object at 0x103d17310>
private_key = <cryptography.hazmat.backends.openssl.rsa._RSAPrivateKey object at 0x1042b0750>
padding = <cryptography.hazmat.primitives.asymmetric.padding.PSS object at 0x1042b0790>
algorithm = <cryptography.hazmat.primitives.hashes.SHA384 object at 0x1042b0810>

    def __init__(self, backend, private_key, padding, algorithm):
        self._backend = backend
        self._private_key = private_key

        if not isinstance(padding, AsymmetricPadding):
            raise TypeError("Expected provider of AsymmetricPadding.")

        self._pkey_size = self._backend._lib.EVP_PKEY_size(
            self._private_key._evp_pkey
        )
        self._backend.openssl_assert(self._pkey_size > 0)

        if isinstance(padding, PKCS1v15):
            if self._backend._lib.Cryptography_HAS_PKEY_CTX:
                self._finalize_method = self._finalize_pkey_ctx
                self._padding_enum = self._backend._lib.RSA_PKCS1_PADDING
            else:
                self._finalize_method = self._finalize_pkcs1
        elif isinstance(padding, PSS):
            if not isinstance(padding._mgf, MGF1):
                raise UnsupportedAlgorithm(
                    "Only MGF1 is supported by this backend.",
                    _Reasons.UNSUPPORTED_MGF
                )

            # Size of key in bytes - 2 is the maximum
            # PSS signature length (salt length is checked later)
            if self._pkey_size - algorithm.digest_size - 2 < 0:
                raise ValueError("Digest too large for key size. Use a larger "
                                 "key.")

            if not self._backend._mgf1_hash_supported(padding._mgf._algorithm):
                raise UnsupportedAlgorithm(
                    "When OpenSSL is older than 1.0.1 then only SHA1 is "
                    "supported with MGF1.",
>                   _Reasons.UNSUPPORTED_HASH
                )
E               UnsupportedAlgorithm: When OpenSSL is older than 1.0.1 then only SHA1 is supported with MGF1.
henri-hulski commented 8 years ago

It seems that this concerns only buildout and not a recent Pip (>= 8) setup:

The wheel package on OS X is a statically linked build (as of 1.0.1) so for users with pip 8 or above you only need one step:

$ pip install cryptography

See https://cryptography.io/en/latest/installation.

So maybe it's possible to add a warning to the buildout process somewhere in buildout.cfg.

Maybe someone with more buildout experience can have a look.

taschini commented 8 years ago

Binary wheels on OS X are affected by the same UCS2/UCS4 issue that rendered them pretty much useless on Linux. So, I had to build my own Cryptography, following the instructions with static linking a few paragraphs below.

henri-hulski commented 8 years ago

Alternatively we could make Cryptography optional.

See http://pyjwt.readthedocs.io/en/latest/installation.html.

taschini commented 8 years ago

I am not sure that making Cryptography optional would ultimately make life any easier.

henri-hulski commented 8 years ago

Thought about introducing the legacy dependencies but they really seems to outdated.

Making Cryptography optional at least would allow using the HMAC hash algorithms without installing Cryptography.

Just pushing a new branch 'optional_cryptography', where I start to try it out. For Pip pip install more.jwtauth should install more.jwtauth without Cryptography and pip install more.jwtauth[cryptography] with Cryptography.

Don't know how to do this with Buildout.

Also skipping now tests, which depends on Cryptography when it's not installed.

Of course we have to document this.

henri-hulski commented 8 years ago

Thought about introducing the legacy dependencies but they really seems to outdated.

That will say allowing something like pip install more.jwtauth[legacy]. See http://pyjwt.readthedocs.io/en/latest/installation.html#legacy-dependencies.

henri-hulski commented 8 years ago

In the 'optional_cryptography' branch i make Cryptography now optional. I also updated the README and added install instructions.

@taschini Please take a look and tell me what you think and if something is missing.

taschini commented 8 years ago

It looks good to me.

By default you provide the safe option: no crypto, no trouble. If you need any of the algorithms that do require Cryptography, you must install it explicitly and hopefully you did that because you read the Installation notes.

I see that Tox and Travis have now the full matrix Python-version X Install-option, and everything seems to working as expected.

I would only modify one thing in README.rst: I would mark with an asterisk the algorithms under the Algorithms that need the “crypto” extra, and have an explanation like:

*) The marked algorithms require this library to be installed with its crypto dependencies:

pip install -U more.jwtauth[crypto]

See Installation for details. In case of problems be sure to have read the note in the Requirements section.

henri-hulski commented 8 years ago

@taschini I've now merged this into master. Can you try if it now works with buildout for you?

taschini commented 8 years ago

I confirm that buildout works: it picks the safe option of no Cryptography.

henri-hulski commented 8 years ago

Thanks.