(followups) Fix OpenID vulnerability from SA-CORE-2012-003 (and backport anything to 7.x-dev as necessary)

Let's also tag this issue as a D7 release blocker, since although the original fix from Drupal 7.16 was merged in to 7.x-dev already, we want to double check that the latest 7.x code is not vulnerable before doing the next bugfix release of Drupal core.

(In this particular case, I think the chance is around 0% that this is the case, but I'm tagging it anyway to keep track of it and because in theory it is always a possibility due to differences between the previous stable release that the security release was made from, and the current code in 7.x-dev. Per the new release policy, we are now able to do this step in the public issue queue as a followup, rather than having it be part of the security team's private work.)

I reviewed and tested this patch and it seemed to work fine. (I'm attaching a version that fixes a code comment typo - missing capital letter at the beginning of a sentence - but no other changes.) This should be good to go for Drupal 8.

I also double checked that the latest 7.x-dev code isn't subject to this vulnerability, so I'm removing that tag as well.

For Drupal 8, we likely could rely on libxml_disable_entity_loader() entirely, but things get a little tricky there (it would require further refactoring to make sure the correct error messages still display and the tests still pass), so no real need to do it here. Removing the doctype entirely should always work anyway.

I was also a bit curious about why the Drupal 8 tests needed to specify the form ID here but the Drupal 7 ones didn't:

+    $this->drupalPost('', $edit, t('Log in'), array(), array(), 'openid-login-form');

But @Berdir pointed out to me that it's because the OpenID forms were refactored in Drupal 8 in #1538462: Cannot log in with OpenID due to "required" attribute so that they're now separate from the user login form.

There's actually a minor bug here:

+  // Protect against malicious doctype declarations and other unexpected entity
+  // loading.
+  $load_entities = libxml_disable_entity_loader(TRUE);
.....
+  // Since DOCTYPE declarations from an untrusted source could be malicious, we
+  // stop parsing here and treat the XML as invalid since XRDS documents do not
+  // require, and are not expected to have, a DOCTYPE.
+  if (isset($dom->doctype)) {
+    return array();
+  }
.....
+  // Return the LIBXML options to the previous state before returning.
+  libxml_disable_entity_loader($load_entities);
+
   return $services;
 }

Since the first return doesn't reset the libxml options to their original state, only the second one.

But that's very minor and exists in Drupal 7 too, so I think this is still RTBC.

There are no longer any security issues in 7.x here, so moving back to 8.x for possible followups discussed above - or it could be a separate issue but I don't have the energy to file one right now :)