According to OpenID 2.0 section 10

If the Relying Party supplied an association handle with the authentication request, the OP SHOULD attempt to look up an association based on that handle. If the association is missing or expired, the OP SHOULD send the "openid.invalidate_handle" parameter as part of the response with the value of the request's "openid.assoc_handle" parameter, and SHOULD proceed as if no association handle was specified.

This is not a security issue as the signature will fail and openid_complete will not be able to verify responses. The bug 1) is against spec and 2) blocks sign-in for certain OPs when their authentication handles expire.

CommentFileSizeAuthor
#11 openid-invalidate-handle-1-D6.patch3.4 KBc960657
#8 730462-invalid-handle-response.patch2.89 KBDamien Tournoud
PASSED: [[SimpleTest]]: [MySQL] 18,684 pass(es). View
#5 do-730462-invalidate_handle.patch1.76 KBHeine
PASSED: [[SimpleTest]]: [MySQL] 18,682 pass(es). View
#4 do-730462.patch1.76 KBHeine
FAILED: [[SimpleTest]]: [MySQL] 18,144 pass(es), 48 fail(s), and 27 exception(es). View
#2 730462-invalidate-handle.patch1.17 KBDamien Tournoud
PASSED: [[SimpleTest]]: [MySQL] 18,197 pass(es). View
Join #DrupalCares to support the DA through COVID-19. Drupal Association Give today

Comments

Damien Tournoud’s picture

Hm. The way I'm reading that is that when the request openid.assoc_handle is expired, the OP has to "proceed as if no association handle was specified" (ie. as if that field was not in the request), and has to put the expired association handle in the response as openid.invalidate_handle.

As a consequence, the response should not contain any openid.assoc_handle, and openid_verify_assertion() will proceed with direct verification.

So, except if the OP doesn't comply with the spec, nothing wrong should happen. The only issue is that we will continue to send invalid association handles to this OP, that will have to be verified in the (less performant) direct verification mode.

Damien Tournoud’s picture

Status: Active » Needs review
FileSize
1.17 KB
PASSED: [[SimpleTest]]: [MySQL] 18,197 pass(es). View

Anyway, this should do it.

Damien Tournoud’s picture

Just to clarify #1, my analysis is that 1) we do not violate the spec here, because the spec only specify the behavior of the OP, not of the RP, 2) nothing wrong should happen as long as the RP comply with the spec.

Heine’s picture

Status: Needs review » Needs work
FileSize
1.76 KB
FAILED: [[SimpleTest]]: [MySQL] 18,144 pass(es), 48 fail(s), and 27 exception(es). View

Alas,

If we receive an invalid_handle response, we must verify directly with the OP (we do now, assuming the OP is well-behaved and sends an empty assoc_handle).

Only then, when the OP confirms that the handle is in valid should we remove it. see section 11.4.2.2:

Note: This two-step process for invalidating associations is necessary to prevent an attacker from invalidating an association at will by adding "invalidate_handle" parameters to an authentication response.

TODO - I still need to look into 11.4.2.1. about shared keys.
TODO - we must do ns checking - maybe in a separate issue?

Heine’s picture

Status: Needs work » Needs review
FileSize
1.76 KB
PASSED: [[SimpleTest]]: [MySQL] 18,682 pass(es). View

Gah, wrong patch.

Status: Needs review » Needs work

The last submitted patch, do-730462-invalidate_handle.patch, failed testing.

Heine’s picture

Status: Needs work » Needs review

#5: do-730462-invalidate_handle.patch queued for re-testing.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
2.89 KB
PASSED: [[SimpleTest]]: [MySQL] 18,684 pass(es). View

Rerolled with a some additional comments and a smallish style issue.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

Status: Fixed » Closed (fixed)

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

c960657’s picture

Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Needs review
FileSize
3.4 KB

D6 backport.

pwolanin’s picture

still an open core bug

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.