build(deps): bump opensaml2 from 2.6.6 to 4.0.1 #2908

Tallicia · 2024-05-30T21:51:52Z

Replacing the other feature branch #2862 for new SAML library replacement effort.

Sonar
https://sonarcloud.io/summary/new_code?id=cloudfoundry-identity-parent&pullRequest=2908

cf-gitbot · 2024-05-30T21:51:56Z

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/187710947

The labels on this github issue will be updated when the story is started.

...ava/org/cloudfoundry/identity/uaa/provider/saml/SamlLegacyAliasResponseForwardingFilter.java

needed later for SLO

in case of decryption issue (wrong key) do not show class internals

Found in manual test with SAML SLO , POST Binding

…-session-index

…0-saml-session-index

…-index Store saml session index in UaaSamlPrincipal

re-establish IT see former retrieval https://github.com/cloudfoundry/uaa/blob/develop/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProvider.java#L292-L298

* Cleanup libraries not needed anymore bound to old opensaml * Remove ESAPI finally this dependency is only there because of old saml * fix rebase

# Conflicts: # dependencies.gradle

https://sonarcloud.io/project/issues?impactSoftwareQualities=RELIABILITY&sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=2908&id=cloudfoundry-identity-parent

Will add more coverage in Saml2Utils

Signed-off-by: Duane May <[email protected]>

strehle · 2024-11-14T06:34:10Z

.../cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java

+        }
+
+        if (!identityProviderDefinitions.isEmpty()) {
+            SamlIdentityProviderDefinition identityProviderDefinition = identityProviderDefinitions.get(0);


This is an unexpected behavior, because we use then first found trust material to verify the incoming SAML message.

strehle · 2024-11-14T06:36:22Z

.../cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java

+
+        for (SamlIdentityProviderDefinition identityProviderDefinition : identityProviderDefinitions) {
+            if (identityProviderDefinition.getIdpEntityAlias().equals(registrationId)) {
+                return createRelyingPartyRegistration(registrationId, identityProviderDefinition, currentZone);


This is a search by origin only. In case of SAML bearer we dont have the origin key in the token (SAML assertion) but only the issuer.
A loop is something I would like to omit because we now have an alternative , see #2825

strehle · 2024-11-14T06:38:03Z

server/src/main/resources/dummy-saml-idp-metadata.xml

+            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+                <ds:X509Data>
+                    <ds:X509Certificate>
+                        MIIEEzCCAvugAwIBAgIJAIc1qzLrv+5nMA0GCSqGSIb3DQEBCwUAMIGfMQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ08xFDASBgNVBAcMC0Nhc3RsZSBSb2NrMRwwGgYDVQQKDBNTYW1sIFRlc3RpbmcgU2VydmVyMQswCQYDVQQLDAJJVDEgMB4GA1UEAwwXc2ltcGxlc2FtbHBocC5jZmFwcHMuaW8xIDAeBgkqhkiG9w0BCQEWEWZoYW5pa0BwaXZvdGFsLmlvMB4XDTE1MDIyMzIyNDUwM1oXDTI1MDIyMjIyNDUwM1owgZ8xCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDTzEUMBIGA1UEBwwLQ2FzdGxlIFJvY2sxHDAaBgNVBAoME1NhbWwgVGVzdGluZyBTZXJ2ZXIxCzAJBgNVBAsMAklUMSAwHgYDVQQDDBdzaW1wbGVzYW1scGhwLmNmYXBwcy5pbzEgMB4GCSqGSIb3DQEJARYRZmhhbmlrQHBpdm90YWwuaW8wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC4cn62E1xLqpN34PmbrKBbkOXFjzWgJ9b+pXuaRft6A339uuIQeoeH5qeSKRVTl32L0gdz2ZivLwZXW+cqvftVW1tvEHvzJFyxeTW3fCUeCQsebLnA2qRa07RkxTo6Nf244mWWRDodcoHEfDUSbxfTZ6IExSojSIU2RnD6WllYWFdD1GFpBJOmQB8rAc8wJIBdHFdQnX8Ttl7hZ6rtgqEYMzYVMuJ2F2r1HSU1zSAvwpdYP6rRGFRJEfdA9mm3WKfNLSc5cljz0X/TXy0vVlAV95l9qcfFzPmrkNIst9FZSwpvB49LyAVke04FQPPwLgVH4gphiJH3jvZ7I+J5lS8VAgMBAAGjUDBOMB0GA1UdDgQWBBTTyP6Cc5HlBJ5+ucVCwGc5ogKNGzAfBgNVHSMEGDAWgBTTyP6Cc5HlBJ5+ucVCwGc5ogKNGzAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAvMS4EQeP/ipV4jOG5lO6/tYCb/iJeAduOnRhkJk0DbX329lDLZhTTL/x/w/9muCVcvLrzEp6PN+VWfw5E5FWtZN0yhGtP9R+vZnrV+oc2zGD+no1/ySFOe3EiJCO5dehxKjYEmBRv5sU/LZFKZpozKN/BMEa6CqLuxbzb7ykxVr7EVFXwltPxzE9TmL9OACNNyF5eJHWMRMllarUvkcXlh4pux4ks9e6zV9DQBy2zds9f1I3qxg0eX6JnGrXi/ZiCT+lJgVe3ZFXiejiLAiKB04sXW3ti0LW3lx13Y1YlQ4/tlpgTgfIJxKV6nyPiLoK0nywbMd+vpAirDt2Oc+hk


Can we omit the key (default key) in the production ?

I found identitfied this xml as part of test package... if the template is needed isnt a constant string (without key) a better approach ?

strehle · 2024-11-14T07:06:25Z

...ava/org/cloudfoundry/identity/uaa/provider/saml/Saml2BearerGrantAuthenticationConverter.java

+
+    @Override
+    public Authentication convert(HttpServletRequest request) throws AuthenticationException {
+        RelyingPartyRegistration relyingPartyRegistration = relyingPartyRegistrationResolver.resolve(request, null);


This returns the dummy IdP registration.

strehle · 2024-11-14T07:07:12Z

...ava/org/cloudfoundry/identity/uaa/provider/saml/Saml2BearerGrantAuthenticationConverter.java

+        boolean addNew;
+        IdentityProvider<SamlIdentityProviderDefinition> idp;
+        SamlIdentityProviderDefinition samlConfig;
+        try {


a 2nd lookup in DB. right now dont have a better idea, but I would do it earlier, before signature validation

strehle · 2024-11-14T07:09:31Z

...ava/org/cloudfoundry/identity/uaa/provider/saml/Saml2BearerGrantAuthenticationConverter.java

+
+        Assertion assertion = parseAssertion(assertionXml);
+        Saml2AuthenticationToken authenticationToken = new Saml2AuthenticationToken(relyingPartyRegistration, assertionXml);
+        process(authenticationToken, assertion);


here we validate signature, but we are not confident that we have the correct IdP

Did not found a security problem, but the error messages are misleading because then you see always signature error message

strehle · 2024-11-14T07:14:08Z

server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/Saml2Utils.java

+        return Base64.getEncoder().encodeToString(b);
+    }
+
+    public static byte[] samlDecode(String s) {


missing methods for SAML bearer which needs UrlDecoder for Base64.

strehle · 2024-11-14T15:29:55Z

SAML2 bearer and IdP initated flows are similar and the IdP initiated flow still is not working.
The tests work, but this is by accident because we have only one trusted IdP and the TODO in resolution of IdP.

The resolver needs to be forked / implemented on Uaa side, I assume then we can resolve from issuer to idP .
With more than 1 trusted IdP the flow is working by accident...

SAML2 bearer works now independent from amount of SAML2 trusts, #3132

Tallicia added in progress DO NOT MERGE Internal Test or WIP, please DO NOT MERGE labels May 30, 2024

cf-gitbot added unscheduled and removed in progress labels May 30, 2024

Tallicia changed the title ~~New saml 0530~~ New SAML 2024.05.30 - Not to merge but just for SAML feature branch testing May 30, 2024

Tallicia mentioned this pull request May 30, 2024

New saml 20240430 - Not to merge but just for SAML feature branch testing #2862

Closed

github-advanced-security bot found potential problems May 30, 2024

View reviewed changes

...ava/org/cloudfoundry/identity/uaa/provider/saml/SamlLegacyAliasResponseForwardingFilter.java Fixed Show fixed Hide fixed

duanemay force-pushed the new-saml-0530 branch 5 times, most recently from 65b0d64 to e67a40a Compare June 14, 2024 22:11

duanemay force-pushed the new-saml-0530 branch 9 times, most recently from a761b67 to 7de27a1 Compare June 24, 2024 15:54

duanemay force-pushed the new-saml-0530 branch from 14ffbb7 to 17cac18 Compare June 25, 2024 16:17

duanemay force-pushed the new-saml-0530 branch 2 times, most recently from 0d3a595 to f199f50 Compare July 5, 2024 22:17

peterhaochen47 force-pushed the new-saml-0530 branch from 46248b9 to 290f89c Compare July 9, 2024 01:32

duanemay force-pushed the new-saml-0530 branch 2 times, most recently from 46248b9 to b6cb65b Compare July 9, 2024 18:49

peterhaochen47 force-pushed the new-saml-0530 branch 2 times, most recently from a97457f to 745fff3 Compare July 10, 2024 17:04

strehle and others added 10 commits November 11, 2024 17:39

Store saml session index in UaaSamlPrincipal

6fa5a1d

needed later for SLO

return plain error message (#3119)

530e885

in case of decryption issue (wrong key) do not show class internals

Disable csrf check in SAML-SLO (#3123)

5faaeb4

Found in manual test with SAML SLO , POST Binding

Merge remote-tracking branch 'origin/develop' into new-saml-0530-saml…

f872cb7

…-session-index

Merge remote-tracking branch 'origin/new-saml-0530' into new-saml-053…

da1b1f2

…0-saml-session-index

fix integration test

8f99520

fix integration test

2ca7bae

Merge pull request #3122 from cloudfoundry/new-saml-0530-saml-session…

acf805d

…-index Store saml session index in UaaSamlPrincipal

Add acr value into User Authentication (#3127)

115fff6

re-establish IT see former retrieval https://github.com/cloudfoundry/uaa/blob/develop/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProvider.java#L292-L298

Merge remote-tracking branch 'origin/develop' into new-saml-0530

3c012b2

strehle added scheduled in_review The PR is currently in review and removed unscheduled labels Nov 12, 2024

Cleanup shadow library (#3130)

403d642

strehle changed the title ~~New SAML 2024.05.30 - Not to merge but just for SAML feature branch testing~~ build(deps): bump opensaml2 from 2.6.6 to 4.0.1 Nov 12, 2024

strehle marked this pull request as ready for review November 12, 2024 17:44

Cleanup libraries not needed anymore (#3129)

78e21ba

* Cleanup libraries not needed anymore bound to old opensaml * Remove ESAPI finally this dependency is only there because of old saml * fix rebase

strehle requested review from a team, strehle and duanemay November 12, 2024 17:50

strehle and others added 8 commits November 13, 2024 12:56

Merge remote-tracking branch 'origin/develop' into new-saml-0530

4a6cdc6

# Conflicts: # dependencies.gradle

sonar issue

c127bbb

https://sonarcloud.io/project/issues?impactSoftwareQualities=RELIABILITY&sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=2908&id=cloudfoundry-identity-parent

remove not needed method

f36bd79

Add test to run Authn with redirect binding

ca9e360

Will add more coverage in Saml2Utils

minor sonar issue

6fb8135

cleanup not used code

71b4429

sonar issue with unspecified type

9ebac66

Fix Sonar issues

d02c5dd

Signed-off-by: Duane May <[email protected]>

strehle reviewed Nov 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build(deps): bump opensaml2 from 2.6.6 to 4.0.1 #2908

build(deps): bump opensaml2 from 2.6.6 to 4.0.1 #2908

Tallicia commented May 30, 2024 •

edited by strehle

Loading

cf-gitbot commented May 30, 2024

strehle Nov 14, 2024

strehle Nov 14, 2024

strehle Nov 14, 2024

strehle Nov 14, 2024

strehle Nov 14, 2024

strehle Nov 14, 2024

strehle Nov 14, 2024

strehle commented Nov 14, 2024

build(deps): bump opensaml2 from 2.6.6 to 4.0.1 #2908

Are you sure you want to change the base?

build(deps): bump opensaml2 from 2.6.6 to 4.0.1 #2908

Conversation

Tallicia commented May 30, 2024 • edited by strehle Loading

cf-gitbot commented May 30, 2024

strehle Nov 14, 2024

Choose a reason for hiding this comment

strehle Nov 14, 2024

Choose a reason for hiding this comment

strehle Nov 14, 2024

Choose a reason for hiding this comment

strehle Nov 14, 2024

Choose a reason for hiding this comment

strehle Nov 14, 2024

Choose a reason for hiding this comment

strehle Nov 14, 2024

Choose a reason for hiding this comment

strehle Nov 14, 2024

Choose a reason for hiding this comment

strehle commented Nov 14, 2024

Tallicia commented May 30, 2024 •

edited by strehle

Loading