diff options
4 files changed, 75 insertions, 3 deletions
diff --git a/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java b/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java new file mode 100644 index 0000000000..5f09658ba2 --- /dev/null +++ b/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java @@ -0,0 +1,44 @@ +// Copyright (C) 2024 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.sshd; + +import java.security.PublicKey; +import java.security.spec.InvalidKeySpecException; + +public class InvalidKeyAlgorithmException extends InvalidKeySpecException { + private final String invalidKeyAlgo; + private final String expectedKeyAlgo; + private final PublicKey publicKey; + + public InvalidKeyAlgorithmException( + String invalidKeyAlgo, String expectedKeyAlgo, PublicKey publicKey) { + super("Key algorithm mismatch: expected " + expectedKeyAlgo + " but got " + invalidKeyAlgo); + this.invalidKeyAlgo = invalidKeyAlgo; + this.expectedKeyAlgo = expectedKeyAlgo; + this.publicKey = publicKey; + } + + public String getInvalidKeyAlgo() { + return invalidKeyAlgo; + } + + public String getExpectedKeyAlgo() { + return expectedKeyAlgo; + } + + public PublicKey getPublicKey() { + return publicKey; + } +} diff --git a/java/com/google/gerrit/sshd/SshKeyCacheImpl.java b/java/com/google/gerrit/sshd/SshKeyCacheImpl.java index 773c25bce6..713435d302 100644 --- a/java/com/google/gerrit/sshd/SshKeyCacheImpl.java +++ b/java/com/google/gerrit/sshd/SshKeyCacheImpl.java @@ -19,6 +19,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USE import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.exceptions.InvalidSshKeyException; import com.google.gerrit.server.account.AccountSshKey; import com.google.gerrit.server.account.VersionedAuthorizedKeys; import com.google.gerrit.server.account.externalids.ExternalId; @@ -138,6 +139,15 @@ public class SshKeyCacheImpl implements SshKeyCache { // to do with the key object, and instead we must abort this load. // throw e; + } catch (InvalidKeyAlgorithmException e) { + logger.atWarning().withCause(e).log( + "SSH key %d of account %s has an invalid algorithm %s: fixing the algorithm to %s", + k.seq(), k.accountId(), e.getInvalidKeyAlgo(), e.getExpectedKeyAlgo()); + if (fixKeyAlgorithm(k, e.getExpectedKeyAlgo())) { + kl.add(new SshKeyCacheEntry(k.accountId(), e.getPublicKey())); + } else { + markInvalid(k); + } } catch (Throwable e) { markInvalid(k); } @@ -152,5 +162,20 @@ public class SshKeyCacheImpl implements SshKeyCache { "Failed to mark SSH key %d of account %s invalid", k.seq(), k.accountId()); } } + + private boolean fixKeyAlgorithm(AccountSshKey k, String keyAlgo) { + try { + logger.atInfo().log( + "Fixing SSH key %d of account %s algorithm to %s", k.seq(), k.accountId(), keyAlgo); + authorizedKeys.deleteKey(k.accountId(), k.seq()); + String sshKey = k.sshPublicKey(); + authorizedKeys.addKey(k.accountId(), keyAlgo + sshKey.substring(sshKey.indexOf(' '))); + return true; + } catch (IOException | ConfigInvalidException | InvalidSshKeyException e) { + logger.atSevere().withCause(e).log( + "Failed to fix SSH key %d of account %s with algo %s", k.seq(), k.accountId(), keyAlgo); + return false; + } + } } } diff --git a/java/com/google/gerrit/sshd/SshUtil.java b/java/com/google/gerrit/sshd/SshUtil.java index abbd81d83a..29d0e90f9f 100644 --- a/java/com/google/gerrit/sshd/SshUtil.java +++ b/java/com/google/gerrit/sshd/SshUtil.java @@ -57,7 +57,12 @@ public class SshUtil { throw new InvalidKeySpecException("No key string"); } final byte[] bin = BaseEncoding.base64().decode(s); - return new ByteArrayBuffer(bin).getRawPublicKey(); + String publicKeyAlgo = new ByteArrayBuffer(bin).getString(); + PublicKey publicKey = new ByteArrayBuffer(bin).getRawPublicKey(); + if (!key.algorithm().equals(publicKeyAlgo)) { + throw new InvalidKeyAlgorithmException(key.algorithm(), publicKeyAlgo, publicKey); + } + return publicKey; } catch (RuntimeException | SshException e) { throw new InvalidKeySpecException("Cannot parse key", e); } diff --git a/javatests/com/google/gerrit/sshd/SshUtilTest.java b/javatests/com/google/gerrit/sshd/SshUtilTest.java index 3225230bbd..1585bc3a97 100644 --- a/javatests/com/google/gerrit/sshd/SshUtilTest.java +++ b/javatests/com/google/gerrit/sshd/SshUtilTest.java @@ -20,7 +20,6 @@ import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.entities.Account; import com.google.gerrit.server.account.AccountSshKey; import java.security.spec.InvalidKeySpecException; -import org.junit.Ignore; import org.junit.Test; public class SshUtilTest { @@ -31,7 +30,6 @@ public class SshUtilTest { "AAAAB3NzaC1yc2EAAAABIwAAAIEA0R66EoZ7hFp81w9sAJqu34UFyE+w36H/mobUqnT5Lns7PcTOJh3sgMJAlswX2lFAWqvF2gd2PRMpMhbfEU4iq2SfY8x+RDCJ4ZQWESln/587T41BlQjOXzu3W1bqgmtHnRCte3DjyWDvM/fucnUMSwOgP+FVEZCLTrk3thLMWsU="; private static final Object VALID_SSH_RSA_ALGO = "ssh-rsa"; - @Ignore("To be enabled once the SSH key parsing is fixed") @Test public void shouldFailParsingOpenSshKeyWithInvalidAlgo() { String sshKeyWithInvalidAlgo = String.format("%s %s", INVALID_ALGO, VALID_OPENSSH_RSA_KEY); |