opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.14'

Browse Source 
* stable-2.14:
  Use account cache instead of ReviewDb in MailProcessor
  Change log message to reflect IMAP RFC
  Get rid of calls to SecurityUtils.isBouncyCastleRegistered()
  SshDaemon: Improve log message when formatting ssh host key fails

Change-Id: Ibbc3a1d32c7127335840233d66324fca7f664e12
This commit is contained in:
David Pursehouse
2017-05-08 23:09:23 +09:00
parent 37d85c3959 9269cb5687
commit 75c9189fc2
5 changed files with 100 additions and 154 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitSshd.java
View File

@@ -14,8 +14,6 @@
package com.google.gerrit.pgm.init;
import static com.google.gerrit.common.FileUtil.chmod;
import static com.google.gerrit.pgm.init.api.InitUtil.die;
import static com.google.gerrit.pgm.init.api.InitUtil.hostname;
import static java.nio.file.Files.exists;
@@ -30,10 +28,6 @@ import com.google.inject.Singleton;
import java.io.IOException;
import java.lang.ProcessBuilder.Redirect;
import java.net.InetSocketAddress;
import java.nio.file.Files;
import java.nio.file.Path;
import org.apache.sshd.common.util.security.SecurityUtils;
import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
/** Initialize the {@code sshd} configuration section. */
@Singleton
@@ -92,7 +86,6 @@ class InitSshd implements InitStep {
System.err.print("Generating SSH host key ...");
System.err.flush();
if (SecurityUtils.isBouncyCastleRegistered()) {
// Generate the SSH daemon host key using ssh-keygen.
//
final String comment = "gerrit-code-review@" + hostname();
@@ -190,43 +183,6 @@ class InitSshd implements InitStep {
System.err.flush();
}
}
} else {
// Generate the SSH daemon host key ourselves. This is complex
// because SimpleGeneratorHostKeyProvider doesn't mark the data
// file as only readable by us, exposing the private key for a
// short period of time. We try to reduce that risk by creating
// the key within a temporary directory.
//
Path tmpdir = site.etc_dir.resolve("tmp.sshkeygen");
try {
Files.createDirectory(tmpdir);
} catch (IOException e) {
throw die("Cannot create directory " + tmpdir, e);
}
chmod(0600, tmpdir);
Path tmpkey = tmpdir.resolve(site.ssh_key.getFileName().toString());
SimpleGeneratorHostKeyProvider p;
System.err.print(" rsa(simple)...");
System.err.flush();
p = new SimpleGeneratorHostKeyProvider();
p.setPath(tmpkey.toAbsolutePath());
p.setAlgorithm("RSA");
p.loadKeys(); // forces the key to generate.
chmod(0600, tmpkey);
try {
Files.move(tmpkey, site.ssh_key);
} catch (IOException e) {
throw die("Cannot rename " + tmpkey + " to " + site.ssh_key, e);
}
try {
Files.delete(tmpdir);
} catch (IOException e) {
throw die("Cannot delete " + tmpdir, e);
}
}
System.err.println(" done");
}
}

3
gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ImapMailReceiver.java
View File

@@ -71,7 +71,8 @@ public class ImapMailReceiver extends MailReceiver {
// Fetch just the internal dates first to know how many messages we
// should fetch.
if (!imap.fetch("1:*", "(INTERNALDATE)")) {
log.error("IMAP fetch failed. Will retry in next fetch cycle.");
// false indicates that there are no messages to fetch
log.info("Fetched 0 messages via IMAP");
return;
}
// Format of reply is one line per email and one line to indicate

6
gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailProcessor.java
View File

@@ -38,6 +38,7 @@ import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountByEmailCache;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.change.EmailReviewComments;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.extensions.events.CommentAdded;
@@ -84,6 +85,7 @@ public class MailProcessor {
private final EmailReviewComments.Factory outgoingMailFactory;
private final CommentAdded commentAdded;
private final ApprovalsUtil approvalsUtil;
private final AccountCache accountCache;
private final Provider<String> canonicalUrl;
@Inject
@@ -101,6 +103,7 @@ public class MailProcessor {
EmailReviewComments.Factory outgoingMailFactory,
ApprovalsUtil approvalsUtil,
CommentAdded commentAdded,
AccountCache accountCache,
@CanonicalWebUrl Provider<String> canonicalUrl) {
this.accountByEmailCache = accountByEmailCache;
this.buf = buf;
@@ -115,6 +118,7 @@ public class MailProcessor {
this.outgoingMailFactory = outgoingMailFactory;
this.commentAdded = commentAdded;
this.approvalsUtil = approvalsUtil;
this.accountCache = accountCache;
this.canonicalUrl = canonicalUrl;
}
@@ -153,7 +157,7 @@ public class MailProcessor {
return;
}
Account.Id account = accounts.iterator().next();
if (!reviewDb.get().accounts().get(account).isActive()) {
if (!accountCache.get(account).getAccount().isActive()) {
log.warn(String.format("Mail: Account %s is inactive. Will delete message.", account));
return;
}

8
gerrit-sshd/src/main/java/com/google/gerrit/sshd/HostKeyProvider.java
View File

@@ -25,7 +25,6 @@ import java.util.ArrayList;
import java.util.List;
import org.apache.sshd.common.keyprovider.FileKeyPairProvider;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.util.security.SecurityUtils;
import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
class HostKeyProvider implements Provider<KeyPairProvider> {
@@ -73,13 +72,6 @@ class HostKeyProvider implements Provider<KeyPairProvider> {
if (stdKeys.isEmpty()) {
throw new ProvisionException("No SSH keys under " + site.etc_dir);
}
if (!SecurityUtils.isBouncyCastleRegistered()) {
throw new ProvisionException(
"Bouncy Castle Crypto not installed;"
+ " needed to read server host keys: "
+ stdKeys
+ "");
}
FileKeyPairProvider kp = new FileKeyPairProvider();
kp.setFiles(stdKeys);
return kp;

13
gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshDaemon.java
View File

@@ -84,7 +84,6 @@ import org.apache.sshd.common.io.nio2.Nio2ServiceFactoryFactory;
import org.apache.sshd.common.kex.KeyExchange;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.mac.Mac;
import org.apache.sshd.common.random.JceRandomFactory;
import org.apache.sshd.common.random.Random;
import org.apache.sshd.common.random.SingletonRandomFactory;
import org.apache.sshd.common.session.ConnectionService;
@@ -217,11 +216,7 @@ public class SshDaemon extends SshServer implements SshInfo, LifecycleListener {
? MinaServiceFactoryFactory.class.getName()
: Nio2ServiceFactoryFactory.class.getName());
if (SecurityUtils.isBouncyCastleRegistered()) {
initProviderBouncyCastle(cfg);
} else {
initProviderJce();
}
initCiphers(cfg);
initKeyExchanges(cfg);
initMacs(cfg);
@@ -405,7 +400,9 @@ public class SshDaemon extends SshServer implements SshInfo, LifecycleListener {
try {
r.add(new HostKey(addr, keyBin));
} catch (JSchException e) {
sshDaemonLog.warn("Cannot format SSHD host key", e);
sshDaemonLog.warn(
String.format(
"Cannot format SSHD host key [%s]: %s", pub.getAlgorithm(), e.getMessage()));
}
}
}
@@ -524,10 +521,6 @@ public class SshDaemon extends SshServer implements SshInfo, LifecycleListener {
}
}
private void initProviderJce() {
setRandomFactory(new SingletonRandomFactory(JceRandomFactory.INSTANCE));
}
@SuppressWarnings("unchecked")
private void initCiphers(final Config cfg) {
final List<NamedFactory<Cipher>> a = BaseBuilder.setUpDefaultCiphers(true);