Skip to content

Commit

Permalink
[SECURITY-1682]
Browse files Browse the repository at this point in the history
  • Loading branch information
jeffret-b authored and Wadeck committed Jan 14, 2020
1 parent 6f35dbb commit 5054bc6
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 17 deletions.
31 changes: 27 additions & 4 deletions core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol3.java
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
package jenkins.slaves;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.Extension;
import hudson.ExtensionComponent;
import hudson.ExtensionList;
import hudson.Util;
import hudson.model.Computer;
import java.io.IOException;
import java.net.Socket;
import java.util.Collections;
import java.util.logging.Logger;
import javax.inject.Inject;

import jenkins.AgentProtocol;
import jenkins.model.Jenkins;
import jenkins.util.SystemProperties;
import jenkins.ExtensionFilter;
import org.jenkinsci.Symbol;
import org.jenkinsci.remoting.engine.JnlpConnectionState;
import org.jenkinsci.remoting.engine.JnlpProtocol3Handler;
Expand All @@ -29,6 +29,10 @@
@Extension
@Symbol("jnlp3")
public class JnlpSlaveAgentProtocol3 extends AgentProtocol {

private static final Logger logger = Logger.getLogger(JnlpSlaveAgentProtocol3.class.getName());
@Restricted(NoExternalUse.class)
public static final String ALLOW_UNSAFE = JnlpSlaveAgentProtocol3.class.getName() + ".ALLOW_UNSAFE";
private NioChannelSelector hub;

private JnlpProtocol3Handler handler;
Expand Down Expand Up @@ -73,4 +77,23 @@ public void handle(Socket socket) throws IOException, InterruptedException {
ExtensionList.lookup(JnlpAgentReceiver.class));
}

@Extension
@Restricted(NoExternalUse.class)
public static class Impl extends ExtensionFilter {
@Override
public <T> boolean allows(Class<T> type, ExtensionComponent<T> component) {
if (Boolean.getBoolean(ALLOW_UNSAFE)) {
return true;
}
if (!AgentProtocol.class.isAssignableFrom(type)) {
return true;
}
boolean isJnlp3 = component.getInstance().getClass().isAssignableFrom(JnlpSlaveAgentProtocol3.class);
if (isJnlp3) {
logger.info("Inbound TCP Agent Protocol/3 has been forcibly disabled for additional security reasons. To enable it yet again set the system property " + ALLOW_UNSAFE);
}
return !isJnlp3;
}
}

}
135 changes: 135 additions & 0 deletions test/src/test/java/jenkins/AgentProtocolSecurity1682Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
* The MIT License
*
* Copyright (c) 2020 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package jenkins;

import jenkins.model.Jenkins;
import jenkins.slaves.DeprecatedAgentProtocolMonitor;
import jenkins.slaves.JnlpSlaveAgentProtocol3;
import org.apache.commons.lang.StringUtils;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.recipes.LocalData;

import javax.annotation.CheckForNull;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;

import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

/**
* Tests for {@link AgentProtocol}.
*
* @author Oleg Nenashev
*/
public class AgentProtocolSecurity1682Test {

@Rule
public JenkinsRule j = new JenkinsRule();

@BeforeClass
public static void setupClass() {
System.setProperty(JnlpSlaveAgentProtocol3.ALLOW_UNSAFE, "true");
}

@Test
@LocalData
@Issue("SECURITY-1682")
public void testShouldReallyEnable3() {
assertEnabled("JNLP-connect", "JNLP3-connect");
assertDisabled("JNLP2-connect", "JNLP4-connect");
assertProtocols(true, "System protocols should be always enabled", "Ping");
assertMonitorTriggered("JNLP-connect", "JNLP3-connect");
}

private void assertEnabled(String ... protocolNames) throws AssertionError {
assertProtocols(true, null, protocolNames);
}

private void assertDisabled(String ... protocolNames) throws AssertionError {
assertProtocols(false, null, protocolNames);
}

private void assertProtocols(boolean shouldBeEnabled, @CheckForNull String why, String ... protocolNames) {
assertProtocols(j.jenkins, shouldBeEnabled, why, protocolNames);
}

public static void assertProtocols(Jenkins jenkins, boolean shouldBeEnabled, @CheckForNull String why, String ... protocolNames)
throws AssertionError {
Set<String> agentProtocols = jenkins.getAgentProtocols();
List<String> failedChecks = new ArrayList<>();
for (String protocol : protocolNames) {
if (shouldBeEnabled && !(agentProtocols.contains(protocol))) {
failedChecks.add(protocol);
}
if (!shouldBeEnabled && agentProtocols.contains(protocol)) {
failedChecks.add(protocol);
}
}

if (!failedChecks.isEmpty()) {
String message = String.format("Protocol(s) are not %s: %s. %sEnabled protocols: %s",
shouldBeEnabled ? "enabled" : "disabled",
StringUtils.join(failedChecks, ','),
why != null ? "Reason: " + why + ". " : "",
StringUtils.join(agentProtocols, ','));
fail(message);
}
}

public static void assertMonitorNotActive(JenkinsRule j) {
DeprecatedAgentProtocolMonitor monitor = new DeprecatedAgentProtocolMonitor();
assertFalse("Deprecated Agent Protocol Monitor should not be activated. Current protocols: "
+ StringUtils.join(j.jenkins.getAgentProtocols(), ","), monitor.isActivated());
}

public static void assertMonitorTriggered(String ... expectedProtocols) {
DeprecatedAgentProtocolMonitor monitor = new DeprecatedAgentProtocolMonitor();
assertTrue("Deprecated Agent Protocol Monitor should be activated", monitor.isActivated());
String protocolList = monitor.getDeprecatedProtocols();
assertThat("List of the protocols should not be null", protocolList, not(nullValue()));

List<String> failedChecks = new ArrayList<>();
for(String protocol : expectedProtocols) {
if (!protocolList.contains(protocol)) {
failedChecks.add(protocol);
}
}

if (!failedChecks.isEmpty()) {
String message = String.format(
"Protocol(s) should in the deprecated protocol list: %s. Current list: %s",
StringUtils.join(expectedProtocols, ','), protocolList);
fail(message);
}
}
}
17 changes: 4 additions & 13 deletions test/src/test/java/jenkins/AgentProtocolTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,24 +77,15 @@ public void testShouldNotDisableProtocolsForMigratedInstances() throws Exception
assertProtocols(false, "JNLP3-connect protocol should be disabled by default", "JNLP3-connect");
assertMonitorTriggered("JNLP-connect", "JNLP2-connect", "CLI-connect");
}

@Test
@LocalData
@Issue("JENKINS-45841")
public void testShouldNotOverrideUserConfiguration() throws Exception {
assertEnabled("CLI-connect", "JNLP-connect", "JNLP3-connect");
assertDisabled("CLI2-connect", "JNLP2-connect", "JNLP4-connect");
assertEnabled("JNLP-connect");
assertDisabled("JNLP2-connect", "JNLP4-connect");
assertProtocols(true, "System protocols should be always enabled", "Ping");
assertMonitorTriggered("JNLP-connect", "JNLP3-connect", "CLI-connect");
}

@Test
@LocalData
public void testShouldDisableCLIProtocolsWhenCLIisDisabled() throws Exception {
assertProtocols(false, "CLI is forcefully disabled, protocols should be blocked",
"CLI-connect", "CLI2-connect");
assertEnabled("JNLP3-connect");
assertMonitorTriggered("JNLP3-connect");
assertMonitorTriggered("JNLP-connect");
}

private void assertEnabled(String ... protocolNames) throws AssertionError {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?xml version='1.0' encoding='UTF-8'?>
<hudson>
<disabledAdministrativeMonitors/>
<version>1.0</version>
<numExecutors>2</numExecutors>
<mode>NORMAL</mode>
<useSecurity>true</useSecurity>
<authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
<securityRealm class="hudson.security.SecurityRealm$None"/>
<disableRememberMe>false</disableRememberMe>
<projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
<workspaceDir>${ITEM_ROOTDIR}/workspace</workspaceDir>
<buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
<markupFormatter class="hudson.markup.EscapedMarkupFormatter"/>
<jdks/>
<viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
<myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
<clouds/>
<scmCheckoutRetryCount>0</scmCheckoutRetryCount>
<views>
<hudson.model.AllView>
<owner class="hudson" reference="../../.."/>
<name>all</name>
<filterExecutors>false</filterExecutors>
<filterQueue>false</filterQueue>
<properties class="hudson.model.View$PropertyList"/>
</hudson.model.AllView>
</views>
<primaryView>all</primaryView>
<slaveAgentPort>0</slaveAgentPort>
<!-- YOLO: unstable configuration -->
<enabledAgentProtocols>
<!-- Since removed, but may as well test robustness of load: -->
<string>CLI-connect</string>
<string>JNLP-connect</string>
<string>JNLP3-connect</string>
</enabledAgentProtocols>
<disabledAgentProtocols>
<string>JNLP4-connect</string>
<string>JNLP2-connect</string>
<!-- Ditto: -->
<string>CLI2-connect</string>
</disabledAgentProtocols>
<label></label>
<nodeProperties/>
<globalNodeProperties/>
</hudson>

0 comments on commit 5054bc6

Please # to comment.