I have the strange feeling that this module is not widely used. At the very least, it is certainly only used within the Drupal community, and as such, as seen a small number of people look through the code looking for vulnerabilities and implementation issues.

Considering how complicated and sensitive the OpenID spec is, I think it would be important to review our adherence to the specification before a final release is performed.

There are a few issues that made me kind of freak out, and that I think should be attentively reviewed:

http://lists.openid.net/pipermail/openid-general/2009-April/017584.html - possible DOS attacks on nonce generation (especially with the patch in #1158356: can't login to ikiwiki.info (perl's Net-OpenID-Consumer?))
http://blog.nerdbank.net/2009/03/replay-protection-for-openid-1x-relying... - relay protection for 1.1 RPs, something we're not doing
http://wiki.openid.net/w/page/12995200/OpenID-Security-Best-Practices - clickjacking vulnerability, login_security and securepages integration, provide dummy pages for non-existing accounts, opting out of certain RPs, etc...

Other libraries could provide a more robust provider support:

http://www.dotnetopenauth.net/openid/ - Dot net, but is a good example of a robust library
http://www.janrain.com/openid-enabled - jainrain's de facto standard library
https://openid.net/developers/specs/ - the specs, especially:
https://openid.net/specs/openid-provider-authentication-policy-extension... - PAPE, authentication policies

Comments

anarcat’s picture

anarcat CreditAttribution: anarcat commented

In the spec, this is especially important:

https://openid.net/specs/openid-authentication-2_0.html#security_conside...

anarcat’s picture

anarcat CreditAttribution: anarcat commented
Title: perform security audit or use an existing, well established OP library » perform security audit

I have done a sweep of the standard recently and fixed a bunch of issues, but while implementing #506530: Private association support, I *may* have created an issue, see that ticket for more information.

But just a thorough read of the standard should provide good insight on some stuff we may be missing - again.

anarcat’s picture

anarcat CreditAttribution: anarcat commented

New issues have crept up here, which renders this even more important, see https://en.wikipedia.org/wiki/OpenID#Authentication_bugs for more information.

anarcat’s picture

anarcat CreditAttribution: anarcat commented

... although I wonder if the latter issues were not mitigated by the bugfix in #1441586: SREG and AX keys aren't signed, so they are ignored by Drupal OpenID clients. At least it seems to correspond to the DRUPAL-SA-CORE-2012-001 security advisory. But there's more to check here, and I guess that I'd need to read that research paper and the report from openid.net!