Skip to content

Commit

Permalink
Warn when using insecure auth (#275)
Browse files Browse the repository at this point in the history
Additionally if running in FIPS use an error in the form validation and
do not support auth without SSL or TLS.

This is handled when we attempt to send as the config is part of the
global configuration and could leave the config in an non determinate
state
  • Loading branch information
jtnord authored Mar 12, 2024
1 parent f3f62b2 commit f7c289a
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 21 deletions.
18 changes: 3 additions & 15 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</licenses>
<properties>
<changelist>999999-SNAPSHOT</changelist>
<jenkins.version>2.387.3</jenkins.version>
<jenkins.version>2.440.1</jenkins.version>
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
<useBeta>true</useBeta>
</properties>
Expand All @@ -32,23 +32,11 @@
<dependencies>
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.387.x</artifactId>
<version>2543.vfb_1a_5fb_9496d</version>
<artifactId>bom-2.440.x</artifactId>
<version>2884.vc36b_64ce114a_</version>
<scope>import</scope>
<type>pom</type>
</dependency>
<!-- TODO Remove once in BOM -->
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>jakarta-activation-api</artifactId>
<version>2.1.3-1</version>
</dependency>
<!-- TODO Remove once in BOM -->
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>jakarta-mail-api</artifactId>
<version>2.1.3-1</version>
</dependency>
</dependencies>
</dependencyManagement>

Expand Down
28 changes: 26 additions & 2 deletions src/main/java/hudson/tasks/Mailer.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import hudson.Util;
import hudson.model.*;
import jenkins.plugins.mailer.tasks.i18n.Messages;
import jenkins.security.FIPS140;
import hudson.security.Permission;
import hudson.util.FormValidation;
import hudson.util.Secret;
Expand Down Expand Up @@ -408,8 +409,15 @@ private static Session createSession(String smtpHost, String smtpPort, boolean u
props.put("mail.smtp.starttls.enable", "true");
props.put("mail.smtp.starttls.required", "true");
}
if(smtpAuthUserName!=null)
if(smtpAuthUserName!=null) {

Check warning on line 412 in src/main/java/hudson/tasks/Mailer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 412 is only partially covered, one branch is missing
props.put("mail.smtp.auth","true");
if (FIPS140.useCompliantAlgorithms()) {
// the authentication mechanisms can only be performed when protected by TLS/SSL
if (!(useSsl || useTls)) {
throw new IllegalStateException("SMTP Authentication can only be used when either TLS or SSL is used");
}
}
}

// avoid hang by setting some timeout.
props.put("mail.smtp.timeout","60000");
Expand Down Expand Up @@ -703,7 +711,10 @@ public FormValidation doSendTestMail(
username = null;
password = null;
}

if (FIPS140.useCompliantAlgorithms() && authentication && !(useSsl || useTls)) {
return FormValidation.error(Messages.Mailer_InsecureAuthError());

Check warning on line 715 in src/main/java/hudson/tasks/Mailer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 414-715 are not covered by tests
}

MimeMessage msg = new MimeMessage(createSession(smtpHost, smtpPort, useSsl, useTls, username, password));
msg.setSubject(Messages.Mailer_TestMail_Subject(testEmailCount.incrementAndGet()), charset);
msg.setText(Messages.Mailer_TestMail_Content(testEmailCount.get(), Jenkins.get().getDisplayName()), charset);
Expand All @@ -721,6 +732,19 @@ public FormValidation doSendTestMail(
}
}

@RequirePOST
public FormValidation doCheckAuthentication(@QueryParameter boolean authentication, @QueryParameter boolean useSsl, @QueryParameter boolean useTls) {
Jenkins.get().checkPermission(Jenkins.MANAGE);
if (!authentication || useSsl || useTls) {
return FormValidation.ok();
}
// authentication is enabled without either TLS or SSL
if (FIPS140.useCompliantAlgorithms()) {

Check warning on line 742 in src/main/java/hudson/tasks/Mailer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 742 is only partially covered, one branch is missing
return FormValidation.error(Messages.Mailer_InsecureAuthError());

Check warning on line 743 in src/main/java/hudson/tasks/Mailer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 743 is not covered by tests
}
return FormValidation.warning(Messages.Mailer_InsecureAuthWarning());
}

public boolean isApplicable(Class<? extends AbstractProject> jobType) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ Mailer.EmailSentSuccessfully=Email was successfully sent
Mailer.FailedToSendEmail=Failed to send out e-mail
Mailer.TestMail.Subject=Test email #{0}
Mailer.TestMail.Content=This is test email #{0} sent from {1}
Mailer.InsecureAuthWarning=For security when using authentication it is recommended to enable either TLS or SSL
Mailer.InsecureAuthError=Authentication requires either TLS or SSL to be enabled

MailCommand.ShortDescription=\
Reads stdin and sends that out as an e-mail.
29 changes: 25 additions & 4 deletions src/test/java/hudson/tasks/MailerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import hudson.security.ACLContext;
import hudson.slaves.DumbSlave;
import hudson.tasks.Mailer.DescriptorImpl;
import hudson.util.FormValidation;
import hudson.util.Secret;
import org.hamcrest.MatcherAssert;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
Expand All @@ -40,7 +41,6 @@
import org.junit.Assume;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.FailureBuilder;
import org.jvnet.hudson.test.Email;
import org.jvnet.hudson.test.FakeChangeLogSCM;
Expand All @@ -65,6 +65,7 @@
import java.util.Optional;
import java.util.concurrent.atomic.AtomicLong;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;
Expand All @@ -74,7 +75,6 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

Expand Down Expand Up @@ -193,7 +193,7 @@ public void testFixEmptyAndTrimFour() throws Exception {
assertEquals("UTF-8",d.getCharset());
}

@Bug(1566)
@Issue("JENKINS-1566")
@Test
public void testSenderAddress() throws Exception {
// intentionally give the whole thin in a double quote
Expand Down Expand Up @@ -287,7 +287,28 @@ public void globalConfig() throws Exception {

assertNull(d.getAuthentication());
}


@Test
public void authenticationFormValidation() {
DescriptorImpl d = Mailer.descriptor();

// no authentication without TLS/SSL
assertThat(d.doCheckAuthentication(false, false, false), is(FormValidation.ok()));

// no authentication with TLS / SSL combos
assertThat(d.doCheckAuthentication(false, true, false), is(FormValidation.ok()));
assertThat(d.doCheckAuthentication(false, false, true), is(FormValidation.ok()));
assertThat(d.doCheckAuthentication(false, true, true), is(FormValidation.ok()));

// authentication without TLS/SSL
assertThat(d.doCheckAuthentication(true, false, false).kind, is(FormValidation.Kind.WARNING));

// authentication with TSL / SSL combos
assertThat(d.doCheckAuthentication(true, true, false), is(FormValidation.ok()));
assertThat(d.doCheckAuthentication(true, false, true), is(FormValidation.ok()));
assertThat(d.doCheckAuthentication(true, true, true), is(FormValidation.ok()));
}

/**
* Simulates {@link JenkinsLocationConfiguration} is not configured.
*/
Expand Down

0 comments on commit f7c289a

Please sign in to comment.