From dfa9b06a365be78e7e2bed01935fc0e9ed3f3094 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 28 Sep 2024 23:41:48 +0200 Subject: [PATCH] GH-618: Fix reading an OpenSshCertificate from a Buffer Buffer.getBytesConsumed() is broken. It'll return wrong data for buffers created on a slice of an array not starting at zero. Using it in OpenSSHCertPublicKeyParser will return data before the raw certificate in the buffer if that raw certificate is not the first thing in the buffer. Fix this by adding a Buffer.getBytesConsumed(int from) method, and use that in OpenSSHCertPublicKeyParser. Bug: https://github.com/apache/mina-sshd/issues/618 --- CHANGES.md | 2 ++ .../org/apache/sshd/common/util/buffer/Buffer.java | 5 +++++ .../sshd/common/util/buffer/ByteArrayBuffer.java | 8 ++++++++ .../buffer/keys/OpenSSHCertPublicKeyParser.java | 6 +++++- .../certificates/OpenSSHCertificateParserTest.java | 13 +++++++++---- 5 files changed, 29 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c2133d509..1db0314b0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -40,6 +40,8 @@ ## Bug Fixes +* [GH-618](https://github.com/apache/mina-sshd/issues/618) Fix reading an `OpenSshCertificate` from a `Buffer` + ## New Features ## Potential compatibility issues diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java index f170275e2..803c3cb24 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java @@ -118,6 +118,11 @@ protected Buffer() { */ public abstract byte[] getBytesConsumed(); + /** + * @return The bytes consumed since the given position + */ + public abstract byte[] getBytesConsumed(int from); + /** * @param pos A position in the raw underlying data bytes * @return The byte at the specified position without changing the current {@link #rpos() read position}. diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java index 4a026adba..0129e6d24 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/ByteArrayBuffer.java @@ -163,6 +163,14 @@ public byte[] getBytesConsumed() { return consumed; } + @Override + public byte[] getBytesConsumed(int from) { + if (from >= rpos) { + return new byte[0]; + } + return Arrays.copyOfRange(data, from, rpos); + } + @Override public byte rawByte(int pos) { return data[pos]; diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/OpenSSHCertPublicKeyParser.java b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/OpenSSHCertPublicKeyParser.java index 7ec03a7f6..8ee2e6580 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/OpenSSHCertPublicKeyParser.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/OpenSSHCertPublicKeyParser.java @@ -54,6 +54,7 @@ public OpenSSHCertPublicKeyParser() { public OpenSshCertificate getRawPublicKey(String keyType, Buffer buffer) throws GeneralSecurityException { OpenSshCertificateImpl certificate = new OpenSshCertificateImpl(); certificate.setKeyType(keyType); + final int pos = buffer.rpos(); certificate.setNonce(buffer.getBytes()); @@ -84,7 +85,10 @@ public OpenSshCertificate getRawPublicKey(String keyType, Buffer buffer) throws throw new InvalidKeyException("Could not parse public CA key with ID: " + certificate.getId(), ex); } - certificate.setMessage(buffer.getBytesConsumed()); + Buffer tmp = new ByteArrayBuffer(); + tmp.putString(keyType); + tmp.putRawBytes(buffer.getBytesConsumed(pos)); + certificate.setMessage(tmp.getCompactData()); certificate.setSignature(buffer.getBytes()); if (buffer.rpos() != buffer.wpos()) { diff --git a/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java b/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java index bd60e38f0..a2e317d11 100644 --- a/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java @@ -32,16 +32,14 @@ import org.apache.sshd.common.config.keys.PublicKeyEntry; import org.apache.sshd.common.signature.Signature; import org.apache.sshd.common.util.GenericUtils; +import org.apache.sshd.common.util.buffer.Buffer; +import org.apache.sshd.common.util.buffer.ByteArrayBuffer; import org.apache.sshd.common.util.io.IoUtils; import org.apache.sshd.util.test.BaseTestSupport; import org.junit.jupiter.api.Tag; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - @Tag("NoIoTestCase") // see https://github.com/junit-team/junit/wiki/Parameterized-tests public class OpenSSHCertificateParserTest extends BaseTestSupport { @@ -108,6 +106,13 @@ public void parseCertificate(TestParams params) throws Exception { typedCert.getExtensions()); assertEquals(params.sigAlgorithm, typedCert.getSignatureAlgorithm()); verifySignature(typedCert); + Buffer buffer = new ByteArrayBuffer(); + buffer.putPublicKey(typedCert); + PublicKey readFromBuffer = buffer.getPublicKey(); + assertTrue(readFromBuffer instanceof OpenSshCertificate, + () -> "Expected an OpenSshCertificate but got " + readFromBuffer.getClass().getName()); + OpenSshCertificate readBack = (OpenSshCertificate) readFromBuffer; + verifySignature(readBack); } }