Skip to content
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

[WFLY-16179] Add integrity support to FilesystemSecurityRealm #15411

Merged
merged 1 commit into from Aug 3, 2022

Conversation

Ashpan
Copy link
Contributor

@Ashpan Ashpan commented Apr 6, 2022

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Apr 6, 2022
@luck3y
Copy link
Contributor

luck3y commented Apr 7, 2022

/retest

@bstansberry bstansberry added core-upgrade-needed PR requires a wildfly-core change to be merged and integrated first Feature-Docs PR documents a new feature coming via WildFly Core labels Apr 13, 2022
@@ -6,7 +6,7 @@

<container qualifier="jboss" default="true">
<configuration>
<property name="javaVmArguments">${server.jvm.args}</property>
<property name="javaVmArguments">${server.jvm.args} -agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005</property>
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be included here.

@fjuma
Copy link
Contributor

fjuma commented Jun 3, 2022

@Ashpan Just noticed that the encryption related commits are still appearing here. Looks like that PR (#15269) hasn't been merged yet because CI wasn't passing. I've re-triggered the failing job.

@fjuma
Copy link
Contributor

fjuma commented Jun 9, 2022

@Ashpan This branch now has conflicts that need to be resolved. Note that the encryption changes have been merged so they're no longer needed in this PR.

@@ -148,6 +148,38 @@ An encrypted `filesystem-realm` that makes use of the `credential-store` could b
/subsystem=elytron/filesystem-realm=exampleFsRealm:add(path=fs-realm-users,relative-to=jboss.server.config.dir, credential-store=mycredstore, secret-key=key)
----

==== Integrity

It is also possible to enable integrity support on the filesystem realm by configuring the key-store and key-store-alias attributes for the realm.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/enable integrity support/enable support for integrity checking


It is also possible to enable integrity support on the filesystem realm by configuring the key-store and key-store-alias attributes for the realm.

When enabling integrity on a filesystem realm, you will have to specify the following 2 attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

s/integrity/integrity checking


When enabling integrity on a filesystem realm, you will have to specify the following 2 attributes

* `key-store`: This attribute specifies the key-store where the key-pair (Private Key and Public Key) is used for signing each identity file for the filesystem realm.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/where the key-pair (Private Key and Public Key) is used for signing each identity file for the filesystem realm/that contains the key pair (private key and public key) that's used for signing each identity file and for integrity checking


* `key-store`: This attribute specifies the key-store where the key-pair (Private Key and Public Key) is used for signing each identity file for the filesystem realm.

* `key-store-alias`: This attribute specifies the alias of the key-store that contains the correct key-pair to be using for signing the files.
Copy link
Contributor

Choose a reason for hiding this comment

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

the alias within the key-store that identifies the PrivateKeyEntry to use to sign identity files and perform filesystem integrity checks

/subsystem=elytron/key-store=keystore:store()
----

To create an integrity enabled filesystem realm with a key-store called ``keystore`` and the secret-key alias ``localhost``, the following command would be used
Copy link
Contributor

Choose a reason for hiding this comment

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

key-store-alias

@@ -25,9 +27,11 @@ The charset is UTF-8 and the string encoding is BASE64.

The `credential-store` being referenced is called `mycredstore` and the alias of the `secret-key` in that `credential-store` is `key`. The secret key will be used to encrypt and decrypt the filesystem realm.

The key-store being referenced is called `keystore` and the alias of the key pair in that key-store is `localhost`. The `localhost` key pair will be used to sign identity files and verify the pre-existing signatures to ensure that the filesystem realm has not been tampered with.
Copy link
Contributor

Choose a reason for hiding this comment

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

key-store

@@ -47,12 +51,20 @@ To delete existing identity from filesystem realm:
/subsystem=elytron/filesystem-realm=fsRealm:remove-identity(identity=alex)
----

An operation is added if an integrity-enabled filesystem realm needs to update the key-store or the key-store alias. The following command will update the realm and all the identity signatures to utilize the new key-pair.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this could be reworded to something like:

It's possible to update the key-store or key-store-alias to use for integrity checking using the update-key-pair command as shown below. This command will also update the signature in any existing identity files.

@Ashpan Ashpan force-pushed the WFLY-16179 branch 3 times, most recently from 35fd1f8 to 2eac750 Compare June 15, 2022 19:33
Copy link
Contributor

@fjuma fjuma left a comment

Choose a reason for hiding this comment

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

Thanks @Ashpan!

@fjuma fjuma added the Critical Doesn't block a release but deserves higher priority than most. Use sparingly. label Aug 2, 2022
@bstansberry bstansberry removed the core-upgrade-needed PR requires a wildfly-core change to be merged and integrated first label Aug 2, 2022
@bstansberry
Copy link
Contributor

/retest

@darranl darranl added the ready-for-merge Only for use by those with merge permissions! label Aug 3, 2022
@darranl darranl merged commit 7f1b4ee into wildfly:main Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical Doesn't block a release but deserves higher priority than most. Use sparingly. deps-ok Dependencies have been checked, and there are no significant changes Feature-Docs PR documents a new feature coming via WildFly Core ready-for-merge Only for use by those with merge permissions!
Projects
None yet
6 participants