diff --git a/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF b/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF index 2ba94f205..caa3e700d 100644 --- a/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF @@ -72,6 +72,7 @@ Import-Package: net.i2p.crypto.eddsa;version="[0.3.0,0.4.0)", org.apache.sshd.common.signature;version="[2.8.0,2.9.0)", org.apache.sshd.common.util;version="[2.8.0,2.9.0)", org.apache.sshd.common.util.buffer;version="[2.8.0,2.9.0)", + org.apache.sshd.common.util.buffer.keys;version="[2.8.0,2.9.0)", org.apache.sshd.common.util.closeable;version="[2.8.0,2.9.0)", org.apache.sshd.common.util.io;version="[2.8.0,2.9.0)", org.apache.sshd.common.util.io.der;version="[2.8.0,2.9.0)", diff --git a/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties b/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties index b529e5b5a..4f735bab3 100644 --- a/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties +++ b/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties @@ -93,6 +93,8 @@ sshAgentPayloadLengthError=Expected {0,choice,0#no bytes|1#one byte|1<{0} bytes} sshAgentReplyLengthError=Invalid SSH agent reply message length {0} after command {1} sshAgentReplyUnexpected=Unexpected reply from ssh-agent: {0} sshAgentShortReadBuffer=Short read from SSH agent +sshAgentUnknownKey=SSH agent delivered a key that cannot be handled +sshAgentWrongKeyLength=SSH agent delivered illogical key length {0} at offset {1} in buffer of length {2} sshAgentWrongNumberOfKeys=Invalid number of SSH agent keys: {0} sshClosingDown=Apache MINA sshd session factory is closing down; cannot create new ssh sessions on this factory sshCommandTimeout={0} timed out after {1} seconds while opening the channel diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java index b8c94eea5..19ad85c83 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java @@ -114,6 +114,8 @@ public static SshdText get() { /***/ public String sshAgentReplyLengthError; /***/ public String sshAgentReplyUnexpected; /***/ public String sshAgentShortReadBuffer; + /***/ public String sshAgentUnknownKey; + /***/ public String sshAgentWrongKeyLength; /***/ public String sshAgentWrongNumberOfKeys; /***/ public String sshClosingDown; /***/ public String sshCommandTimeout; diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/agent/SshAgentClient.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/agent/SshAgentClient.java index 49b0d4ad7..cbcb4d240 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/agent/SshAgentClient.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/agent/SshAgentClient.java @@ -33,6 +33,7 @@ import org.apache.sshd.common.util.buffer.BufferException; import org.apache.sshd.common.util.buffer.BufferUtils; import org.apache.sshd.common.util.buffer.ByteArrayBuffer; +import org.apache.sshd.common.util.buffer.keys.BufferPublicKeyParser; import org.apache.sshd.common.util.io.der.DERParser; import org.eclipse.jgit.internal.transport.sshd.SshdText; import org.eclipse.jgit.transport.sshd.agent.Connector; @@ -141,14 +142,17 @@ public Iterable> getIdentities() List> keys = new ArrayList<>( numberOfKeys); for (int i = 0; i < numberOfKeys; i++) { - PublicKey key = reply.getPublicKey(); + PublicKey key = readKey(reply); String comment = reply.getString(); - if (tracing) { - LOG.trace("Got SSH agent {} key: {} {}", //$NON-NLS-1$ - KeyUtils.getKeyType(key), - KeyUtils.getFingerPrint(key), comment); + if (key != null) { + if (tracing) { + LOG.trace("Got SSH agent {} key: {} {}", //$NON-NLS-1$ + KeyUtils.getKeyType(key), + KeyUtils.getFingerPrint(key), comment); + } + keys.add(new AbstractMap.SimpleImmutableEntry<>(key, + comment)); } - keys.add(new AbstractMap.SimpleImmutableEntry<>(key, comment)); } return keys; } catch (BufferException e) { @@ -404,6 +408,48 @@ private static byte[] asn1Parse(byte[] encoded, int n) throws IOException { } } + /** + * A safe version of {@link Buffer#getPublicKey()}. Upon return the + * buffers's read position is always after the key blob; any exceptions + * thrown by trying to read the key are logged and not propagated. + *

+ * This is needed because an SSH agent might contain and deliver keys that + * we cannot handle (for instance ed448 keys). + *

+ * + * @param buffer + * to read the key from + * @return the {@link PublicKey}, or {@code null} if the key could not be + * read + * @throws BufferException + * if the length of the key blob cannot be read or is corrupted + */ + private static PublicKey readKey(Buffer buffer) throws BufferException { + int endOfBuffer = buffer.wpos(); + int keyLength = buffer.getInt(); + int afterKey = buffer.rpos() + keyLength; + if (keyLength <= 0 || afterKey > endOfBuffer) { + throw new BufferException( + MessageFormat.format(SshdText.get().sshAgentWrongKeyLength, + Integer.toString(keyLength), + Integer.toString(buffer.rpos()), + Integer.toString(endOfBuffer))); + } + // Limit subsequent reads to the public key blob + buffer.wpos(afterKey); + try { + return buffer.getRawPublicKey(BufferPublicKeyParser.DEFAULT); + } catch (Exception e) { + LOG.warn(SshdText.get().sshAgentUnknownKey, e); + return null; + } finally { + // Restore real buffer end + buffer.wpos(endOfBuffer); + // Set the read position to after this key, even if failed + buffer.rpos(afterKey); + } + } + private Buffer rpc(byte command, byte[] message) throws IOException { return new ByteArrayBuffer(connector.rpc(command, message)); }