SP: dont require specific NameIDFormat

[Peter Schober]

Hey,

In the past I've successfully used the (undocumented, AFAIK) method of
setting
'NameIDPolicy' => null,
within a 'saml:SP' authsource to stop the SP from requesting a
specific NameID format in the SAML authentication request sent to the
IDP. (See below[1] for why this is important for interop for me.)

Now (at least with 1.15.1) setting NameIDPolicy to null (as above)
causes an exception:

Exception: [ARRAY]: The option 'Format' is not a valid string value.
Backtrace:
9 lib/SimpleSAML/Configuration.php:698 (SimpleSAML_Configuration::getString)
8 modules/saml/lib/Message.php:452 (sspmod_saml_Message::buildAuthnRequest)
7 modules/saml/lib/Auth/Source/SP.php:191 (sspmod_saml_Auth_Source_SP::startSSO2)

Setting this to any specific NameIDFormat URI (e.g. persistent or
emailAddress) works fine. Setting it to an empty string or array
causes the same exception (AFAIR).
So how do I stop the SP from requesting any specific format in the
authnRequest and rely on SAML Metadata alone for signalling NameID
requirements/support?
-peter

[1] Some of the IDPs I need to interop with support persistent
NameIDs, others don't. I can't set this to persistent in the SP's
authsource as not all IDPs support persistent. But if I don't set
NameIDPolicy at all then the SP will request transient specifically,
which will prevent even those IDPs from asserting persistent that
would support them. Only *not* requesting a specific format in the
authnRequest -- but still listing persistent as supported NameIDFormat
within the SP's SAML *Metadata* -- makes everything work in my
environment: (Shibboleth) IDPs that support persistent will then
release NameIDs in that format, (Shibboleth or other) IDPs that don't
will usually send transients (or other formats my SP may be able to
handle) and usually will need to supply identifiers as attributes.

[Tim van Dijen]

Hey Peter,

That's definitely a bug! NameIDPolicy is optional according to the specs, so there should be a way to leave it unset instead of always falling back to transient.
This issue was introduced in 2016:  https://github.com/simplesamlphp/simplesamlphp/commit/412fde9517d3bc961193412b13c11ce86138fdcc#diff-4ba9db222d18133821b1a07d4d4faaba

You could try setting it to 'urn:oasis:names:tc:SAML:1.0:nameid-format:unspecified' and see what that does? Although I think that's only meant for authnresponses...

- Tim

Op maandag 22 januari 2018 00:15:30 UTC+1 schreef Peter Schober:

[Jaime Perez Crespo]

Peter, do you mind applying the following patch and telling me if it then works as expected?

[Peter Schober]

* Jaime Perez Crespo <jaime...@uninett.no> [2018-01-22 10:15]:

> Peter, do you mind applying the following patch and telling me if it
> then works as expected?

Thanks Tim and Jaime for the responses.

@Jaime: Yes, that code works for my purposes (i.e., setting
NameIDPolicy to NULL establishes the earlier behaviour of not
including a NameIDPolicy element in the SAML authn request), thanks!

*But* I think it also has the -- probably unintended -- effect of
changing the default behaviour when not setting NameIDPolicy at all:
Before 'transient' was requested with NameIDPolicy unset (that's also
what the documentation states), now with your code suggestion the
effect of unset NameIDPolicy is the same as setting NameIDPolicy to
null: No NameID format will be requested.

Whether that's a good changed default behaviour or not is another
discussion (as now MS-ADFS deployers will likely complain, since AFAIK
MS-ADFS only issues NameIDs at all if specifically asked for them in
the authnRequest; and last time I checked SSP fell over dead with an
unhandled exception if no NameID at all was part of the Assertion;
even though they're optional in SAML and only useful for SLO, which
itself is 99% broken/unusable in large-scale deployments) -- but I
think it's an unintended change.

-peter

[Thijs Kinkhorst]

Op 22-01-18 om 12:41 schreef Peter Schober:

> and last time I checked SSP fell over dead with an
> unhandled exception if no NameID at all was part of the Assertion;

FYI this has been fixed in SSP 1.15.


Cheers,
Thijs

[Peter Schober]

* Thijs Kinkhorst <thijs.k...@surfnet.nl> [2018-01-22 13:12]:

Ah, thanks. A long-standing issue finally being resolved!

That certainly takes the edge out of a potentialy changed behaviour of
not requesting any specific NameID format by default.

-peter

[Thijs Kinkhorst]

Op 22-01-18 om 12:41 schreef Peter Schober:

> @Jaime: Yes, that code works for my purposes (i.e., setting
> NameIDPolicy to NULL establishes the earlier behaviour of not
> including a NameIDPolicy element in the SAML authn request), thanks!
>
> *But* I think it also has the -- probably unintended -- effect of
> changing the default behaviour when not setting NameIDPolicy at all:
> Before 'transient' was requested with NameIDPolicy unset (that's also
> what the documentation states), now with your code suggestion the
> effect of unset NameIDPolicy is the same as setting NameIDPolicy to
> null: No NameID format will be requested.
>
> Whether that's a good changed default behaviour or not is another
> discussion (as now MS-ADFS deployers will likely complain, since AFAIK
> MS-ADFS only issues NameIDs at all if specifically asked for them in
> the authnRequest; and last time I checked SSP fell over dead with an
> unhandled exception if no NameID at all was part of the Assertion;
> even though they're optional in SAML and only useful for SLO, which
> itself is 99% broken/unusable in large-scale deployments) -- but I
> think it's an unintended change.

I needed the exact same thing tonight and can indeed confirm Peter's
assessment here: you can now opt to leave out the NameIDPolicy but the
default is changed.

Tim pointed out the right commit where this was originally changed
(https://github.com/simplesamlphp/simplesamlphp/commit/412fde9517d3bc961193412b13c11ce86138fdcc#diff-4ba9db222d18133821b1a07d4d4faaba)
which neatly shows on line 406 that the old code would distinguish
between 'hasValue' (which might be null) or not hasValue (which
defaulted to transient).


Cheers,
Thijs

[Jaime Perez Crespo]

Ok, let’s give it another try. I’ve opened an issue in the issue tracker so that we can discuss this more properly, if you don’t mind:

https://github.com/simplesamlphp/simplesamlphp/issues/771

Peter, do you happen to have an account on github?

—
Jaime Pérez
UNINETT / Feide

jaime...@uninett.no
jaime...@protonmail.com
9A08 EA20 E062 70B4 616B 43E3 562A FE3A 6293 62C2

"Two roads diverged in a wood, and I, I took the one less traveled by, and that has made all the difference."
- Robert Frost