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

[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 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
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