diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 257bf6f3af..5f9b369c3e 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -5389,7 +5389,13 @@ The optional file `'$site_path'/etc/peer_keys` controls who can login as the 'Gerrit Code Review' user, required for the link:cmd-suexec.html[suexec] command. -The format is one Base-64 encoded public key per line. +The format is one Base-64 encoded public key per line with optional comment, e.g.: +---- +# Comments allowed at start of line +AAAAC3...51R== john@example.net +# Another comment +AAAAB5...21S== jane@example.net +---- === Configurable Parameters diff --git a/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java b/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java index 916775dc61..6997d9625b 100644 --- a/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java +++ b/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.base.Splitter; import com.google.common.flogger.FluentLogger; import com.google.common.io.BaseEncoding; import com.google.gerrit.common.FileUtil; @@ -38,6 +39,7 @@ import java.security.PublicKey; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Locale; import java.util.Set; import org.apache.sshd.common.SshException; @@ -197,9 +199,15 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator { continue; } + List parts = Splitter.on(' ').splitToList(line); + if (parts.size() > 2) { + throw new IllegalArgumentException( + "Invalid peer key file format, only lines supported"); + } try { byte[] bin = - BaseEncoding.base64().decode(new String(line.getBytes(ISO_8859_1), ISO_8859_1)); + BaseEncoding.base64() + .decode(new String(parts.get(0).getBytes(ISO_8859_1), ISO_8859_1)); keys.add(new ByteArrayBuffer(bin).getRawPublicKey()); } catch (RuntimeException | SshException e) { logBadKey(path, line, e); diff --git a/javatests/com/google/gerrit/integration/ssh/BUILD b/javatests/com/google/gerrit/integration/ssh/BUILD new file mode 100644 index 0000000000..dc8e68c90d --- /dev/null +++ b/javatests/com/google/gerrit/integration/ssh/BUILD @@ -0,0 +1,7 @@ +load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests") + +acceptance_tests( + srcs = ["PeerKeysAuthIT.java"], + group = "peer-keys-auth", + labels = ["ssh"], +) diff --git a/javatests/com/google/gerrit/integration/ssh/PeerKeysAuthIT.java b/javatests/com/google/gerrit/integration/ssh/PeerKeysAuthIT.java new file mode 100644 index 0000000000..a219cc211d --- /dev/null +++ b/javatests/com/google/gerrit/integration/ssh/PeerKeysAuthIT.java @@ -0,0 +1,104 @@ +// Copyright (C) 2020 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.integration.ssh; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static java.nio.charset.StandardCharsets.ISO_8859_1; +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress; +import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.StandaloneSiteTest; +import com.google.gerrit.acceptance.UseSsh; +import com.google.gerrit.common.Version; +import com.google.gerrit.server.PeerDaemonUser; +import com.google.inject.Inject; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.file.Files; +import org.junit.Test; + +@NoHttpd +@UseSsh +public class PeerKeysAuthIT extends StandaloneSiteTest { + private static final String[] SSH_KEYGEN_CMD = + new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f", "id_rsa"}; + private static final String[] SSH_COMMAND = + new String[] { + "ssh", + "-o", + "UserKnownHostsFile=/dev/null", + "-o", + "StrictHostKeyChecking=no", + "-o", + "IdentitiesOnly=yes", + "-i" + }; + + @Inject private @TestSshServerAddress InetSocketAddress sshAddress; + + @Test + public void test() throws Exception { + try (ServerContext ctx = startServer()) { + ctx.getInjector().injectMembers(this); + // Generate private/public key for user + execute(ImmutableList.builder().add(SSH_KEYGEN_CMD).build()); + + String[] parts = + new String(Files.readAllBytes(sitePaths.data_dir.resolve("id_rsa.pub")), UTF_8) + .split(" "); + + // Loose algorithm at index 0, verify the format: "key comment" + Files.write( + sitePaths.peer_keys, String.format("%s %s", parts[1], parts[2]).getBytes(ISO_8859_1)); + assertContent(execGerritVersionCommand()); + + // Only preserve the key material: no algorithm and no comment + Files.write(sitePaths.peer_keys, parts[1].getBytes(ISO_8859_1)); + assertContent(execGerritVersionCommand()); + + // Wipe out the content of the peer keys file + Files.delete(sitePaths.peer_keys); + assertThrows(IOException.class, () -> execGerritVersionCommand()); + } + } + + private String execGerritVersionCommand() throws Exception { + return execute( + ImmutableList.builder() + .add(SSH_COMMAND) + .add(sitePaths.data_dir.resolve("id_rsa").toString()) + .add("-p " + sshAddress.getPort()) + .add(PeerDaemonUser.USER_NAME + "@" + sshAddress.getHostName()) + .add("suexec") + .add("--as") + .add("admin") + .add("--") + .add("gerrit") + .add("version") + .build()); + } + + private String execute(ImmutableList cmd) throws Exception { + return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of()); + } + + private static void assertContent(String result) { + assertThat(result).contains("gerrit version " + Version.getVersion()); + } +}