Security fixes for opensaml2 and xmltooling

[Florian Weimer]

* Scott Cantor:

> Russ Allbery wrote on 2009-09-23:
>>> Is 20090826 remotely exploitable?  Is authentication required?
>> 
>> My guess, although I'm not certain, is that this is potentially remotely
>> exploitable without authentication because it's in the low level URL
>> parsing code, which may used for any data passed to the SP via a URL.  All
>> you need to know is an end point that can take information via GET.
>
> The exploit is a classic buffer overrun caused by the URL parsing
> code, so it "merely" requires injecting binary data onto the
> malformed URL and getting the OS to execute it. Definitely doesn't
> require authentication, no.

Okay, so this should probably get a DSA.

> And I can't correct the function name without going to library version 2.x
> (bumping opensaml to 3.x, etc.) The patch fix is to propagate the
> misspelling and then change them all later.

Thanks for this concern for binary compatiblity.

However, we do have a slight problem with this patch:

--- opensaml2-2.0.orig/saml/saml2/metadata/MetadataCredentialCriteria.h
+++ opensaml2-2.0/saml/saml2/metadata/MetadataCredentialCriteria.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2001-2007 Internet2
+ *  Copyright 2001-2009 Internet2
  * 
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -64,10 +64,10 @@
                 const MetadataCredentialContext* context = dynamic_cast<const MetadataCredentialContext*>(credential.getCredentalContext());
                 if (context) {
                     // Check for a usage mismatch.
-                    if ((getUsage() | (xmltooling::Credential::SIGNING_CREDENTIAL & xmltooling::Credential::TLS_CREDENTIAL)) &&
+                    if ((getUsage() & (xmltooling::Credential::SIGNING_CREDENTIAL | xmltooling::Credential::TLS_CREDENTIAL)) &&
                             XMLString::equals(context->getKeyDescriptor().getUse(),KeyDescriptor::KEYTYPE_ENCRYPTION))
                         return false;
-                    else if ((getUsage() | xmltooling::Credential::ENCRYPTION_CREDENTIAL) &&
+                    else if ((getUsage() & xmltooling::Credential::ENCRYPTION_CREDENTIAL) &&
                             XMLString::equals(context->getKeyDescriptor().getUse(),KeyDescriptor::KEYTYPE_SIGNING))
                         return false;
                 }


This touches a function implicitly declared inline.  We have to
rebuild all reverse build dependencies (direct and indirect) which
instantiate MetadataCredentialCriteria objects because each such
compilation unit has received a local copy of the function.  (There
isn't any out-of-line method in the class, so there is no compilation
unit which contains the class vtable and a master copy of that
method.)

You really should move all definitions of virtual methods (in
particular, virtual destructors) to .cpp files.  With the exception of
destructors, they can't be inlined without whole-program analysis
anyway.  Of course, this is not suitable for a security update because
it breaks the ABI.

I'm afraid, but what initially looked like a clean and simple update
looks increasingly messy. 8-(

Could you check which packages in Debian instantiate
MetadataCredentialCriteria objects?