Skip to content

feat: keycloak fips mode#1469

Merged
slaskawi merged 5 commits intomainfrom
36-keycloak_fips
Apr 25, 2025
Merged

feat: keycloak fips mode#1469
slaskawi merged 5 commits intomainfrom
36-keycloak_fips

Conversation

@slaskawi
Copy link
Copy Markdown
Contributor

@slaskawi slaskawi commented Apr 16, 2025

Description

This Pull Request introduces Keycloak FIPS mode into the UDS Core.

Related Issue

Relates to defenseunicorns/uds-identity-config#36

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Steps to Validate

N/A

Checklist before merging

Comment thread src/keycloak/chart/templates/statefulset.yaml
Comment on lines -181 to -193
# FIPS Mode
{{- if .Values.fips }}
# https://access.redhat.com/documentation/en-us/openjdk/11/html-single/configuring_openjdk_11_on_rhel_with_fips/index
- name: JAVA_TOOL_OPTIONS
value: "-Dcom.redhat.fips=true"
{{- end }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending CI testing I think this is fine, just would want to make sure that dropping this/changing the effect of fips value doesn't have any backwards incompatible effects on FIPS or non-fips hosts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a while but I think I have the answer!

The Red Hat Build of OpenJDK contains OpenJDK bits with an additional FIPS patch. You can download the source from the Red Hat Customer Portal (from here).

The code haven't changed much since JDK11, it looks very similar to this:

+        if (systemSecPropsEnabled) {
+            boolean shouldEnable;
+            String sysProp = System.getProperty("com.redhat.fips");
+            if (sysProp == null) {
+                shouldEnable = true;
+                if (sdebug != null) {
+                    sdebug.println("com.redhat.fips unset, using default value of true");
+                }
+            } else {
+                shouldEnable = Boolean.valueOf(sysProp);
+                if (sdebug != null) {
+                    sdebug.println("com.redhat.fips set, using its value " + shouldEnable);
+                }
+            }
+            if (shouldEnable) {
+                boolean fipsEnabled = SystemConfigurator.configureFIPS(props);
+                if (sdebug != null) {
+                    if (fipsEnabled) {
+                        sdebug.println("FIPS mode support configured and enabled.");
+                    } else {
+                        sdebug.println("FIPS mode support disabled.");
+                    }

Note, that in the absence of com.redhat.fips, the OpenJDK (at least the Red Hat's build of it) tries to enable the FIPS mode by default. The SystemConfigurator.configureFIPS is a JNI (Java Native Interface - calling C from Java).

The other important thing from our angle is the SystemConfigurator.enableFips method that has the following JavaDoc:

+     * OpenJDK FIPS mode will be enabled only if the system is in
+     * FIPS mode.
+     *
+     * Calls to this method only occur if the system property
+     * com.redhat.fips is not set to false.
+     *
+     * There are 2 possible ways in which OpenJDK detects that the system
+     * is in FIPS mode: 1) if the NSS SECMOD_GetSystemFIPSEnabled API is
+     * available at OpenJDK's built-time, it is called; 2) otherwise, the
+     * /proc/sys/crypto/fips_enabled file is read.

The NSS is shipped as part of SSSD, which is part of the RHEL authentication stack and is present in the Keycloak image. According to BZ852023, NSS brokers to /proc/sys/crypto/fips_enabled, which is usually what happens in container world (if the host OS uses FIPS mode it sets /proc/sys/crypto/fips_enabled to all containers it runs based on BZ1957310).

To sum it up - I believe it's safe to remove this property and let the Red Hat Build of OpenJDK do the default thing - try to enable FIPS. This will succeed if the underlying Host has FIPS turned on regardless to the distro (although UBI are expected to be more tested than anything else).

@slaskawi slaskawi marked this pull request as ready for review April 24, 2025 06:38
@slaskawi slaskawi requested a review from a team as a code owner April 24, 2025 06:38
@slaskawi slaskawi enabled auto-merge (squash) April 24, 2025 07:54
Copy link
Copy Markdown
Contributor

@chance-coleman chance-coleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a general questions regarding fips, is there any kind of tests that we should implement to "show/prove" that fips is enabled and working? I don't know what this would look like..but using tests, where we can, to show proof of compliance seems like a good idea to me.

@slaskawi
Copy link
Copy Markdown
Contributor Author

slaskawi commented Apr 25, 2025

just a general questions regarding fips, is there any kind of tests that we should implement to "show/prove" that fips is enabled and working? I don't know what this would look like..but using tests, where we can, to show proof of compliance seems like a good idea to me.

@UnicornChance Once this (and it's UDS Identity Config counterpart) gets merged, I plan to modify pull-request-conditionals.yaml and introduce "keycloak_fips" parameter to the testing matrix. This way we'll be testing all flavors with and without Keycloak FIPS (cc @mjnagel)

As for the test itself, I have three ideas. One is to scan the logs and ensure we emit the KC(BCFIPS version 2.0 Approved Mode string. This one is emitted when Keycloak finishes FIPS configuration. The other one is to check supported certs and cryptographic keys. FIPS installations should use an approved subset of these. The final idea is to use the Keycloak Admin API and get Credentials for the Admin user. The default "argon2" shall not be used there (as it's not approved).

@slaskawi slaskawi merged commit 74e632e into main Apr 25, 2025
16 checks passed
@slaskawi slaskawi deleted the 36-keycloak_fips branch April 25, 2025 14:02
@chance-coleman
Copy link
Copy Markdown
Contributor

@mjnagel and I were chatting about the testing approach. Adding fips/non-fips to the matrix in uds-core will mean at least 6 new jobs that need to run in our already super long testing pipeline. Micah suggested, and i agree, we should implement a fips/non-fips test in identity-config rather than core for this as it shouldn't impact other uds-core integrations. Especially if we make FIPS default.

noahpb pushed a commit that referenced this pull request Apr 29, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.41.0](v0.40.1...v0.41.0)
(2025-04-28)


### Features

* add conditional netpol for coredns
([#1501](#1501))
([fc7ace3](fc7ace3))
* client credential registration default
([#1482](#1482))
([894c5d9](894c5d9))
* keycloak fips mode
([#1469](#1469))
([74e632e](74e632e))
* operator ambient mode
([#1496](#1496))
([71f03fd](71f03fd))
* opt Grafana into ambient
([#1466](#1466))
([dac2d3e](dac2d3e))
* opt logging into ambient
([#1472](#1472))
([117d586](117d586))
* opt metrics-server into ambient
([#1458](#1458))
([01c2ec6](01c2ec6))
* opt velero into ambient
([#1490](#1490))
([a0591c7](a0591c7))


### Bug Fixes

* **ci:** permissions on release workflow
([#1507](#1507))
([cb12f13](cb12f13))
* **ci:** renovate readiness version loop fix
([#1488](#1488))
([a40c15b](a40c15b))
* update loki images to fips images
([#1502](#1502))
([eb20b4e](eb20b4e))


### Miscellaneous

* **ci:** automated renovate readiness action checks
([#1465](#1465))
([ed0ca6b](ed0ca6b))
* **ci:** switch eks CI to FIPS ami, update to 1.31 k8s testing
([#1474](#1474))
([7307d03](7307d03))
* **deps:** update grafana
([#1489](#1489))
([0c063f1](0c063f1))
* **deps:** update istio to v1.25.2
([#1461](#1461))
([1067560](1067560))
* **deps:** update istio to v1.3.0
([#1491](#1491))
([9066584](9066584))
* **deps:** update keycloak to v0.13.0
([#1506](#1506))
([04d42ef](04d42ef))
* **deps:** update keycloak to v26.2.0
([#1452](#1452))
([927a57b](927a57b))
* **deps:** update keycloak to v26.2.1
([#1486](#1486))
([d68cad8](d68cad8))
* **deps:** update loki
([#1483](#1483))
([3a697df](3a697df))
* **deps:** update neuvector
([#1417](#1417))
([4c0d95d](4c0d95d))
* **deps:** update pepr
([#1454](#1454))
([a98640f](a98640f))
* **deps:** update support dependencies to v4.7.0
([#1477](#1477))
([dcee0a3](dcee0a3))
* **deps:** update support-deps
([#1473](#1473))
([3d9d501](3d9d501))
* **deps:** update support-deps
([#1480](#1480))
([c41f359](c41f359))
* **deps:** update support-deps
([#1481](#1481))
([cc2af2b](cc2af2b))
* **deps:** update support-deps
([#1487](#1487))
([cdcba75](cdcba75))
* **deps:** update support-deps
([#1493](#1493))
([88cbf29](88cbf29))
* **deps:** update support-deps
([#1497](#1497))
([f308176](f308176))
* **deps:** update velero
([#1453](#1453))
([7330ea9](7330ea9))
* **deps:** update velero
([#1492](#1492))
([ff504c0](ff504c0))
* **deps:** update velero to v1.32.4
([#1484](#1484))
([06709e8](06709e8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
mjnagel pushed a commit to BagelLab/uds-core that referenced this pull request Nov 14, 2025
## Description

This Pull Request introduces Keycloak FIPS mode into the UDS Core. 

## Related Issue

Relates to
defenseunicorns/uds-identity-config#36

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Steps to Validate

N/A

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed
mjnagel pushed a commit to BagelLab/uds-core that referenced this pull request Nov 14, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.41.0](defenseunicorns/uds-core@v0.40.1...v0.41.0)
(2025-04-28)


### Features

* add conditional netpol for coredns
([defenseunicorns#1501](defenseunicorns#1501))
([fc7ace3](defenseunicorns@fc7ace3))
* client credential registration default
([defenseunicorns#1482](defenseunicorns#1482))
([894c5d9](defenseunicorns@894c5d9))
* keycloak fips mode
([defenseunicorns#1469](defenseunicorns#1469))
([74e632e](defenseunicorns@74e632e))
* operator ambient mode
([defenseunicorns#1496](defenseunicorns#1496))
([71f03fd](defenseunicorns@71f03fd))
* opt Grafana into ambient
([defenseunicorns#1466](defenseunicorns#1466))
([dac2d3e](defenseunicorns@dac2d3e))
* opt logging into ambient
([defenseunicorns#1472](defenseunicorns#1472))
([117d586](defenseunicorns@117d586))
* opt metrics-server into ambient
([defenseunicorns#1458](defenseunicorns#1458))
([01c2ec6](defenseunicorns@01c2ec6))
* opt velero into ambient
([defenseunicorns#1490](defenseunicorns#1490))
([a0591c7](defenseunicorns@a0591c7))


### Bug Fixes

* **ci:** permissions on release workflow
([defenseunicorns#1507](defenseunicorns#1507))
([cb12f13](defenseunicorns@cb12f13))
* **ci:** renovate readiness version loop fix
([defenseunicorns#1488](defenseunicorns#1488))
([a40c15b](defenseunicorns@a40c15b))
* update loki images to fips images
([defenseunicorns#1502](defenseunicorns#1502))
([eb20b4e](defenseunicorns@eb20b4e))


### Miscellaneous

* **ci:** automated renovate readiness action checks
([defenseunicorns#1465](defenseunicorns#1465))
([ed0ca6b](defenseunicorns@ed0ca6b))
* **ci:** switch eks CI to FIPS ami, update to 1.31 k8s testing
([defenseunicorns#1474](defenseunicorns#1474))
([7307d03](defenseunicorns@7307d03))
* **deps:** update grafana
([defenseunicorns#1489](defenseunicorns#1489))
([0c063f1](defenseunicorns@0c063f1))
* **deps:** update istio to v1.25.2
([defenseunicorns#1461](defenseunicorns#1461))
([1067560](defenseunicorns@1067560))
* **deps:** update istio to v1.3.0
([defenseunicorns#1491](defenseunicorns#1491))
([9066584](defenseunicorns@9066584))
* **deps:** update keycloak to v0.13.0
([defenseunicorns#1506](defenseunicorns#1506))
([04d42ef](defenseunicorns@04d42ef))
* **deps:** update keycloak to v26.2.0
([defenseunicorns#1452](defenseunicorns#1452))
([927a57b](defenseunicorns@927a57b))
* **deps:** update keycloak to v26.2.1
([defenseunicorns#1486](defenseunicorns#1486))
([d68cad8](defenseunicorns@d68cad8))
* **deps:** update loki
([defenseunicorns#1483](defenseunicorns#1483))
([3a697df](defenseunicorns@3a697df))
* **deps:** update neuvector
([defenseunicorns#1417](defenseunicorns#1417))
([4c0d95d](defenseunicorns@4c0d95d))
* **deps:** update pepr
([defenseunicorns#1454](defenseunicorns#1454))
([a98640f](defenseunicorns@a98640f))
* **deps:** update support dependencies to v4.7.0
([defenseunicorns#1477](defenseunicorns#1477))
([dcee0a3](defenseunicorns@dcee0a3))
* **deps:** update support-deps
([defenseunicorns#1473](defenseunicorns#1473))
([3d9d501](defenseunicorns@3d9d501))
* **deps:** update support-deps
([defenseunicorns#1480](defenseunicorns#1480))
([c41f359](defenseunicorns@c41f359))
* **deps:** update support-deps
([defenseunicorns#1481](defenseunicorns#1481))
([cc2af2b](defenseunicorns@cc2af2b))
* **deps:** update support-deps
([defenseunicorns#1487](defenseunicorns#1487))
([cdcba75](defenseunicorns@cdcba75))
* **deps:** update support-deps
([defenseunicorns#1493](defenseunicorns#1493))
([88cbf29](defenseunicorns@88cbf29))
* **deps:** update support-deps
([defenseunicorns#1497](defenseunicorns#1497))
([f308176](defenseunicorns@f308176))
* **deps:** update velero
([defenseunicorns#1453](defenseunicorns#1453))
([7330ea9](defenseunicorns@7330ea9))
* **deps:** update velero
([defenseunicorns#1492](defenseunicorns#1492))
([ff504c0](defenseunicorns@ff504c0))
* **deps:** update velero to v1.32.4
([defenseunicorns#1484](defenseunicorns#1484))
([06709e8](defenseunicorns@06709e8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants