Closed rtheys closed 11 months ago
rmartinc
commented
12 months ago @rtheys Yes, it's like you say, checking the code it's one or the other way.
@mhajas Maybe we should always try with the signing key if the compatibility flag is set... WDYT?
rmartinc
commented
12 months ago I have prepared a simple fix in this branch: https://github.com/rmartinc/keycloak/tree/issue-24652 The old RSA key is always added at the end if the deprecated mode is enabled. CI passes OK.
bizkaipc
commented
11 months ago Good morning, I am new to the world of keycloak, I have recently updated the company keycloak from version 17.0.1 to version 22.0.5.
I am having exactly the problem described here, I saw that there is a fix and I have downloaded it, but I don't know how I can install it, could you help me.
Thank you very much in advance.
rmartinc
commented
11 months ago There is no fix for this exact problem, it's just an idea. If you have upgraded you probably just need to put the system property -Dkeycloak.saml.deprecated.encryption=true at startup. Read the release notes to understand what is happening with the certs.
This issue is about the problem you face when you start re-assigning the certificates for encryption, and you have clients with the new enc cert and other clients with the old signing keys.
bizkaipc
commented
11 months ago Good afternoon,
I have set the parameter -Dkeycloak.saml.deprecated.encryption=true and it doesn't work either, could you tell me what tests I can do, I can also send the configuration of the broker realm and the client realm in case there is something missing to configure.
rmartinc
commented
11 months ago @bizkaipc The change done in issue https://github.com/keycloak/keycloak/issues/13606 and it's since version 21.0.0. The idea is that now the key that identity provider (SP) uses to decrypt the SAML is the one specified by the encryption alg in the document. Previously it was the current RSA key, the same one used for signing (RS256). The system prop reverts to do the same than before.
So you have to ask to the IDP what key/certificate they are using to encrypt the requests/responses to your SP and check the alg in the SAML document too. Then check where you have defined it in your realm. But if the IDP has not changed anything, it should be using the same old RS256 key, and that's why I said that the system prop was necessary.
mhajas
commented
11 months ago I have prepared a simple fix in this branch: https://github.com/rmartinc/keycloak/tree/issue-24652 The old RSA key is always added at the end if the deprecated mode is enabled. CI passes OK.
@rmartinc I like the idea. We didn't consider a case when we need to combine both the deprecated and the new approach to encryption.
@rtheys Any chance you could try the fix from @rmartinc in your environment so we can check whether the fix works?
One more thing, this flag is about to be removed in Keycloak 24 (https://github.com/keycloak/keycloak/issues/16726) which means this issue needs to be fixed only for 23.0.x.
rtheys
commented
11 months ago I have prepared a simple fix in this branch: https://github.com/rmartinc/keycloak/tree/issue-24652 The old RSA key is always added at the end if the deprecated mode is enabled. CI passes OK.
@rmartinc I like the idea. We didn't consider a case when we need to combine both the deprecated and the new approach to encryption.
@rtheys Any chance you could try the fix from @rmartinc in your environment so we can check whether the fix works?
@mhajas I've applied @rmartinc his proposed fix on 22.0.5 and I can confirm that it works:
with -Dkeycloak.saml.deprecated.encryption=true NOT set:
with -Dkeycloak.saml.deprecated.encryption=true set:
Regards, Rik
mhajas
commented
11 months ago Thank you @rtheys! @rmartinc Could you please send a PR with the changes?
Before reporting an issue
Area
identity-brokering
Describe the bug
Keycloak 21 included changes to the SAML SP metadata to require a different encryption key. To keep backwards compatibility with identity providers that were using the older metadata, the
keycloak.saml.deprecated.encryptionflag could be set.If this flag is set, and an identity provider is updated to use the new metadata (and the new key for encryption), authentication fails and the following message is logged:
If keycloak is restarted without the flag set, authentication works as expected.
To work around this we would have to make sure all identity providers are updated at the same time, after which we can remove the flag. If multiple identity providers need to be contacted, this can take quite a while and depending on the state of the flag, some of them will be broken until all of them are using the new metadata and the flag is removed.
I believe keycloak should also try to decrypt the saml assertions using the new key (for encryption) if the flag is set (and not just the key for signing).
This is also discussed in https://github.com/keycloak/keycloak/discussions/19672
Version
22.0.5
Expected behavior
Even with
-Dkeycloak.saml.deprecated.encryptionenabled, decryption works with both the new key (with usage encryption) and the key that should be used for signing.Actual behavior
Authentication fails for identity provides that are using the new key (usage encryption). No output is produced in the browser and the following message is logged in the keycloak log:
How to Reproduce?
-Dkeycloak.saml.deprecated.encryption=trueflagWhen the response from the brokered identity provider is sent to keycloak, nothing is shown in the browser and the keycloak log contains the message mentioned above.
Anything else?
No response