Skip to content

[JENKINS-73408] Ensure keys are FIPS compliant when running in FIPS mode, core requirement bump to 2.426.3#210

Merged
olamy merged 27 commits intojenkinsci:masterfrom
PereBueno:JENKINS-73408
Jul 23, 2024
Merged

[JENKINS-73408] Ensure keys are FIPS compliant when running in FIPS mode, core requirement bump to 2.426.3#210
olamy merged 27 commits intojenkinsci:masterfrom
PereBueno:JENKINS-73408

Conversation

@PereBueno
Copy link
Contributor

@PereBueno PereBueno commented Jul 8, 2024 •
edited
Loading

https://issues.jenkins.io/browse/JENKINS-73408
Added checks to make sure provided SSH keys are accepted for FIPS 140.2.
Change should be transparent if not running in FIPS mode.
When provided key is not compliant an exception will be logged and thrown. Checks are done in the constructor and in readResolve, so invalid new keys won't be added and existing ones will be removed on startup. This also covers jcasc scenario.
Also, had to bump jenkins version to 2.426.3 as this needs FIPS140.

Form validation has been added, but as https://issues.jenkins.io/browse/JENKINS-73404 is needed for it to work in secretTextArea.

This has an unwanted side effect: creating a credential with an invalid key in FIPS mode will show an angry jenkins (and obviously, credential won't be saved). Validation was added to at least inform the user saving won't be right.

Testing done

Added 3 unit tests.
Also, tested manually, behaviour is the expected.
Attaching snapshots of form validation (won't be appearing after moving back to secretTextArea). Updated just one snapshot, when there's an original exception (normally raised by bouncycastle) we're attaching the original exception to formvalidation results.
image
image
image
image

image

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@PereBueno PereBueno marked this pull request as ready for review July 9, 2024 15:00
@PereBueno PereBueno requested a review from a team as a code owner July 9, 2024 15:00
@jtnord
Copy link
Member

jtnord commented Jul 9, 2024

Form validation was not added as https://issues.jenkins.io/browse/JENKINS-73404 is needed for that

Can you add the FormValidation as if it did work (ie the upstream bugs where not present). You can test the basis of this by temproarily swapping the secretTextarea to a Textarea.

This means the validation would then "Just work" when run on a core with fixes and no extra work would be needed.

jtnord
jtnord reviewed Jul 9, 2024
View reviewed changes
Copy link
Member

@jtnord jtnord left a comment

Choose a reason for hiding this comment

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

testing and the bc-fips dependency are conflicting. the BouncyCastleProvider would not be available at all in FIPS mode.

@PereBueno
Copy link
Contributor Author

PereBueno commented Jul 10, 2024

testing and the bc-fips dependency are conflicting. the BouncyCastleProvider would not be available at all in FIPS mode.

Removed the test dependency, now using bouncycastle-api only

@PereBueno @fcojfernandez
Update src/main/java/com/cloudbees/jenkins/plugins/sshcredentials/imp…
ad8fbec 
…l/BasicSSHUserPrivateKey.java

Co-authored-by: Francisco Javier Fernandez <31063239+fcojfernandez@users.noreply.github.com>
@PereBueno PereBueno requested a review from jtnord July 11, 2024 12:20
jtnord
jtnord reviewed Jul 12, 2024
View reviewed changes
@PereBueno @jtnord
Update src/main/java/com/cloudbees/jenkins/plugins/sshcredentials/imp…
bdce55b 
…l/BasicSSHUserPrivateKey.java

Co-authored-by: James Nord <jtnord@users.noreply.github.com>
@PereBueno PereBueno requested a review from jtnord July 17, 2024 13:56
@olamy olamy changed the title [JENKINS-73408] Ensure keys are FIPS compliant when running in FIPS mode [JENKINS-73408] Ensure keys are FIPS compliant when running in FIPS mode, core requirement bump to 2.426.3 Jul 23, 2024
@olamy olamy merged commit b5a52ed into jenkinsci:master Jul 23, 2024
@basil
Copy link
Member

basil commented Jul 23, 2024 

diff --git a/pom.xml b/pom.xml
index 831b562..7d58ca5 100644
--- a/pom.xml
+++ b/pom.xml
@@ -92,8 +92,8 @@
     <dependencies>
       <dependency>
         <groupId>io.jenkins.tools.bom</groupId>
-        <artifactId>bom-2.387.x</artifactId>
-        <version>2543.vfb_1a_5fb_9496d</version>
+        <artifactId>bom-2.426.x</artifactId>
+        <version>3208.vb_21177d4b_cd9</version>
         <scope>import</scope>
         <type>pom</type>
       </dependency>
[ERROR] com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKeyFIPSTest.nonCompliantKeysLaunchExceptionTest -- Time elapsed: 2.746 s <<< ERROR!
org.jvnet.hudson.reactor.ReactorException: java.lang.Error: java.lang.reflect.InvocationTargetException
        at org.jvnet.hudson.reactor.Reactor.execute(Reactor.java:290)
        at jenkins.InitReactorRunner.run(InitReactorRunner.java:49)
        at jenkins.model.Jenkins.executeReactor(Jenkins.java:1205)
        at jenkins.model.Jenkins.<init>(Jenkins.java:992)
        at hudson.model.Hudson.<init>(Hudson.java:86)
        at org.jvnet.hudson.test.JenkinsRule.newHudson(JenkinsRule.java:746)
        at org.jvnet.hudson.test.JenkinsRule.before(JenkinsRule.java:408)
        at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:649)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.Error: java.lang.reflect.InvocationTargetException
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:115)
        at hudson.init.TaskMethodFinder$TaskImpl.run(TaskMethodFinder.java:185)
        at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:305)
        at jenkins.model.Jenkins$5.runTask(Jenkins.java:1170)
        at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:221)
        at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:120)
        at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        ... 1 more
Caused by: java.lang.reflect.InvocationTargetException
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:109)
        ... 9 more
Caused by: java.lang.IllegalStateException: The eddsa-api plugin is not FIPS compliant and can not be used in a Jenkins configured to run in FIPS-140 mode
        at io.jenkins.plugins.eddsa_api.FIPSComplianceCheck.preventUsageInFipsMode(FIPSComplianceCheck.java:15)
        ... 11 more
[ERROR] com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKeyFIPSTest.formValidationTest -- Time elapsed: 0.025 s <<< ERROR!
org.jvnet.hudson.reactor.ReactorException: java.lang.Error: java.lang.reflect.InvocationTargetException
        at org.jvnet.hudson.reactor.Reactor.execute(Reactor.java:290)
        at jenkins.InitReactorRunner.run(InitReactorRunner.java:49)
        at jenkins.model.Jenkins.executeReactor(Jenkins.java:1205)
        at jenkins.model.Jenkins.<init>(Jenkins.java:992)
        at hudson.model.Hudson.<init>(Hudson.java:86)
        at org.jvnet.hudson.test.JenkinsRule.newHudson(JenkinsRule.java:746)
        at org.jvnet.hudson.test.JenkinsRule.before(JenkinsRule.java:408)
        at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:649)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.Error: java.lang.reflect.InvocationTargetException
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:115)
        at hudson.init.TaskMethodFinder$TaskImpl.run(TaskMethodFinder.java:185)
        at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:305)
        at jenkins.model.Jenkins$5.runTask(Jenkins.java:1170)
        at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:221)
        at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:120)
        at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        ... 1 more
Caused by: java.lang.reflect.InvocationTargetException
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:109)
        ... 9 more
Caused by: java.lang.IllegalStateException: The eddsa-api plugin is not FIPS compliant and can not be used in a Jenkins configured to run in FIPS-140 mode
        at io.jenkins.plugins.eddsa_api.FIPSComplianceCheck.preventUsageInFipsMode(FIPSComplianceCheck.java:15)
        ... 11 more
[ERROR] com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKeyFIPSTest.invalidKeysAreRemovedOnStartupTest -- Time elapsed: 0.039 s <<< ERROR!
org.jvnet.hudson.reactor.ReactorException: java.lang.Error: java.lang.reflect.InvocationTargetException
        at org.jvnet.hudson.reactor.Reactor.execute(Reactor.java:290)
        at jenkins.InitReactorRunner.run(InitReactorRunner.java:49)
        at jenkins.model.Jenkins.executeReactor(Jenkins.java:1205)
        at jenkins.model.Jenkins.<init>(Jenkins.java:992)
        at hudson.model.Hudson.<init>(Hudson.java:86)
        at org.jvnet.hudson.test.JenkinsRule.newHudson(JenkinsRule.java:746)
        at org.jvnet.hudson.test.JenkinsRule.before(JenkinsRule.java:408)
        at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:649)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.Error: java.lang.reflect.InvocationTargetException
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:115)
        at hudson.init.TaskMethodFinder$TaskImpl.run(TaskMethodFinder.java:185)
        at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:305)
        at jenkins.model.Jenkins$5.runTask(Jenkins.java:1170)
        at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:221)
        at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:120)
        at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        ... 1 more
Caused by: java.lang.reflect.InvocationTargetException
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:109)
        ... 9 more
Caused by: java.lang.IllegalStateException: The eddsa-api plugin is not FIPS compliant and can not be used in a Jenkins configured to run in FIPS-140 mode
        at io.jenkins.plugins.eddsa_api.FIPSComplianceCheck.preventUsageInFipsMode(FIPSComplianceCheck.java:15)
        ... 11 more
[ERROR] com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKeyFIPSTest.invalidKeyIsNotSavedInFIPSModeTest -- Time elapsed: 0.022 s <<< ERROR!
org.jvnet.hudson.reactor.ReactorException: java.lang.Error: java.lang.reflect.InvocationTargetException
        at org.jvnet.hudson.reactor.Reactor.execute(Reactor.java:290)
        at jenkins.InitReactorRunner.run(InitReactorRunner.java:49)
        at jenkins.model.Jenkins.executeReactor(Jenkins.java:1205)
        at jenkins.model.Jenkins.<init>(Jenkins.java:992)
        at hudson.model.Hudson.<init>(Hudson.java:86)
        at org.jvnet.hudson.test.JenkinsRule.newHudson(JenkinsRule.java:746)
        at org.jvnet.hudson.test.JenkinsRule.before(JenkinsRule.java:408)
        at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:649)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.Error: java.lang.reflect.InvocationTargetException
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:115)
        at hudson.init.TaskMethodFinder$TaskImpl.run(TaskMethodFinder.java:185)
        at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:305)
        at jenkins.model.Jenkins$5.runTask(Jenkins.java:1170)
        at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:221)
        at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:120)
        at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        ... 1 more
Caused by: java.lang.reflect.InvocationTargetException
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:109)
        ... 9 more
Caused by: java.lang.IllegalStateException: The eddsa-api plugin is not FIPS compliant and can not be used in a Jenkins configured to run in FIPS-140 mode
        at io.jenkins.plugins.eddsa_api.FIPSComplianceCheck.preventUsageInFipsMode(FIPSComplianceCheck.java:15)
        ... 11 more

@basil basil mentioned this pull request Jul 23, 2024
Merged
basil
basil reviewed Jul 23, 2024
View reviewed changes
pom.xml
<properties>
<changelist>999999-SNAPSHOT</changelist>
<jenkins.version>2.387.3</jenkins.version>
<jenkins.version>2.426.3</jenkins.version>
Copy link
Member

@basil basil Jul 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Core bumps should also update the BOM artifactId: #210 (comment)

Copy link
Member

@olamy olamy Jul 24, 2024

Choose a reason for hiding this comment

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

ah right. we might need something to check this automatically rather than relying on human check. maybe an enforcer rule could do the job here

Copy link
Member

@timja timja Jul 24, 2024

Choose a reason for hiding this comment

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

easiest way that is done in a few plugins is to use variables, e.g.:
https://github.com/jenkinsci/junit-plugin/blob/85201a38881e5811c8632b610d8d655f161db103/pom.xml#L18

https://github.com/jenkinsci/junit-plugin/blob/85201a38881e5811c8632b610d8d655f161db103/pom.xml#L194

Copy link
Contributor Author

@PereBueno PereBueno Jul 24, 2024

Choose a reason for hiding this comment

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

#211

@basil basil mentioned this pull request Jul 23, 2024
Closed
timja
timja reviewed Jul 24, 2024
View reviewed changes
...s/com/cloudbees/jenkins/plugins/sshcredentials/impl/BasicSSHUserPrivateKey/credentials.jelly
<f:password/>
<f:password id="${passId}"/>
</f:entry>
<script><![CDATA[
Copy link
Member

@timja timja Jul 24, 2024

Choose a reason for hiding this comment

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

new script blocks shouldn't be introduced: https://www.jenkins.io/doc/developer/security/csp/

Copy link
Contributor Author

@PereBueno PereBueno Jul 24, 2024

Choose a reason for hiding this comment

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

#211

@PereBueno PereBueno mentioned this pull request Jul 24, 2024
Merged
6 tasks
@timja timja mentioned this pull request Jul 24, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@olamy olamy olamy approved these changes

@basil basil basil left review comments

@timja timja timja left review comments

@fcojfernandez fcojfernandez fcojfernandez approved these changes

@jtnord jtnord Awaiting requested review from jtnord

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@PereBueno @jtnord @basil @olamy @timja @fcojfernandez