simplesamlphp / saml2

SimpleSAMLphp low-level SAML2 PHP library
https://www.simplesamlphp.org
GNU Lesser General Public License v2.1
284 stars 134 forks source link

Switch to using ass/xmlsecurity #9

Closed relaxnow closed 9 years ago

relaxnow commented 10 years ago

Several forks of Rob Richards XML Security library were attempted, the fork by Andreas Schamberger seems to be the one most far along.

Maybe we could switch?

olavmrk commented 10 years ago

I had a look at it now, and it certainly has cleaner code, and a cleaner API.

One point that would have to be addressed is that I see no easy way to avoid the XML signature wrapping attacks with that library. With the current xmlseclibs, we can just ask "is this node signed", but there is no such function here (as far as I can tell).

A different "problem" that makes me nervous is that switching library will mean changes to the structure of the generated signature and encrypted elements. The changes may be insignificant from a technical point, but I have seen SPs break because their XML signature validator has a bug / makes an assumption that makes it fail if the signature changes slightly. (This is not a blocker, it just means that the upgrade would have to be handled very carefully.)

relaxnow commented 10 years ago

I'm not completely happy with all the 'static' methods and 'self::' calls that just make it completely impossible to override, but at least it's 'cleaner' than the original.

Good point, maybe you can raise it as an issue? See how well Andreas responds.

We'll see how it goes for OpenConext (switching from our Corto signatures to SSP signatures), so far no issues.

olavmrk commented 10 years ago

Done: https://github.com/aschamberger/XmlSecurity/issues/19

treyhyde commented 10 years ago

Have far off do you think this might be? We have a number of important fixes to contribute against the upstream googlecode library and I'd just like to know where to focus my efforts. Thanks

relaxnow commented 10 years ago

Currently this is undecided. We have no concrete plans though. I suspect that neither does UNINETT. I would focus my patches on the Rob Richards repository, if need be they can / will be ported.

cb8 commented 9 years ago

As Rob Richards repository is now dead for more than a year, are there any plans on migrating?

relaxnow commented 9 years ago

Wishes, yes. Strong desires even. But no plans, no funding and no known issues (other than performance on large metadata files) in the old library so far.

thijskh commented 9 years ago

I would like to note that although the Rob Richard repo is indeed not active anymore, and it would be desirable to have a more active project, I don't think there's acute problems with the current library that make migration urgent.

relaxnow commented 9 years ago

Well... there is the performance issue for which we forked and an issue with encrypted assertions.

But yeah, switching out our 'XML Security library' is scary and without very good (functionality or security) reasons it's unlikely that we will switch any time soon unfortunately.

thijskh commented 9 years ago

It seems that robrichards has been revived.

jaimeperez commented 9 years ago

Indeed! The XML canonicalization performance issue is now fixed upstream, I got some other cleanings of the code in, and I also just submitted a PR providing PSR-0 compatibility :smile:

thijskh commented 9 years ago

Invite him to the next hackathon?