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

[JENKINS-60381] Remove old, deprecated agent protocols. #4387

Merged
merged 5 commits into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/slaves/SlaveComputer.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
import jenkins.security.ChannelConfigurator;
import jenkins.security.MasterToSlaveCallable;
import jenkins.slaves.EncryptedSlaveAgentJnlpFile;
import jenkins.slaves.JnlpSlaveAgentProtocol;
import jenkins.slaves.JnlpAgentReceiver;
import jenkins.slaves.RemotingVersionInfo;
import jenkins.slaves.systemInfo.SlaveSystemInfo;
import jenkins.util.SystemProperties;
Expand Down Expand Up @@ -180,7 +180,7 @@ public boolean isAcceptingTasks() {
* @since 1.498
*/
public String getJnlpMac() {
return JnlpSlaveAgentProtocol.SLAVE_SECRET.mac(getName());
return JnlpAgentReceiver.SLAVE_SECRET.mac(getName());
}

/**
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/jenkins/security/ConfidentialKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@

import javax.annotation.CheckForNull;
import java.io.IOException;
import jenkins.slaves.JnlpSlaveAgentProtocol;

import jenkins.slaves.JnlpAgentReceiver;

/**
* Confidential information that gets stored as a singleton in Jenkins, mostly some random token value.
Expand All @@ -25,7 +26,7 @@
* for the secret to leak.
*
* <p>
* The {@link ConfidentialKey} subtypes are expected to be used as a singleton, like {@link JnlpSlaveAgentProtocol#SLAVE_SECRET}.
* The {@link ConfidentialKey} subtypes are expected to be used as a singleton, like {@link JnlpAgentReceiver#SLAVE_SECRET}.
* For code that relies on XStream for persistence (such as {@link Builder}s, {@link SCM}s, and other fragment objects
* around builds and jobs), {@link Secret} provides more convenient way of storing secrets.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,5 +216,5 @@ public void setLog(@Nonnull OutputStream log) {

private static final Logger LOGGER = Logger.getLogger(DefaultJnlpSlaveReceiver.class.getName());

private static final String COOKIE_NAME = JnlpSlaveAgentProtocol2.class.getName()+".cookie";
jeffret-b marked this conversation as resolved.
Show resolved Hide resolved
private static final String COOKIE_NAME = "JnlpAgentProtocol.cookie";
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void generateResponse(StaplerRequest req, final StaplerResponse res, Obje
if(it instanceof SlaveComputer) {
jnlpMac = Util.fromHexString(((SlaveComputer)it).getJnlpMac());
} else {
jnlpMac = JnlpSlaveAgentProtocol.SLAVE_SECRET.mac(slaveName.getBytes(StandardCharsets.UTF_8));
jnlpMac = JnlpAgentReceiver.SLAVE_SECRET.mac(slaveName.getBytes(StandardCharsets.UTF_8));
}
SecretKey key = new SecretKeySpec(jnlpMac, 0, /* export restrictions */ 128 / 8, "AES");
byte[] encrypted;
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/java/jenkins/slaves/JnlpAgentReceiver.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
import hudson.model.Slave;
import java.security.SecureRandom;
import javax.annotation.Nonnull;

import jenkins.security.HMACConfidentialKey;
import org.jenkinsci.remoting.engine.JnlpClientDatabase;
import org.jenkinsci.remoting.engine.JnlpConnectionStateListener;

/**
* Receives incoming agents connecting through {@link JnlpSlaveAgentProtocol2}, {@link JnlpSlaveAgentProtocol3}, {@link JnlpSlaveAgentProtocol4}.
* Receives incoming agents connecting through {@link JnlpSlaveAgentProtocol4}.
*
* <p>
* This is useful to establish the communication with other JVMs and use them
Expand All @@ -29,6 +31,12 @@
*/
public abstract class JnlpAgentReceiver extends JnlpConnectionStateListener implements ExtensionPoint {

/**
* This secret value is used as a seed for agents.
*/
public static final HMACConfidentialKey SLAVE_SECRET =
new HMACConfidentialKey(JnlpSlaveAgentProtocol.class, "secret");

private static final SecureRandom secureRandom = new SecureRandom();

public static final JnlpClientDatabase DATABASE = new JnlpAgentDatabase();
Expand Down Expand Up @@ -62,7 +70,7 @@ public boolean exists(String clientName) {

@Override
public String getSecretOf(@Nonnull String clientName) {
return JnlpSlaveAgentProtocol.SLAVE_SECRET.mac(clientName);
return SLAVE_SECRET.mac(clientName);
}
}
}
95 changes: 7 additions & 88 deletions core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java
Original file line number Diff line number Diff line change
@@ -1,97 +1,16 @@
package jenkins.slaves;

import hudson.Extension;
import hudson.ExtensionList;
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.security.HMACConfidentialKey;
import org.jenkinsci.Symbol;
import org.jenkinsci.remoting.engine.JnlpConnectionState;
import org.jenkinsci.remoting.engine.JnlpProtocol1Handler;

/**
* {@link AgentProtocol} that accepts connection from agents.
*
* <h2>Security</h2>
* <p>
* Once connected, remote agents can send in commands to be
* executed on the master, so in a way this is like an rsh service.
* Therefore, it is important that we reject connections from
* unauthorized remote agents.
*
* <p>
* We do this by computing HMAC of the agent name.
* This code is sent to the agent inside the {@code .jnlp} file
* (this file itself is protected by HTTP form-based authentication that
* we use everywhere else in Jenkins), and the agent sends this
* token back when it connects to the master.
* Unauthorized agents can't access the protected {@code .jnlp} file,
* so it can't impersonate a valid agent.
*
* <p>
* We don't want to force the inbound agents to be restarted
* whenever the server restarts, so right now this secret master key
* is generated once and used forever, which makes this whole scheme
* less secure.
*
* @author Kohsuke Kawaguchi
* @since 1.467
* This class was part of the old JNLP1 protocol, which has been removed.
* The SLAVE_SECRET was still used by some plugins. It has been moved to
* JnlpAgentReceiver as a more suitable location, but this alias retained
* for compatibility. References should be updated to the new location.
*/
@Extension
@Symbol("jnlp")
public class JnlpSlaveAgentProtocol extends AgentProtocol {
/**
* Our logger
*/
private static final Logger LOGGER = Logger.getLogger(JnlpSlaveAgentProtocol.class.getName());
/**
* This secret value is used as a seed for agents.
*/
public static final HMACConfidentialKey SLAVE_SECRET =
new HMACConfidentialKey(JnlpSlaveAgentProtocol.class, "secret");

private NioChannelSelector hub;

private JnlpProtocol1Handler handler;

@Inject
public void setHub(NioChannelSelector hub) {
this.hub = hub;
this.handler = new JnlpProtocol1Handler(JnlpAgentReceiver.DATABASE, Computer.threadPoolForRemoting,
hub.getHub(), true);
}

@Override
public boolean isOptIn() {
return true;
}

@Override
public boolean isDeprecated() {
return true;
}

@Override
public String getName() {
return handler.isEnabled() ? handler.getName() : null;
}

@Override
public String getDisplayName() {
return Messages.JnlpSlaveAgentProtocol_displayName();
}

@Override
public void handle(Socket socket) throws IOException, InterruptedException {
handler.handle(socket,
Collections.singletonMap(JnlpConnectionState.COOKIE_KEY, JnlpAgentReceiver.generateCookie()),
ExtensionList.lookup(JnlpAgentReceiver.class));
}
@Deprecated
public class JnlpSlaveAgentProtocol {

public static final HMACConfidentialKey SLAVE_SECRET = JnlpAgentReceiver.SLAVE_SECRET;

}
67 changes: 0 additions & 67 deletions core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol2.java

This file was deleted.

Loading