Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[JENKINS-73506] Enforce use of TLS in FIPS mode #1611

Merged
merged 18 commits into from
Jul 30, 2024
Merged

[JENKINS-73506] Enforce use of TLS in FIPS mode #1611

merged 18 commits into from
Jul 30, 2024

Conversation

olamy
Copy link
Member

@olamy olamy commented Jul 26, 2024 •
edited by MarkEWaite
Loading

Signed-off-by: Olivier Lamy olamy@apache.org

JENKINS-73506 - Enforce use of TLS in FIPS mode

Enforce use of transport layer security (TLS) when running in a mode compatible with Federal Information Processing Standard 140-2 (FIPS 140-2)

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • Documentation in README has been updated as necessary
  • Online help has been added and reviewed for any new or modified fields
  • I have interactively tested my changes
  • Any dependent changes have been merged and published in upstream modules (like git-client-plugin)

Types of changes

What types of changes does your code introduce?

  • New feature (non-breaking change which adds functionality)

@github-actions github-actions bot added tests Automated test addition or improvement dependencies Dependency related change labels Jul 26, 2024
@olamy olamy changed the title [JENKINS-73506] start validation with GitSCMSource [JENKINS-73506] Enforce use of TLS in FIPS mode Jul 28, 2024
@olamy olamy force-pushed the JENKINS-73506-check-tls branch from 3ecaeee to bc82b51 Compare July 28, 2024 11:18
olamy added a commit that referenced this pull request Jul 29, 2024
@olamy
useContainerAgent to false to be able to use testcontainer for PR #1611
304d0d3
MarkEWaite pushed a commit that referenced this pull request Jul 29, 2024
@olamy
useContainerAgent to false to be able to use testcontainer for PR #1611…
fbb5d91 
… (#1613)
olamy added 2 commits July 29, 2024 11:54
@olamy
[JENKINS-73506] Enforce use of TLS in FIPS mode
275345d 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
fix test
f1ec735
@olamy olamy force-pushed the JENKINS-73506-check-tls branch from 8ed93a7 to f1ec735 Compare July 29, 2024 01:54
olamy added 2 commits July 29, 2024 14:49
@olamy
fix ssh keys to be FIPS compliant
d894341 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
add test for checkout step
c158a34 
Signed-off-by: Olivier Lamy <olamy@apache.org>
olamy added 3 commits July 29, 2024 15:28
@olamy
extract to one single method
35f5a2b 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
test the extract method
2503236 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
format magnify :)
9e3a855 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy olamy marked this pull request as ready for review July 29, 2024 05:52
@olamy olamy requested a review from a team as a code owner July 29, 2024 05:52
@olamy
windauze...
a297109 
Signed-off-by: Olivier Lamy <olamy@apache.org>
fcojfernandez
fcojfernandez approved these changes Jul 29, 2024
View reviewed changes
Copy link
Member

@fcojfernandez fcojfernandez left a comment

Choose a reason for hiding this comment

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

Minor comment

MarkEWaite
MarkEWaite reviewed Jul 29, 2024
View reviewed changes
src/main/java/hudson/plugins/git/UserRemoteConfig.java
@@ -58,6 +61,9 @@ public UserRemoteConfig(String url, String name, String refspec, @CheckForNull S
this.name = fixEmpty(name);
this.refspec = fixEmpty(refspec);
this.credentialsId = fixEmpty(credentialsId);
if (FIPS140.useCompliantAlgorithms() && StringUtils.isNotEmpty(this.credentialsId) && StringUtils.startsWith(this.url, "http:")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that the use of the unauthenticated git:// protocol is outside the scope of FIPS compliance, since it does not require any credential and allows read access to anyone that can access that network port on the host.

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch. using git:// protocol with a credentials should be restricted in FIPS env.
done with rev 08a38cf

olamy added 2 commits July 30, 2024 08:47
@olamy
improve namimg
f9668af 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
control not used of git protocol with credentials in FIPS mode
08a38cf 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@MarkEWaite MarkEWaite added rfe Improvement or new feature and removed dependencies Dependency related change tests Automated test addition or improvement labels Jul 29, 2024
@MarkEWaite
Use commons.lang.StringUtils instead of commons.lang3.StringUtils
e7603db 
Remain consistent with other usages in plugin.  These would be the first
imports fo commons.lang3 in the plugin and a method of the same name is
already provided by commons.lang.
@github-actions github-actions bot added dependencies Dependency related change tests Automated test addition or improvement labels Jul 30, 2024
MarkEWaite
MarkEWaite approved these changes Jul 30, 2024
View reviewed changes
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Thanks very, very much!

I think this is ready to merge so long as you're OK with the method name isFIPSLtsCompliant. I don't understand the use of Lts in that method name, but I'm not aware of the details of FIPS, so it may just be my failure to understand.

@olamy
Copy link
Member Author

olamy commented Jul 30, 2024

Thanks very, very much!

I think this is ready to merge so long as you're OK with the method name isFIPSLtsCompliant. I don't understand the use of Lts in that method name, but I'm not aware of the details of FIPS, so it may just be my failure to understand.

the only reason I can see is a typo ;) I will fix that

@olamy
fix typo
f647031 
Signed-off-by: Olivier Lamy <olamy@apache.org>
@MarkEWaite MarkEWaite removed dependencies Dependency related change tests Automated test addition or improvement labels Jul 30, 2024
@MarkEWaite MarkEWaite enabled auto-merge (squash) July 30, 2024 01:25
@MarkEWaite
Copy link
Contributor

Auto-merge enabled. Thanks!

@MarkEWaite MarkEWaite merged commit 43a56cb into master Jul 30, 2024
17 checks passed
@olamy olamy deleted the JENKINS-73506-check-tls branch July 30, 2024 01:51
@PereBueno PereBueno mentioned this pull request Jul 31, 2024
Merged
12 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@MarkEWaite MarkEWaite MarkEWaite approved these changes

@fcojfernandez fcojfernandez Awaiting requested review from fcojfernandez

Assignees
No one assigned
Labels
rfe Improvement or new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@olamy @MarkEWaite @fcojfernandez