parsivori / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

Logout quick fix #561

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
This is just quick fix for logout problem introduced in r3257.

You throw away destination definition from sspmod_saml_Message, and in 
sspmod_saml_IdP_SAML2::getLogoutURL() is destination not set, and generated url 
contains only parameters ?SAMLRequest=... without other parts (protocol, 
hostname) i.e. url is invalid, and SP is not logged out and "Unable to 
initialize logout to ..." is logged.

I say this is just a quick fix, because the whole logout process should be 
probably rewritten to support HTTP-POST binding, and it's currently working 
only for HTTP-Redirect despite the introduced SingleLogoutServiceBinding 
directive, which as I see is currently only used for metadata generation, but 
not for actually enabling support for specified bindings (e.g. 
SimpleSAML_Configuration::getDefaultEndpoint() is not taking 
SingleLogoutServiceBinding into account).

BTW Is Olav still working on SSP?

Original issue reported on code.google.com by comel...@gmail.com on 7 Aug 2013 at 4:48

Attachments:

GoogleCodeExporter commented 8 years ago
Hi,

That's definitely a bug, thanks for reporting. I've committed your patch in 
revision r3259.

Apart from that, what exactly is not working for you? I tested both 
SingleSignOn and SingleLogout using HTTP-POST binding prior to committing that 
revision, and both worked perfectly.

The Single*ServiceBinding is used, as you said, "only" for metadata generation. 
But given that metadata is the key for other parties to configure the 
connection with a SSP instance, that's exactly the way to tell others which 
binding to use.

If you want to enable HTTP-POST for a certain remote party, you need to do that 
on its local metadata (i.e. saml20-idp-remote.php). If you want to switch to 
HTTP-POST, you just have to specify that in 
Single(Logout|SignOn)ServiceBinding. If you want to support both, specify both. 
If you want to set priority for one over the other, do it as SAML specs say, 
place the one which you want to be used by default (with more priority) the 
first. Then make sure your new metadata is distributed to your parties. Let's 
say you want to support both, but give priority to HTTP-POST. Then, you 
configure Single*ServiceBinding like this:

    'SingleSignOnServiceBinding' => array(SAML2_Const::BINDING_HTTP_POST, SAML2_Const::BINDING_HTTP_REDIRECT),
    'SingleLogoutServiceBinding' => array(SAML2_Const::BINDING_HTTP_POST, SAML2_Const::BINDING_HTTP_REDIRECT),

The metadata generated then will look like this:

    <md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://idp.famine.vm/saml2/idp/SingleLogoutService.php"/>
    <md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://idp.famine.vm/saml2/idp/SingleLogoutService.php"/>
     [...]
    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://idp.famine.vm/saml2/idp/SSOService.php"/>
    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://idp.famine.vm/saml2/idp/SSOService.php"/>

If the other end is using SSP, that translates into:

  'SingleSignOnService' => 
  array (
    0 => 
    array (
      'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
      'Location' => 'https://idp.famine.vm/saml2/idp/SSOService.php',
    ),
    1 => 
    array (
      'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
      'Location' => 'https://idp.famine.vm/saml2/idp/SSOService.php',
    ),
  ),
  'SingleLogoutService' => 
  array (
    0 => 
    array (
      'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
      'Location' => 'https://idp.famine.vm/saml2/idp/SingleLogoutService.php',
    ),
    1 => 
    array (
      'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
      'Location' => 'https://idp.famine.vm/saml2/idp/SingleLogoutService.php',
    ),
  ),

and, as expected, the other party will default to HTTP-POST then, as you 
requested.

SimpleSAML_Configuration::getDefaultEndpoint() is, indeed, not taking 
Single*ServiceBinding directives into account. That's because it doesn't have 
to. What getDefaultEndpoint() needs to do is to determine the default endpoint 
from a set of endpoints, according to the rules specified by 
saml-metadata-2.0-os:2.2.3 (Complex Type IndexedEndpointType). SimpleSAMLphp 
now supports both HTTP-POST and HTTP-Redirect, and that's why both are 
specified as supported bindings when calling getDefaultEndpoint() all over the 
code. Still, HTTP-Redirect is prioritized by default in order to keep backwards 
compatibility.

Regarding Olav, yes, he's still working on SSP, though he's been stepping back 
a little bit lately. This update to support HTTP-POST was something we both 
were discussing about since it's a big and difficult update (as it has proven, 
given the bug you found).

Original comment by jaim...@gmail.com on 9 Aug 2013 at 10:21

GoogleCodeExporter commented 8 years ago
HTTP-POST binding for LogoutRequest is currently supported only for IdP part, 
i.e. SP -> IdP, right? Because I was looking at the 
SimpleSAML_IdP_LogoutTraditional::logoutNextSP() because there was breaking 
before the patch, and I see that there always only HTTP redirect is used, i.e. 
<Handler>::getLogoutURL() and SimpleSAML_Utilities::redirect(). I thought that 
HTTP-POST binding for LogoutRequest IdP -> SP was also supported, and was 
referring to that part.

Original comment by comel...@gmail.com on 9 Aug 2013 at 1:44

GoogleCodeExporter commented 8 years ago
HTTP-POST binding works also for IdP -> SP. But only when the SP using 
HTTP-POST is the one initiating the logout. That leaves out the case for other 
SPs not initiating SLO. In that case, it doesn't work even with your patch,  
since HTTPPost binding does not implement getRedirectURL() (for obvious 
reasons).

I'm working on it. Hope to have a fix along the day.

Original comment by jaim...@gmail.com on 12 Aug 2013 at 9:25

GoogleCodeExporter commented 8 years ago
My patch is only quick fix for HTTP-Redirect LogoutRequest, and nothing more, 
and the other parts of the logout process should be rewritten to work properly 
(see initial comment).

Original comment by comel...@gmail.com on 12 Aug 2013 at 5:13

GoogleCodeExporter commented 8 years ago
Hi,

I've just committed r3262 to fix both the bug with HTTP-Redirect, and the 
missing support for HTTP-Post for SPs not initiating SLO. It works for both 
traditional and iFrame SLO, JavaScript and non-JavaScript versions of the 
latter. The only restriction is that an IdP using iFrame SLO should not 
prioritise HTTP-Post binding if it wants to support users with JavaScript 
disabled. Otherwise, the SPs will logout correctly, but the users will see an 
error message telling something went wrong when logging out from them, because 
the LogoutResponse (SP -> IdP) will never reach the IdP.

Original comment by jaim...@gmail.com on 13 Aug 2013 at 10:39