Security fix diffs for 2.x

[Scott Cantor]

Ferenc Wagner wrote on 2009-11-24:
>  That's just the nature of static, see for example 4. on
> http://www.cppreference.com/wiki/keywords/static

I guess I was referring to an extern but non-exported function, but on Unix
there's no reliable support for hiding symbols. The gcc options for that
don't work in any rational way based on the testing I've done.

> C and C++ just don't provide the middle ground, as far as I know.

Not built-in to the language, no. It's a compiler issue to handle hiding
symbols from shared library callers but it isn't mature with gcc.

> But now I wonder why the implementations in SAMLConfig.cpp and
SPConfig.cpp
> wouldn't clash...  Shouldn't one be renamed at least?  I fear these
> won't be usable together, but can't check it right now.

They'd be in different C++ namespaces, or could be static members of
different classes, or both.

>> The fix itself:
>> http://svn.middleware.georgetown.edu/view/cpp-sp?view=rev&revision=3183
> 
> I've got problems with the shibsp/impl/XMLServiceProvider.cpp changes.
> The first hunk fails, but I hope it does not matter, because the third
> hunk is the new parser, and the second seems to be something unrelated.
> Unless it's another security fix, this changeset could be dropped
> altogether.  Is it?

With your changes, yes, that's not going to be used. The first two changes
there are unrelated cleanup (switching to new code to handle hashing) and
the last change is to initialize the config setting you aren't adding.

> On the practical side, it seems harder to find a good place for the
> function definitions in the SP, because shibsp/internal.h is not
> included by mod_apache, isapi, nsapi, and fastcgi, so they can't find
> the declaration by default.  I've made up a new header for this purpose
> alone, hope it makes sense, please check the attached patch.

If you're ending up with a new static symbol anyway, you could just add it
to a public header if you wanted to.

Given the additional exported symbol that doesn't break the ABI (on Unix
anyway), I think you need to adjust the soname to reflect that it's not a
revision of the old API but an additional API superset that also supports
the original. I think that involves changing the third number? I'd have to
go back and check.
 
-- Scott