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

Add FIPS specific testclusters configuration #41199

Merged
merged 1 commit into from Apr 19, 2019

Conversation

Projects
None yet
3 participants
@atorok
Copy link
Contributor

commented Apr 15, 2019

ClusterFormationTasks auto configured these properties for clusters.
This PR adds FIPS specific configuration across all test clusters from
the main build script to prevent coupling betwwen testclusters and the
build plugin.

Closes #40904

@elasticmachine

This comment has been minimized.

Copy link

commented Apr 15, 2019

Add FIPS specific testclusters configuration
ClusterFormationTasks auto configured these properties for clusters.
This PR adds FIPS specific configuration across all test clusters from
the main build script to prevent coupling betwwen testclusters and the
build plugin.

Closes #40904
@atorok

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

@jkakavas yes, other projects still use ClusterFormationTasks. That will go away when the whole class goes.

@jkakavas

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@jkakavas yes, other projects still use ClusterFormationTasks. That will go away when the whole class goes.

So, any further fips specific system properties should be added both in the build.gradle of the elasticsearch project and in the ClusterFormationTasks until that's removed, correct ?

@jkakavas
Copy link
Contributor

left a comment

LGTM

@jkakavas jkakavas self-requested a review Apr 15, 2019

@jkakavas

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

On second thought: I'm adding a few things related to FIPS config in #41024 and my initial approach is to add that config in BuildPlugin too , i.e.

                // Set the system keystore/truststore password if we're running tests in a FIPS-140 JVM
                if (project.ext.inFipsJvm) {
                    ExportElasticsearchBuildResourcesTask buildResources = project.tasks.getByName('buildResources')
                    dependsOn buildResources
                    project.dependencies.add('testCompile', "org.bouncycastle:bc-fips:1.0.1:jar")
                    systemProperty 'javax.net.ssl.keyStore', buildResources.copy("cacerts.bcfks")
                    systemProperty 'javax.net.ssl.keyStoreType', 'BCFKS'
                    systemProperty 'javax.net.ssl.keyStorePassword', 'password'
                    systemProperty 'javax.net.ssl.trustStore', buildResources.copy("cacerts.bcfks")
                    systemProperty 'javax.net.ssl.trustStoreType', 'BCFKS'
                    systemProperty 'javax.net.ssl.trustStorePassword', 'password'
                    String policyFile = "java_${project.ext.runtimeJavaVersion.getMajorVersion()}_fips.policy"
                    String securityPropertiesFile = "java_${project.ext.runtimeJavaVersion.getMajorVersion()}_fips.security"
                    systemProperty 'java.security.policy', buildResources.copy(policyFile)
                    systemProperty 'java.security.properties', buildResources.copy(securityPropertiesFile)
                }

AFAIU, we can't move the ones depending on buildResources to the main build script, so would it be worth it having fips related config in multiple places ?

@atorok

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

I think it's very much worth having everything in a single place, but it doesn't have to be the build plugin. Some projects ( tests ) will need fips without having that plugin applied. The build resources is still universally available since other tasks create it as well.
You could add configuration to the task with tasks.withType(ExportElasticsearchBuildResourcesTask) {} so it's only applicable when the task is created.

We could also have everything fips specific live in a new plugin.

@atorok atorok merged commit 92483d4 into elastic:master Apr 19, 2019

8 checks passed

CLA All commits in pull request signed
Details
elasticsearch-ci/1 Build finished.
Details
elasticsearch-ci/2 Build finished.
Details
elasticsearch-ci/bwc Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

@atorok atorok deleted the atorok:fix-reindex-fips branch Apr 19, 2019

atorok added a commit that referenced this pull request Apr 19, 2019

Add FIPS specific testclusters configuration (#41199)
ClusterFormationTasks auto configured these properties for clusters.
This PR adds FIPS specific configuration across all test clusters from
the main build script to prevent coupling betwwen testclusters and the
build plugin.

Closes #40904
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.