-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Fix forbidden apis on FIPS #33202
Fix forbidden apis on FIPS #33202
Conversation
- third party audit detects jar hell with JDK so we disable it - jdk non portable in forbiddenapis detects classes being used from the JDK ( for fips ) that are not portable, this is intended so we don't scan for it on fips. - different exclusion rules for third party audit on fips Closes elastic#33179
Pinging @elastic/es-core-infra |
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.
Added a couple of suggestions for comments as what we're doing here might not be obvious, but other than that LGTM
@@ -2141,3 +2141,8 @@ if (project.runtimeJavaVersion > JavaVersion.VERSION_1_8) { | |||
'javax.xml.bind.Unmarshaller' | |||
] | |||
} | |||
|
|||
if (project.inFipsJvm) { | |||
// jar hell with JDK on FIPS |
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.
Maybe expand a little on jar hell with JDK on FIPS
to make what happens more obvious ?
@@ -163,3 +162,9 @@ thirdPartyAudit.excludes = [ | |||
'org.conscrypt.Conscrypt', | |||
'org.conscrypt.HandshakeListener' | |||
] | |||
|
|||
if (project.inFipsJvm == false) { | |||
thirdPartyAudit.excludes += [ |
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.
Maybe add a comment , i.e.:
// BouncyCastleFIPS provides this class, so the exclusion is invalid when running CI in
// a FIPS JVM with BouncyCastleFIPS Provider
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.
I agree with @jkakavas about adding comments for why we disable things with FIPS
@@ -39,3 +39,7 @@ test { | |||
// TODO: find a way to add permissions for the tests in this module | |||
systemProperty 'tests.security.manager', 'false' | |||
} | |||
|
|||
if (project.inFipsJvm) { | |||
thirdPartyAudit.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.
I think this definitely warrants a comment as to why we disable third party audit
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.
I don't understand why we are completely disabling this as opposed to excluding the forbidden classes when using a FIPS JVM?
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.
@jasontedor we could exclude but it's a really long list, and those classes are AFAIK the only difference between fips and java 8 from a third party audit perspective, if we except all I don't think there's any reason to keep it enabled.
- third party audit detects jar hell with JDK so we disable it - jdk non portable in forbiddenapis detects classes being used from the JDK ( for fips ) that are not portable, this is intended so we don't scan for it on fips. - different exclusion rules for third party audit on fips Closes #33179
* master: Painless: Add Bindings (#33042) Update version after client credentials backport Fix forbidden apis on FIPS (#33202) Remote 6.x transport BWC Layer for `_shrink` (#33236) Test fix - Graph HLRC tests needed another field adding to randomisation exception list HLRC: Add ML Get Records API (#33085) [ML] Fix character set finder bug with unencodable charsets (#33234) TESTS: Fix overly long lines (#33240) Test fix - Graph HLRC test was missing field name to be excluded from randomisation logic Remove unsupported group_shard_failures parameter (#33208) Update BucketUtils#suggestShardSideQueueSize signature (#33210) Parse PEM Key files leniantly (#33173) INGEST: Add Pipeline Processor (#32473) Core: Add java time xcontent serializers (#33120) Consider multi release jars when running third party audit (#33206) Update MSI documentation (#31950) HLRC: create base timed request class (#33216) [DOCS] Fixes command page titles HLRC: Move ML protocol classes into client ml package (#33203) Scroll queries asking for rescore are considered invalid (#32918) Painless: Fix Semicolon Regression (#33212) ingest: minor - update test to include dissect (#33211) Switch remaining LLREST usage to new style Requests (#33171) HLREST: add reindex API (#32679)
* 6.x: Fix forbidden apis on FIPS (#33202) HLRC: Add ML Get Records API (#33085) [ML] Fix character set finder bug with unencodable charsets (#33234) Tests fix - Graph HLRC client overly long line and syncing core’s copy of GraphExploreResponseTests taken from protocol. Related to #33231 Test fix - Graph HLRC test was missing field name to be excluded from randomisation logic Parse PEM Key files leniantly (#33173) Core: Add java time xcontent serializers (#33120) Consider multi release jars when running third party audit (#33206) Update MSI documentation (#31950) [DOCS] Fixes command page titles HLRC: Move ML protocol classes into client ml package (#33203) Painless: Fix Semicolon Regression (#33212)
JDK ( for fips ) that are not portable, this is intended so we don't
scan for it on fips.
Closes #33179