Skip to content
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

Warn when using insecure auth #275

Merged
merged 1 commit into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,302 +409,312 @@
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");
props.put("mail.smtp.connectiontimeout","60000");

return Session.getInstance(props,getAuthenticator(smtpAuthUserName,Secret.toString(smtpAuthPassword)));
}

private static Authenticator getAuthenticator(final String smtpAuthUserName, final String smtpAuthPassword) {
if(smtpAuthUserName == null) {
return null;
}
return new Authenticator() {
@Override
protected PasswordAuthentication getPasswordAuthentication() {
return new PasswordAuthentication(smtpAuthUserName,smtpAuthPassword);
}
};
}

@Override
public boolean configure(StaplerRequest req, JSONObject json) throws FormException {

// Nested Describable (SMTPAuthentication) is not set to null in case it is not configured.
// To mitigate that, it is being set to null before (so it gets set to sent value or null correctly) and, in
// case of failure to databind, it gets reverted to previous value.
// Would not be necessary by https://github.com/jenkinsci/jenkins/pull/3669
SMTPAuthentication current = this.authentication;

try (BulkChange b = new BulkChange(this)) {
this.authentication = null;
req.bindJSON(this, json);
b.commit();
} catch (IOException e) {
this.authentication = current;
throw new FormException("Failed to apply configuration", e, null);
}

return true;
}

private String nullify(String v) {
if(v!=null && v.length()==0) v=null;
return v;
}

public String getSmtpHost() {
return smtpHost;
}


/** @deprecated as of 1.23, use {@link #getSmtpHost()} */
@Deprecated
public String getSmtpServer() {
return smtpHost;
}

/**
* Method added to pass findbugs verification when compiling against 1.642.1
* @return The JenkinsLocationConfiguration object.
* @throws IllegalStateException if the object is not available (e.g., Jenkins not fully initialized).
*/
@SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE",
justification = "False positive. See https://sourceforge.net/p/findbugs/bugs/1411/")
private JenkinsLocationConfiguration getJenkinsLocationConfiguration() {
final JenkinsLocationConfiguration jlc = JenkinsLocationConfiguration.get();
if (jlc == null) {
throw new IllegalStateException("JenkinsLocationConfiguration not available");
}
return jlc;
}

/**
* @deprecated as of 1.4
* Use {@link JenkinsLocationConfiguration} instead
* @return administrator mail address
*/
@Deprecated
public String getAdminAddress() {
return getJenkinsLocationConfiguration().getAdminAddress();
}

/**
* @deprecated as of 1.4
* Use {@link JenkinsLocationConfiguration} instead
* @return Jenkins base URL
*/
@Deprecated
public String getUrl() {
return getJenkinsLocationConfiguration().getUrl();
}

/**
* @deprecated as of 1.21
* Use {@link #authentication}
*/
@Deprecated
public String getSmtpAuthUserName() {
if (authentication == null) return null;
return authentication.getUsername();
}

/**
* @deprecated as of 1.21
* Use {@link #authentication}
*/
@Deprecated
public String getSmtpAuthPassword() {
if (authentication == null) return null;
return Secret.toString(authentication.getPassword());
}

public Secret getSmtpAuthPasswordSecret() {
if (authentication == null) return null;
return authentication.getPassword();
}

public boolean getUseSsl() {
return useSsl;
}

public boolean getUseTls() {
return useTls;
}

public String getSmtpPort() {
return smtpPort;
}

public String getCharset() {
String c = charset;
if (c == null || c.length() == 0) c = "UTF-8";
return c;
}

@DataBoundSetter
public void setDefaultSuffix(String defaultSuffix) {
this.defaultSuffix = defaultSuffix;
save();
}

/**
* @deprecated as of 1.4
* Use {@link JenkinsLocationConfiguration} instead
* @param hudsonUrl Jenkins base URL to set
*/
@Deprecated
public void setHudsonUrl(String hudsonUrl) {
getJenkinsLocationConfiguration().setUrl(hudsonUrl);
}

/**
* @deprecated as of 1.4
* Use {@link JenkinsLocationConfiguration} instead
* @param adminAddress Jenkins administrator mail address to set
*/
@Deprecated
public void setAdminAddress(String adminAddress) {
getJenkinsLocationConfiguration().setAdminAddress(adminAddress);
}

@DataBoundSetter
public void setSmtpHost(String smtpHost) {
this.smtpHost = Util.fixEmptyAndTrim(smtpHost);
save();
}

@DataBoundSetter
public void setUseSsl(boolean useSsl) {
this.useSsl = useSsl;
save();
}

@DataBoundSetter
public void setUseTls(boolean useTls) {
this.useTls = useTls;
save();
}

@DataBoundSetter
public void setSmtpPort(String smtpPort) {
this.smtpPort = Util.fixEmptyAndTrim(smtpPort);
save();
}

@DataBoundSetter
public void setCharset(String charset) {
if (charset == null || charset.length() == 0) {
charset = "UTF-8";
}
this.charset = Util.fixEmptyAndTrim(charset);
save();
}

@DataBoundSetter
public void setAuthentication(@CheckForNull SMTPAuthentication authentication) {
this.authentication = authentication;
save();
}

@CheckForNull
public SMTPAuthentication getAuthentication() {
return authentication;
}

/**
* @deprecated as of 1.21
* Use {@link #authentication}
*/
@Deprecated
public void setSmtpAuth(String userName, String password) {
if (userName == null && password == null) {
this.authentication = null;
} else {
this.authentication = new SMTPAuthentication(userName, Secret.fromString(password));
}
}

@Override
public Publisher newInstance(StaplerRequest req, JSONObject formData) throws FormException {
Mailer m = (Mailer)super.newInstance(req, formData);

if(hudsonUrl==null) {
// if Hudson URL is not configured yet, infer some default
hudsonUrl = Functions.inferHudsonURL(req);
save();
}

return m;
}

private Object readResolve() {
if (smtpAuthPassword != null) {
authentication = new SMTPAuthentication(smtpAuthUsername, smtpAuthPassword);
}
return this;
}

public FormValidation doAddressCheck(@QueryParameter String value) {
try {
new InternetAddress(value);
return FormValidation.ok();
} catch (AddressException e) {
return FormValidation.error(e.getMessage());
}
}

@RequirePOST
public FormValidation doCheckSmtpHost(@QueryParameter String value) {
Jenkins.get().checkPermission(Jenkins.MANAGE);
try {
if (Util.fixEmptyAndTrim(value)!=null)
InetAddress.getByName(value);
return FormValidation.ok();
} catch (UnknownHostException e) {
return FormValidation.error(Messages.Mailer_Unknown_Host_Name()+value);
}
}

public FormValidation doCheckDefaultSuffix(@QueryParameter String value) {
if (value.matches("@[A-Za-z0-9.\\-]+") || Util.fixEmptyAndTrim(value)==null)
return FormValidation.ok();
else
return FormValidation.error(Messages.Mailer_Suffix_Error());
}

/**
* Send an email to the admin address
* @throws IOException in case the active jenkins instance cannot be retrieved
* @param smtpHost name of the SMTP server to use for mail sending
* @param adminAddress Jenkins administrator mail address
* @param authentication if set to {@code true} SMTP is used without authentication (username and password)
* @param username plaintext username for SMTP authentication
* @param password secret password for SMTP authentication
* @param useSsl if set to {@code true} SSL is used
* @param useTls if set to {@code true} TLS is used
* @param smtpPort port to use for SMTP transfer
* @param charset charset of the underlying MIME-mail message
* @param sendTestMailTo mail address to send test mail to
* @return response with http status code depending on the result of the mail sending
*/
@RequirePOST
public FormValidation doSendTestMail(
@QueryParameter String smtpHost, @QueryParameter String adminAddress, @QueryParameter boolean authentication,
@QueryParameter String username, @QueryParameter Secret password,
@QueryParameter boolean useSsl, @QueryParameter boolean useTls, @QueryParameter String smtpPort, @QueryParameter String charset,
@QueryParameter String sendTestMailTo) throws IOException {
try {
Jenkins.get().checkPermission(Jenkins.MANAGE);
if (!authentication) {
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 @@
}
}

@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
Loading