As a follow up of #302632: Use PHP5 functionality for _openid_sha1(). Here is a patch that tests if hash extension (Default in PHP5) is available and use hash_hmac() function in _openid_hmac().

See chx's comment for details on why we can't just rely on this extension being available:

#302632-8: Use PHP5 functionality for _openid_sha1()

CommentFileSizeAuthor
#16 openid_hmac-D7-3.patch768 bytesc960657
#13 drupal8.openid-hmac.13.patch1.39 KBsun
#10 openid_hmac-A-3.patch1.4 KBc960657
#7 openid_hmac-A-2.patch2.14 KBc960657
#7 openid_hmac-B-2.patch1.2 KBc960657
#3 openid_hmac.patch1.28 KBrecidive
openid_hmac.patch1.28 KBrecidive

Comments

recidive’s picture

recidive commented
Title: Use hash extension in _openid_hmac() is available » Use hash extension in _openid_hmac() if available

Typo in title.

cburschka’s picture

cburschka
they
commented
Status: Needs review » Needs work

This patch looks ready to go, except for one blank line containing two spaces.

recidive’s picture

recidive commented
Status: Needs work » Needs review
StatusFileSize
new openid_hmac.patch1.28 KB

Ok, this time without the spaces.

dries’s picture

dries commented

Personally, I'd not bother with having two code paths. This code shouldn't be in the critical performance path, so the existing code is probably fine. I'm tempted to mark this "won't fix".

dries’s picture

dries commented
Status: Needs review » Closed (won't fix)

Marking this as "won't fix".

c960657’s picture

c960657 commented

We now require the hash extension (see system_requirements()), so we can now switch completely to hash_hmac().

openid_hmac-A-2.patch removes _openid_hmac() and OPENID_SHA1_BLOCKSIZE completely, openid_hmac-B-2.patch preserves them for backwards-compatibility. It think we should use the former—_openid_hmac() is a private function, and I doubt anybody outside of the OpenID module would use the OPENID_SHA1_BLOCKSIZE constant.

Edit: The patches are attached to the next comment.

c960657’s picture

c960657 commented
Title: Use hash extension in _openid_hmac() if available » Use hash extension in _openid_hmac()
Status: Closed (won't fix) » Needs review
StatusFileSize
new openid_hmac-B-2.patch1.2 KB
new openid_hmac-A-2.patch2.14 KB
recidive’s picture

recidive commented

I also think the "A" patch is better.

c960657’s picture

c960657 commented
Version: 7.x-dev » 8.x-dev

For D8, openid_hmac-A-2.patch is definitely the right choice.

c960657’s picture

c960657 commented
StatusFileSize
new openid_hmac-A-3.patch1.4 KB
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

Awesome.

catch’s picture

catch
he/him
Primary language English
commented

#10: openid_hmac-A-3.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal8.openid-hmac.13.patch1.39 KB

Re-rolled against HEAD.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Looks great. Committed/pushed to 8.x.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

c960657’s picture

c960657 commented
Version: 8.x-dev » 7.x-dev
Status: Closed (fixed) » Needs review
StatusFileSize
new openid_hmac-D7-3.patch768 bytes

Reroll of openid_hmac-A-2.patch for D7.

  • catch committed 06373cf on 8.3.x 
    Issue #448162 by c960657, recidive, sun: Use hash extension in...

  • catch committed 06373cf on 8.3.x 
    Issue #448162 by c960657, recidive, sun: Use hash extension in...

  • catch committed 06373cf on 8.4.x 
    Issue #448162 by c960657, recidive, sun: Use hash extension in...

  • catch committed 06373cf on 8.4.x 
    Issue #448162 by c960657, recidive, sun: Use hash extension in...

  • catch committed 06373cf on 9.1.x 
    Issue #448162 by c960657, recidive, sun: Use hash extension in...

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.