SAML signature is optional #4519
Comments
|
@DaanHoogland @rhtyd Guys, this looks like a pretty nasty security bug. SAML isn't used by default, but regardless this may be a blocker for 4.15 and 4.14.x. |
|
@kiwiflyer yes let's discuss and understand how the env was setup, I suspect it may related to env configuration. @ngrosc can you share details of your IDP setup? For example the IDP xml metadata, what IDP server are you using (Shibboleth, keycloak etc). Usually, the SP (CloudStack acts as a service provider in SAML terminology) is not in control of whether the SAML token is encrypted or not, and signing and encryption is enforced by the IDP server. For best practices, IDP servers generally should enforce that both signing and encryption be done and (authn) tokens be signed/encrypted (when this is done correctly we should see the encryption and signing public keys in the IDP metadata xml, for example see https://samltest.id/saml/idp). I think only if encryption is not enforced you'll be able to read/extract/change the token parameters, can you try to enforce encryption in your IDP and try your attack/test again? In case that works then we certainly have a blocker. CloudStack's SAML support (SP) was implemented and tested against Shibboleth and you may use https://samltest.id/ to test the setup. For doc ref: http://docs.cloudstack.apache.org/en/latest/adminguide/accounts.html#using-a-saml-2-0-identity-provider-for-user-authentication |
|
I guess we need exact steps of reproduction if encryption and signing are both enabled. In which case, @ngrosc can you email me privately on rohit AT apache.org? Thanks. I ran a quick test, this server is SAML enabled: http://primate-qa.cloudstack.cloud:8080/client/master
I tracked both SAML request and response. SAML request (base64 encoded) was encrypted so couldn't decode it, SAML response (base64 encoded) was not but I see the username attribute is not plaintext in the XML as encryption in enforced: |
|
Hand-waving the details, it sounds to me like we're saying that it's the IDP that controls whether the token is encrypted. |
|
+1 on what Paul and Rohit said. |
|
So, agreed that everything should be encrypted. I'm not going to claim in anyway that I'm a SAML expert, but if the username changes, shouldn't we be sending a new AuthnRequest back to the IDP? |
|
hi guys sorry for the late answer. we're using https, not http. so that should not be the the main issue. as IDP, we're using keycloack with the following config:
regarding to PaulAngus' answer: thanks for your help! |
|
@ngrosc in that case can you please close the issue. Thanks. |
|
Hi guys Meanwhile we were able to secure the connection between our IDP and Cloudstack and are now encrypting the assertions. When someone removes the signature, cloudstack accepts this. But from my point of view, it should be possible to enforce signature checking. As far as I understood, the underlying java component has a flag for this, but its not used from cloudstack. |
|
i agree it is an issue but only if a demontrable exploit exists can it be critival, @ngrosc . that said, it should be addressed. |
|
@rhtyd can you shed a light on why teh signature is ignored (and how to fix)? |
|
Needs investigation, I don't remember if it does or does not. However, if the assertions are not encrypted - I suppose that's issue of the overall IDP setup. |
|
Hi Rohit
I completely agree with you. If the assertion is encrypted or not is an IDP
"issue".
But if a signature is present or not, has to be enforced on client side
(cloudstack). Because even when the IDP adds a signature, a man in the
middle can remove it and fake data.
Of course, in case of encrypted assertions only if he has the private key
of the IDP, and then you definitely have other issues..
Nevertheless, it should be possible to enforce the check of the signature.
Then, cloudstack can ensure, that the data is valid..
Am Sa., 6. März 2021 um 11:13 Uhr schrieb Rohit Yadav <
[email protected]>:
… Needs investigation, I don't remember if it does or does not. However, if
the assertions are not encrypted - I suppose that's issue of the overall
IDP setup.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4519 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARBOIGJTIFMRSXJYLMUWPL3TCH54HANCNFSM4UNP732Q>
.
|
|
Hi, |
|
@s-seitz yes, there is no issue if the SAML IDP enforces encryption. The issue reporter is essentially asking that assertions/signature be checked even when encryption is not enforced. |
|
I stumbled upon this - and from what I can see the validation of signature is dependent upon the IDP metadata containing a signing cert. If the IDP metadata XML doesn't specify a signing cert, cloudstack simply doesn't check the signature. The sig checks are wrapped in code like |
|
@mlsorensen are you saying this is a security issue (as opposed to how it was ruled earlier)? |
|
@mlsorensen A man in the middle could easy modify the original message, remove the signature and send it to Cloudstack. Of course, as soon as the assertions are encrypted, thats not a real issue anymore. But from a security point of view, it's not uncritical when a security mechanism can be removed without any problems. I'm still thinking about a simple true/false flag named "require signed assertions". In case of not set, assertions will be processed with or without signature. But as soon as enabled, every assertion must be signed or it will be dropped. |
|
I tend to agree with @mlsorensen 's remarks, if the IDP configuration/metadata has the signing key then the signature check would be performed. I'm less worried about the MITM, as long as API activities are happening on https. Nonetheless, I've introduced a global setting that can fail a SAML based login check when enabled for more stricter env or http-based env, when the signature is missing from the SAMl response - #9219 |
rohityadavcloud
moved this from Todo
to In Review
in Apache CloudStack BugFest - Issues
|
Addressed by #9219 |
github-project-automation
bot
moved this from In Review
to Done
in Apache CloudStack BugFest - Issues
|
@ngrosc - regarding this issue, please ping me: [email protected], need some info from you. |
|
As the documentation does not state the exact verification of signatures: The linked and integrated PR expects the whole SAMLResponse to be signed, not only the assertion. As far i can see in the source code the assertion signature is not validated. That's a problem with Authentik as it only signs the assertions (which itself would be also valid, Link to authentik issue). E.g. Nextcloud offers a more fine grained control about the validation. Hope this helps someone to save time debugging SAML in that case. A workaround is to disable signature validation here for now. |
ISSUE TYPE
COMPONENT NAME
CLOUDSTACK VERSION
CONFIGURATION
SAML authentication activated
OS / ENVIRONMENT
N/A
SUMMARY
The SAML signature is optional and not mandatory. A user can login into a domain where he's not allowed to.
STEPS TO REPRODUCE
Use a tool to intercept the web requests between your browser ant the cloudstack UI (our pentester used "burp"). Login to cloudstack using a SSO account (SAML).
Now, you're logged in successfully to your cloudstack domain.
Logout from cloudstack.
Now, log in again to cloudstack, but with the "interceptor" active. In the request, delete the whole saml-signature and change the username to another SSO user from within another tenant.
Run the modified request, and you're logged in into this foreign account.
EXPECTED RESULTS
User can't log in into foreign tenant
ACTUAL RESULTS
User can log in into foreign tenant
The text was updated successfully, but these errors were encountered: