Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 'sha256' instead of 'sha' when validating RSA signing supportability #142

Merged
merged 1 commit into from May 25, 2023

Conversation

pavledjo
Copy link
Contributor

@pavledjo pavledjo commented Mar 6, 2023

In Openssl 3.0.1 in default configuration, jose_server.erl will determine that no RS* algorithms are supported for signing. The reason for this is usage of sha for DigestType when determining RSA signing supportability.

This breaks various flows even though 2 workarounds exist (both of which are not satisfactory enough in our case):

  1. Set JOSE crypto_fallback env var to true
  2. Allow explicitly RSA+SHA1 signatures in openssl config

I propose we change the DigestType to sha256, since sha1 signature creation will probably be disabled in the future on both RHEL 9 and RHEL 10, based on info from [1].

Currently, SHA1+RSA signature creation is not allowed in OpenSSL configuration on public cloud Alma Linux images. Where the config is hardened to disallow certain signature algorithms. On those machines, example output of JOSE.JWA.supports() is as follows:

...
{:jws,
   {:alg, [“ES256”, “ES384", “ES512”, “HS256", “HS384”, “HS512", “Poly1305”]}} 
....

If we use sha256 to create and validate RSA signing algorithms, the supported list becomes correct:

...
{:jws,
   {:alg,  [“ES256”, “ES384", “ES512”, “HS256", “HS384”, “HS512", “PS256”, “PS384", “PS512”, “Poly1305", “RS256”, “RS384", “RS512”]}} 
....

Setup:

Alma Linux 9.1
OpenSSL 3.0.1
Elixir 1.14.0-otp-25
Erlang 25.0.4
jose 1.11.2

[1] openssl/openssl#17662

@pavledjo
Copy link
Contributor Author

pavledjo commented Mar 8, 2023

Hello @potatosalad / @ericmj (sorry if I tagged the wrong people, if so do let me know who I can reach out to); do you have any feedback for the proposed approach? Thanks in advance!

@potatosalad
Copy link
Owner

Awesome, thank you @pavledjo !

@pavledjo
Copy link
Contributor Author

Hi @potatosalad, are there are any obstacles to merging this?

@comanjoni
Copy link

Hi guys, is this going to be released any time soon? I'm interested into this fix too.

@potatosalad
Copy link
Owner

Thanks! Will get a new version tagged and released soon.

@potatosalad potatosalad merged commit fb7547c into potatosalad:main May 25, 2023
2 checks passed
@pavledjo
Copy link
Contributor Author

Sorry to bug you @potatosalad, will the release happen anytime soon? :)

@potatosalad
Copy link
Owner

Sorry to bug you @potatosalad, will the release happen anytime soon? :)

No worries, I apologize for the delayed release. jose 1.11.6 is now published with this fix included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants