Issue with oauth/token if you have setup hash_salt less then 32 in length [#2935122]

Steps to reproduce:
1. Install the module per documentation.
2. Setup your application/consumer
3. Make sure that you have setup hash_salt in settings.php or settings local and that it is less then 32 in length
4. Try to request token for your consumer on oauth/token

You'll get issue like below in recent log messages:

Defuse\Crypto\Exception\EnvironmentIsBrokenException: Your version of PHP has bug #66797. Its implementation of mb_substr() is incorrect. See the details here: https://bugs.php.net/bug.php?id=66797 in Defuse\Crypto\Core::ourSubstr() (line 335 of /var/www/html/vendor/defuse/php-encryption/src/Core.php).

This is because Oauth2GrantManager has 32 as length passed to ourSubstr and it then function compares length of 32 to your salt which doesn't have to be 32 in length and then it fails with error which falsely represents the issue here.

Proposed solution

Since this is not something the module can fix for you, we should fail with a clear error message if the the salt is shorter than 32 characters.

Comment	File	Size	Author
#8	simple_oauth-hash-salt-2935122-8.patch	681 bytes	Berdir
#8
#2	2935122-2.patch	588 bytes	xSDx
#2	8.x-3.x: PHP 7 & MySQL 5.5, D8.4 21 pass, 2 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

8 January 2018 at 12:34

xSDx created an issue. See original summary.

Comment #2

xSDx CreditAttribution: xSDx at Websolutions Agency commented 8 January 2018 at 12:37

File	Size
2935122-2.patch	588 bytes
8.x-3.x: PHP 7 & MySQL 5.5, D8.4 21 pass, 2 fail

Attaching patch

Comment #3

xSDx CreditAttribution: xSDx at Websolutions Agency commented 8 January 2018 at 15:16

Title:

Issue with oauth/token if you have setup hash_salt other then 32 in length

» Issue with oauth/token if you have setup hash_salt less then 32 in length

Comment #4

xSDx CreditAttribution: xSDx at Websolutions Agency commented 8 January 2018 at 15:16

Issue summary:

View changes

Comment #5

Berdir

German

Switzerland

CreditAttribution: Berdir at MD Systems GmbH commented 10 January 2018 at 21:11

I noticed this too but the patch doesn't really help. The correct fix is IMHO to throw a clearer exception if the has is too short instead of the very confusing one about that mb_string bug, this is a developer problem that can not be fixed automatically, so we should just fail with a clear error.