-
Notifications
You must be signed in to change notification settings - Fork 25k
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
[WIP] Add BouncyCastleFipsProvider as test dependency #41024
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,9 +152,8 @@ class BuildPlugin implements Plugin<Project> { | |
runtimeJavaVersionDetails = findJavaVersionDetails(project, runtimeJavaHome) | ||
runtimeJavaVersionEnum = JavaVersion.toVersion(findJavaSpecificationVersion(project, runtimeJavaHome)) | ||
} | ||
|
||
String inFipsJvmScript = 'print(java.security.Security.getProviders()[0].name.toLowerCase().contains("fips"));' | ||
boolean inFipsJvm = Boolean.parseBoolean(runJavaAsScript(project, runtimeJavaHome, inFipsJvmScript)) | ||
// Java home name checking is fragile, but we control the environment | ||
boolean inFipsJvm = runtimeJavaHome.contains("fips") | ||
|
||
// Build debugging info | ||
println '=======================================' | ||
|
@@ -906,6 +905,16 @@ class BuildPlugin implements Plugin<Project> { | |
File heapdumpDir = new File(project.buildDir, 'heapdump') | ||
|
||
project.tasks.withType(Test) { Test test -> | ||
RepositoryHandler repos = project.repositories | ||
jkakavas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (project.ext.inFipsJvm) { | ||
repos.ivy { | ||
url "https://downloads.bouncycastle.org" | ||
patternLayout { | ||
artifact 'fips-java/[module]-[revision].[ext]' | ||
} | ||
} | ||
project.dependencies.add('testRuntimeOnly', "org.bouncycastle:bc-fips:1.0.1:jar") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we carry this classpath tot he clusters running in the rest tests, so these will probably not be running with fips enabled. We should have a smoke test to verify that fips is really enabled everywhere we think it is when we build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks How about the integTest clusters? Those are covered by the above, correct ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes internal clusters run in the same jvm as tests so would are covered. |
||
} | ||
File testOutputDir = new File(test.reports.junitXml.getDestination(), "output") | ||
|
||
doFirst { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,8 +40,7 @@ test { | |
systemProperty 'tests.security.manager', 'false' | ||
} | ||
|
||
if (project.inFipsJvm) { | ||
// FIPS JVM includes manny classes from bouncycastle which count as jar hell for the third party audit, | ||
// rather than provide a long list of exclusions, disable the check on FIPS. | ||
thirdPartyAudit.enabled = false | ||
} | ||
if (inFipsJvm){ | ||
// Disable jarHell in FIPS as we add the BC FIPS Provider as a dependency for the tests | ||
jarHell.enabled = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd move this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we fine with disabling jarHell for all projects while running tests in a FIPS JVM ? If so, I'd be more than happy to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. We have numerous other builds that will catch any issues here. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,7 @@ dependencies { | |
compile 'org.codehaus.jackson:jackson-core-asl:1.9.2' | ||
compile 'org.codehaus.jackson:jackson-mapper-asl:1.9.2' | ||
compile 'org.codehaus.jackson:jackson-jaxrs:1.9.2' | ||
compile 'org.codehaus.jackson:jackson-xc:1.9.2' | ||
compile 'org.codehaus.jackson:jackson-xc:1.9.2' | ||
|
||
// HACK: javax.xml.bind was removed from default modules in java 9, so we pull the api in here, | ||
// and whitelist this hack in JarHell | ||
|
@@ -63,6 +63,7 @@ File keystore = new File(project.buildDir, 'keystore/test-node.jks') | |
|
||
// generate the keystore | ||
task createKey(type: LoggedExec) { | ||
onlyIf { inFipsJvm == false } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this task would fail as the JVM is setup for FIPS (BCFKS keystores, etc) but the dependency is not added when this is evaluated so the necessary classes are not available. We don't use the produced keystore eitherway as this can only work with a JKS keystore which is not available in FIPS - and this is why the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I follow. If integTest is disabled, what would try to run this task in FIPS mode? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it's because of |
||
doFirst { | ||
project.delete(keystore.parentFile) | ||
keystore.parentFile.mkdirs() | ||
|
@@ -133,3 +134,8 @@ thirdPartyAudit.ignoreMissingClasses ( | |
'com.sun.xml.fastinfoset.stax.StAXDocumentParser', | ||
'com.sun.xml.fastinfoset.stax.StAXDocumentSerializer' | ||
) | ||
|
||
if (inFipsJvm) { | ||
// We do not run integ tests in FIPS mode as these use a JKS keystore for the azure settings | ||
integTest.enabled = false | ||
} |
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.
Previously, the Provider was statically added as a system wide dependency ( in
lib/ext
of a jdk8 JVM ) so we could evaluate the getProviders() result here. Now the preference order is still set injava.security
but the dependency is not yet available so the provider won't be loaded