Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-39738] - Enable aes192ctr and aes256ctr ciphers if JVM…
… supports them (#14)

* [FIXED JENKINS-39738] - Enable aes192ctr and aes256ctr ciphers if JVM supports them

If the JVM supports unlimited-strength encryption, we can enable more ciphers.
And the new SSHD core version provides good API for it.

CBC ciphers won't be added due to https://www.kb.cert.org/vuls/id/958563

* [JENKINS-39738] - Use FINE logging level for Disabled ciphers

* [JENKINS-39738] - Address comment from @jglick regarding the logging formatters
  • Loading branch information
oleg-nenashev committed Apr 25, 2017
1 parent 499228b commit bb69634
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 7 deletions.
45 changes: 39 additions & 6 deletions src/main/java/org/jenkinsci/main/modules/sshd/SSHD.java
Expand Up @@ -6,10 +6,13 @@
import hudson.init.Initializer;
import java.io.IOException;
import java.security.KeyPair;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nonnull;
import javax.annotation.concurrent.GuardedBy;
import javax.inject.Inject;
import jenkins.model.GlobalConfiguration;
Expand All @@ -32,6 +35,14 @@
@Extension
public class SSHD extends GlobalConfiguration {

//TODO: Make Ciphers configurable from UI, with logic similar to Remoting protocols?
/**
* Lists Built-in Ciphers, which are enabled by default in SSH Core
*/
private static final List<NamedFactory<Cipher>> ENABLED_CIPHERS = Arrays.<NamedFactory<Cipher>>asList(
BuiltinCiphers.aes128ctr, BuiltinCiphers.aes192ctr, BuiltinCiphers.aes256ctr
);

@Override
public GlobalConfigurationCategory getCategory() {
return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Security.class);
Expand Down Expand Up @@ -83,18 +94,40 @@ public void run() {
}
}

/**
* Provides a list of Cipher factories, which can be activated on the instance.
* Cyphers will be considered as activated if they are defined in {@link #ENABLED_CIPHERS} and supported in the current JVM.
* @return List of factories
*/
@Nonnull
/*package*/ static List<NamedFactory<Cipher>> getActivatedCiphers() {
final List<NamedFactory<Cipher>> activatedCiphers = new ArrayList<>(ENABLED_CIPHERS.size());
for (NamedFactory<Cipher> cipher : ENABLED_CIPHERS) {
if (cipher instanceof BuiltinCiphers) {
final BuiltinCiphers c = (BuiltinCiphers)cipher;
if (c.isSupported()) {
activatedCiphers.add(cipher);
} else {
LOGGER.log(Level.FINE, "Discovered unsupported built-in Cipher: {0}. It will not be enabled", c);
}
} else {
// We cannot determine if the cipher is supported, but the default configuration lists only Built-in ciphers.
// So somebody explicitly added it on his own risk.
activatedCiphers.add(cipher);
}
}
return activatedCiphers;
}

public synchronized void start() throws IOException, InterruptedException {
if (port<0) return; // don't start it

stop();
sshd = SshServer.setUpDefaultServer();

sshd.setUserAuthFactories(Arrays.<NamedFactory<UserAuth>>asList(new UserAuthNamedFactory()));

sshd.setCipherFactories(Arrays.<NamedFactory<Cipher>>asList(// AES 256 and 192 requires unlimited crypto, so don't use that
// CBC modes are not secure, so they have been dropped (see JENKINS-39805)
BuiltinCiphers.aes128ctr));


sshd.setCipherFactories(getActivatedCiphers());
sshd.setPort(port);

sshd.setKeyPairProvider(new AbstractKeyPairProvider() {
Expand Down Expand Up @@ -127,7 +160,7 @@ public Iterable<KeyPair> loadKeys() {
sshd.start();
LOGGER.info("Started SSHD at port " + sshd.getPort());
}

public synchronized void restart() {
try {
if (sshd!=null) {
Expand Down
@@ -1,14 +1,19 @@
package org.jenkinsci.main.modules.ssh;
package org.jenkinsci.main.modules.sshd;

import java.util.List;
import org.hamcrest.CoreMatchers;
import org.jenkinsci.main.modules.sshd.SSHD;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;

import javax.inject.Inject;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.cipher.Cipher;
import org.junit.Assert;

import static org.junit.Assert.assertThat;
import org.jvnet.hudson.test.Issue;

/**
* Tests of {@link SSHD}.
Expand All @@ -34,4 +39,12 @@ public void configRoundtrip() throws Exception {
assertThat("SSHD has not been allocated to the specified port", sshd.getPort(), CoreMatchers.equalTo(i));
}
}

@Test
@Issue("JENKINS-39738")
public void checkActivatedCiphers() throws Exception {
// Just ensure the method does not blow up && that at least one Cipher is available
List<NamedFactory<Cipher>> activatedCiphers = SSHD.getActivatedCiphers();
Assert.assertTrue("At least one cipher should be activated", activatedCiphers.size() >= 1);
}
}

0 comments on commit bb69634

Please sign in to comment.