[ELY-2496] Add integrity to existing filesystem realms using Elytron Tool #1896

jessicarod7 · 2023-04-19T16:10:24Z

https://issues.redhat.com/browse/ELY-2496

Commits:

~~1. The first two commits come from ELY-2514, since the integrity command uses JDK 11 features. First JDK 11 commit removes Java 8 specific classes~~
~~2. Second JDK 11 commit replaces with Java 9 specific classes and updates POMs.~~ (dropped from this PR)

Moves reused parameters into a common class Params within Command
Add hash-charset parameter and test cases to filesystem-realm-encrypt, which was missed in the original PR [ELY-2078] Add encryption support to FileSystemSecurityRealm #1676
EDIT: Updated warning messages in Command.getSecretKey() to be more consistent, and always warn (vs. sometimes crashing from an uncaught exception)
Adds new filesystem-realm-integrity command, with support for upgraded encrypted realms
Support upgrading realms with integrity enabled in filesystem-realm-encrypt. Also disabled upgrade of empty realms (see below)
Add test cases in both commands for filesystem realms with integrity
Modified to remove methods which require JDK 11 (see [ELY-2514] Update minimum JDK for all modules to 11 #1880)

Summary of changes:

Addition of a new command filesystem-realm-integrity to upgrade existing realms to support integrity verification. Requires an non-empty filesystem realm, and a password-protected keystore with a key pair.
- Supports upgrading encrypted realms
- Supports in-place realm upgrade (upgraded realm is placed in the same directory)
- Test cases for all params, with validation of CLI scripts via validateScript method and ScriptParams class
- Encryption command issues not present in integrity command: ELY-2530, ELY-2536, ELY-2540, ELY-2541, ELY-2542, and ELY-2546
Update filesystem-realm-encrypt command to support upgrading filesystem realms which already have integrity enabled (Note that encryption command cannot enable integrity, and vice versa)
- Added missing option hash-charset to encryption command
Changes for both commands
- Move static Elytron password/keystore providers, overrideable warning/error handlers, and methods getSecretKey and getKeyPair to Command class
- Add new method FileSystemRealmUtil.createRealmWithIntegrityValidation() to build new realm; moved shared code between encryption and integrity commands into a private method
- Empty filesystem realms cannot upgraded by Elytron Tool (the directory is not copied), so instead provides a warning and instructs user to update within management client (see [WFCORE-6129] Unable to add integrity support to existing filesystem realm wildfly/wildfly-core#5411)
Other
- Move common parameters and strings across Elytron Tool commands (ex. HELP_PARAM, System.getProperty(“line.separator”)) to new class org.wildfly.security.tool.Command$Params

Also, I've manually imported and tested all of the test case realms with the wildfly/wildfly@784cc50 commit. All identities are readable, and the :verify-integrity CLI command works.

tool/src/main/java/org/wildfly/security/tool/Command.java

fjuma · 2023-04-25T21:53:14Z

tool/src/main/java/org/wildfly/security/tool/Command.java

+        SecretKey key;
+        try {
+            key = credentialStore.retrieve(alias, SecretKeyCredential.class).getSecretKey();
+        } catch (NullPointerException e) {


Instead of catching the NullPointerException here, it would be better to check the return value of credentialStore.retrieve prior to calling getSecretKey. If it's null, we can print the error and return null.

I'll update this and the other section to be similar to getKeyPair. This code was unchanged from the original encrypt command, so I'm not as familiar with the functionality.

@OndrejKotek Noting that I made some changes in response to this, in the new 5th commit

tool/src/main/java/org/wildfly/security/tool/Command.java

auth/realm/base/src/main/java/org/wildfly/security/auth/realm/FileSystemRealmUtil.java

tool/src/main/java/org/wildfly/security/tool/Command.java

fjuma · 2023-05-10T19:27:56Z

auth/realm/base/src/main/java/org/wildfly/security/auth/realm/FileSystemRealmUtil.java

-    public static void createEncryptedRealmFromUnencrypted(FileSystemSecurityRealm unencryptedRealm, FileSystemSecurityRealm encryptedRealm) throws RealmUnavailableException {
-        Assert.checkNotNullParam("unencryptedRealm", unencryptedRealm);
-        Assert.checkNotNullParam("encryptedRealm", encryptedRealm);
+    public static void cloneIdentitiesToNewRealm(FileSystemSecurityRealm oldRealm, FileSystemSecurityRealm newRealm) throws RealmUnavailableException {


FYI for other reviewers, this is private API.

tool/src/main/java/org/wildfly/security/tool/Command.java

tool/src/main/java/org/wildfly/security/tool/CredentialStoreCommand.java

tool/src/main/java/org/wildfly/security/tool/ElytronTool.java

jessicarod7 · 2023-05-19T15:55:04Z

Looks like a random failure due to a timeout on the Windows build. Could someone please rerun the CI?

fjuma · 2023-05-19T16:02:29Z

Thanks, re-triggered it.

Skyllarr

@cam-rod LGTM, thanks!

Skyllarr · 2023-05-22T16:22:27Z

tool/src/main/java/org/wildfly/security/tool/Command.java

+     * @param warning The warning to be shown
+     */
+    protected void warningHandler(String warning) {
+        System.out.print("WARNING: ");


@cam-rod just a minor, we can set status to GENERAL_CONFIGURATION_WARNING here

turns out this change actually broke a number of tests in FileSystemRealmCommandTest, so I'm reverting it

Skyllarr · 2023-05-22T17:53:18Z

tool/src/main/java/org/wildfly/security/tool/FileSystemRealmIntegrityCommand.java

+            if (passwordOption == null && passwordEnvOption == null) {
+                passwordOption = prompt(false, ElytronToolMessages.msg.keyStorePasswordPrompt(), false, null);
+                if (passwordOption == null) {
+                    setStatus(GENERAL_CONFIGURATION_ERROR);


@cam-rod just a total minor, this line is superfluous since we configured the status to be GENERAL_CONFIGURATION_ERROR at the beginning

fixed in this command and encryption (where I made the same change)

fjuma

Thanks @cam-rod!

fjuma · 2023-05-23T19:18:54Z

@cam-rod Just noticed, we have some CI failures now.

…ommand

…Tool

Due to ELY-2514, this module must be compiled for Java 8. This commit can be reversed or reverted once this issue is fixed.

jessicarod7 · 2023-05-23T20:26:07Z

Thanks for the heads up, added some patches for the rebase.

fjuma · 2023-05-23T20:34:36Z

Thanks @cam-rod!

jessicarod7 requested review from fjuma and Skyllarr as code owners April 19, 2023 16:10

jessicarod7 mentioned this pull request Apr 19, 2023

[WFCORE-6129] Unable to add integrity support to existing filesystem realm wildfly/wildfly-core#5411

Merged

jessicarod7 force-pushed the ELY-2496 branch from 95b1004 to 846a4cd Compare April 19, 2023 16:56

fjuma reviewed Apr 19, 2023

View reviewed changes

tool/src/main/java/org/wildfly/security/tool/Command.java Outdated Show resolved Hide resolved

fjuma reviewed Apr 19, 2023

View reviewed changes

tool/src/main/java/org/wildfly/security/tool/Command.java Outdated Show resolved Hide resolved

jessicarod7 force-pushed the ELY-2496 branch from 846a4cd to 9d12f35 Compare April 20, 2023 18:48

fjuma reviewed Apr 25, 2023

View reviewed changes

tool/src/main/java/org/wildfly/security/tool/Command.java Outdated Show resolved Hide resolved

fjuma reviewed Apr 25, 2023

View reviewed changes

auth/realm/base/src/main/java/org/wildfly/security/auth/realm/FileSystemRealmUtil.java Outdated Show resolved Hide resolved

jessicarod7 force-pushed the ELY-2496 branch 2 times, most recently from bffc0c0 to 782550d Compare April 26, 2023 20:31

fjuma reviewed May 10, 2023

View reviewed changes

jessicarod7 force-pushed the ELY-2496 branch 2 times, most recently from 8ed421b to 42fcbee Compare May 19, 2023 13:47

Skyllarr approved these changes May 22, 2023

View reviewed changes

Skyllarr added the +1 DV label May 23, 2023

jessicarod7 force-pushed the ELY-2496 branch 2 times, most recently from c760eea to 108d334 Compare May 23, 2023 19:10

fjuma approved these changes May 23, 2023

View reviewed changes

fjuma added the +1 FJ label May 23, 2023

jessicarod7 added 6 commits May 23, 2023 15:36

[ELY-2496] Move common methods and variables to Command class

051f5ee

[ELY-2496] Add missing hash-charset option to FileSystemEncryptRealmC…

0bd8c07

…ommand

[ELY-2496] Update warnings for SecretKey retrieval

babbb20

[ELY-2496] Add integrity to existing filesystem realms using Elytron …

161a2fe

…Tool

[ELY-2496] FileSystemEncryptRealmCommand supports realms with integrity

d655c6a

[ELY-2496] Add integrity command test cases

814505b

[ELY-2496] Modify commands to support JDK 8

4bfd3e4

Due to ELY-2514, this module must be compiled for Java 8. This commit can be reversed or reverted once this issue is fixed.

jessicarod7 force-pushed the ELY-2496 branch from 108d334 to 4bfd3e4 Compare May 23, 2023 20:17

fjuma merged commit f4794df into wildfly-security:2.x May 23, 2023

jessicarod7 deleted the ELY-2496 branch May 24, 2023 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ELY-2496] Add integrity to existing filesystem realms using Elytron Tool #1896

[ELY-2496] Add integrity to existing filesystem realms using Elytron Tool #1896

jessicarod7 commented Apr 19, 2023 •

edited

Loading

fjuma Apr 25, 2023 •

edited

Loading

jessicarod7 Apr 26, 2023

jessicarod7 Apr 26, 2023

fjuma May 10, 2023

jessicarod7 commented May 19, 2023

fjuma commented May 19, 2023

Skyllarr left a comment

Skyllarr May 22, 2023

jessicarod7 May 23, 2023 •

edited

Loading

jessicarod7 May 23, 2023

Skyllarr May 22, 2023

jessicarod7 May 23, 2023

fjuma left a comment

fjuma commented May 23, 2023

jessicarod7 commented May 23, 2023

fjuma commented May 23, 2023

[ELY-2496] Add integrity to existing filesystem realms using Elytron Tool #1896

[ELY-2496] Add integrity to existing filesystem realms using Elytron Tool #1896

Conversation

jessicarod7 commented Apr 19, 2023 • edited Loading

fjuma Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

jessicarod7 Apr 26, 2023

Choose a reason for hiding this comment

jessicarod7 Apr 26, 2023

Choose a reason for hiding this comment

fjuma May 10, 2023

Choose a reason for hiding this comment

jessicarod7 commented May 19, 2023

fjuma commented May 19, 2023

Skyllarr left a comment

Choose a reason for hiding this comment

Skyllarr May 22, 2023

Choose a reason for hiding this comment

jessicarod7 May 23, 2023 • edited Loading

Choose a reason for hiding this comment

jessicarod7 May 23, 2023

Choose a reason for hiding this comment

Skyllarr May 22, 2023

Choose a reason for hiding this comment

jessicarod7 May 23, 2023

Choose a reason for hiding this comment

fjuma left a comment

Choose a reason for hiding this comment

fjuma commented May 23, 2023

jessicarod7 commented May 23, 2023

fjuma commented May 23, 2023

jessicarod7 commented Apr 19, 2023 •

edited

Loading

fjuma Apr 25, 2023 •

edited

Loading

jessicarod7 May 23, 2023 •

edited

Loading