New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vault: Migrate to RSA-OAEP #6959
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @f-trivino
The patch is missing an upgrade part. If IPA 4.10.2 is installed on a FIPS server, and then upgraded with the patch, /etc/pki/pki-tomcat/kra/CS.cfg
does not contain keyWrap.useOAEP=true
and the vault commands executed on the server are still failing.
The second issue is the support of older clients. If I install a RHEL8 client + RHEL9 server with the patch, the RHEL8 client is unable to access the vault on the server. It fails with
# ipa vault-retrieve vault1 --out /dev/stdout
ipa: ERROR: an internal error has occurred
It means the patch would be needed on all client versions.
c9405bd
to
f89cf9d
Compare
f89cf9d
to
472e465
Compare
Hi @flo-renaud , I implemented the missing parts:
I tested the patch in a RHEL7.9-Client and RHEL9.3-Server: Server with KRA using PKCS1v15 wrap padding (it will try OAEP and will fallback into PKCS):
Client (it will try OAEP and will fallback into PKCS):
Switch KRA Server to use OAEP:
Client will try OAEP at a first attempt and will succeed:
Added ipa-* as we will need to backport this implementation to all rhel versions. I'm removing also temp_commit, this is the link of the vault gating tests passing: test install: |
472e465
to
799a00a
Compare
One of my test systems still has F37 and it's failing with OAEP trying to unwrap the response. Both the client and server are using this patch. ipa vault-archive test --data Zm9vCg== |
@rcritten thanks for testing it. I think the issue might be that your F37 test system is running with a KRA without the 'keyWrap.useOAEP=true' option enabled. This could occur if you applied the patch without performing an upgrade. The upgrade function will enable 'useOAEP' in the KRA. Another possibility could be that dogtag-pki-kra-11.2.0-2.fc37.noarch doesn't implement support for OAEP. |
I re-tested on F39 and it works. I tested a RHEL 7.9 client against it and it fails: PKIException: Cannot encrypt passphrase: org.mozilla.jss.crypto.TokenException: Failed to unwrap key: (-8190) security library: received bad data. I assume because a client-side patch is needed as well. I wonder if we should catch this on the server and raise a new exception telling the client what to do (update to provide OAEP support). I double-checked my new F-37 install and indeed it is missing the useOAEP setting. I re-installed and confirmed it is simply not being set. Manually running ipa-server-upgrade adds it. I think that a call to enable_oaep_wrap_algo should be made during the kra install and not rely on the upgrade scripts alone to set the value. |
Hi @rcritten , The patch adds the new default "pki_use_oaep_rsa_keywrap=True" to the "install/share/ipaca_default.ini" file. This covers new installations, it works for me (F38 and RHEL). Should I remove this setting and call enable_oaep_wrap_algo()?, I didn't want to introduce new restarts of the pki service. Concerning old clients, yes, we need to backport this patch to older RHEL systems. The reason is that there is no way for an old client to switch to OAEP padding if a server is running in FIPS mode, where the use of an old padding, PKCS1v15, is forbidden in the server. The good point is that OAEP is supported in RHEL7. |
F39 server has the patch, vault operations on the server:
F39 client with the patch, against F39 server with the patch:
F39 server with the patch, F39 client without the patch:
The vault-add failure doesn't roll back removing the vault. It remains and as near as I can tell, it's usable. On an add failure it might be nice to roll things back but I don't think this is a new issue so it can wait if you'd prefer. The client-side behavior isn't very nice, with an InternalError. I'd propose catching the error and raising it as an IPA error. I used NotFound here for demonstration but something more specific would be better. Perhaps EncodingError (it's really for text encoding but meh). Note that we may want to do the kra logout when raising the exception. I was just banging on this until it worked.
F39 server without the patch, F39 client with the patch:
The fallback isn't working: it doesn't fall back at all in my testing. It fails when unwrapping with the OAEP session key. If I force it to use PKCS1v15 then it succeeds (obviously b/c the code is then the same between client and server). This change works for me. It moves the wrapping try/except a level higher:
|
799a00a
to
f4c915a
Compare
eadabe2
to
32ad674
Compare
Thanks @rcritt for the review and testing. After reconsideration, I believe that enabling RSA-OAEP as the default option will cause numerous issues in existing deployments, as it would require all RHEL systems to upgrade. Therefore, the transition to the new padding should be considered as part of a major release, and we can make the switch at that point in time. However, PKCS1v15 is not usable at all in FIPS mode. Consequently, the code now detects if a system is in FIPS mode (at install/upgrade time) and picks the appropriate algo, ensuring compatibility. Regarding the fallback, you are correct; it was not working properly. I had to enhance the fallback code because PKCS1v15() triggers an exception in FIPS mode:
This explains why the fallback was failing. I believe the new code now handles this situation properly. I have tested all these scenarios: F39 server/client with the patches in FIPS mode (KRA uses RSA-OAEP):
F39 server/client with the patches in normal mode (KRA uses PKCS1v15):
The fallback is a bit tricky because of the absence of a REST call to query the KRA padding status (DRM using the REST interface). With the proper call returning such status, we would be able to handle all use cases more easily. Btw, I haven't addressed the roll back of the vault if something fails, it is not straight forward as when the issue happens (keyArchival of an empty vault from vault-add), the LDAP container for the vault is already created. I would prefer to address this issue as a separate PR. |
None of the FIPS certified modules in RHEL support PKCS#1 v1.5 as FIPS approved mechanism. This commit adds support for RSA-OAEP padding as a fallback. Fixes: https://pagure.io/freeipa/issue/9191 Signed-off-by: Francisco Trivino <ftrivino@redhat.com>
If a vault operation fails, the error message just says "InternalError". This commit improves error handling of key archival and retrieval calls by catching the PKIException error and raising it as an IPA error. Related: https://pagure.io/freeipa/issue/9191 Signed-off-by: Francisco Trivino <ftrivino@redhat.com>
Vault uses PKCS1v15 as default padding wrapping algo, which is not an approved FIPS algorithm. This commit ensures that KRA is installed with RSA-OAEP if FIPS is enabled. It also handles upgrade path. Fixes: https://pagure.io/freeipa/issue/9191 Signed-off-by: Francisco Trivino <ftrivino@redhat.com>
32ad674
to
5332c8c
Compare
This is working well in all my previous test cases. I just noticed one thing during the fallback. I set the KRA to use OAEP and I see two failures prior the prompt for the password when trying to retrieve data. I added a print before each wrapped_session_key creation and on InternalError print the exception. It functions ok I just don't know if this doulbe PKCS1v15 failure is expected. It happens
It is even more when trying to add a new vault
Maybe this is the trade-off we have to accept for now. |
This occurs when the transport certificate is not cached. The reason is that internal() attempts to call _do_internal() with the cached transport certificate on the first attempt. Since the transport certificate is not cached the first time, there is a second call where _get_vaultconfig() retrieves the transport certificate. |
Alright this is working for me. ack. |
manual backport required for ipa-4-6 branch |
PKCS#1 v1.5 padding support has been removed as it will not be allowed in FIPS mode after 2023.
None of the FIPS certified modules in RHEL will support it as a FIPS approved mechanism.
This commit migrates PKCS#1 v1.5 padding to RSA-OAEP. Mew installations of KRA will use RSA-OAEP
as default key wrapping algorithm.
Fixes: https://pagure.io/freeipa/issue/9191