This is an automated email from the ASF dual-hosted git repository. lgoldstein pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 44467af2e4ad40f2341c1b1e05a3c1f2f961054b Author: Lyor Goldstein <lgoldst...@apache.org> AuthorDate: Mon Oct 7 16:22:13 2019 +0300 [SSHD-945] Added sshd-contrib code that uses SHA1 with DSA regardless of its key length --- CHANGES.md | 2 + docs/extensions.md | 6 + .../sshd/common/signature/LegacyDSASigner.java | 301 +++++++++++++++++++++ .../sshd/common/signature/LegacyDSASignerTest.java | 144 ++++++++++ .../org/apache/sshd/common/signature/ssh-dss-1024 | 12 + .../org/apache/sshd/common/signature/ssh-dss-2048 | 17 ++ 6 files changed, 482 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 44229ab..0f48121 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -75,6 +75,8 @@ exchange via properties. * [SSHD-943](https://issues.apache.org/jira/browse/SSHD-943) - Provide session instance when KEX factory is invoked in order to create a KeyExchange instance. +* [SSHD-945](https://issues.apache.org/jira/browse/SSHD-945) - Added sshd-contrib code that uses SHA1 with DSA regardless of its key length + * [SSHD-947](https://issues.apache.org/jira/browse/SSHD-947) - Added configuration allowing the user to specify whether client should wait for the server's identification before sending KEX-INIT message. diff --git a/docs/extensions.md b/docs/extensions.md index 9d0b044..1f762ae 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -60,3 +60,9 @@ described in [SSHD-754](https://issues.apache.org/jira/browse/SSHD-754) and [SSH * `AndroidOpenSSLSecurityProviderRegistrar` - A security registrar that uses the [AndroidOpenSSL](https://github.com/guardianproject/openssl-android) security provider + +* `LegacyDSASigner` - A `java.security.Signature` that applies SHA-1 with DSA keys regardless of their +key length - i.e., despite FIPS186-3 section 4.2 that mandates usage of SHA-2 for keys greater than +1024 bits. This is in accordance with RFC 4253 that was never amended to specify any other digest for +such keys. The signer can be use to provide a custom implementation of `SignatureDSA` (and its factory) +that uses this signer instead of the JCE or _Bouncycastle_ one - see comments on issue [SSHD-945](https://issues.apache.org/jira/browse/SSHD-945). diff --git a/sshd-contrib/src/main/java/org/apache/sshd/common/signature/LegacyDSASigner.java b/sshd-contrib/src/main/java/org/apache/sshd/common/signature/LegacyDSASigner.java new file mode 100644 index 0000000..b8272bb --- /dev/null +++ b/sshd-contrib/src/main/java/org/apache/sshd/common/signature/LegacyDSASigner.java @@ -0,0 +1,301 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.sshd.common.signature; + +import java.io.IOException; +import java.math.BigInteger; +import java.nio.ByteBuffer; +import java.security.GeneralSecurityException; +import java.security.InvalidKeyException; +import java.security.InvalidParameterException; +import java.security.MessageDigest; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.SignatureException; +import java.security.interfaces.DSAKey; +import java.security.interfaces.DSAParams; +import java.security.interfaces.DSAPrivateKey; +import java.security.interfaces.DSAPublicKey; +import java.util.Arrays; + +import org.apache.sshd.common.Factory; +import org.apache.sshd.common.random.Random; +import org.apache.sshd.common.util.ValidateUtils; +import org.apache.sshd.common.util.io.der.DERParser; +import org.apache.sshd.common.util.io.der.DERWriter; + +/** + * A special signer for DSA that uses SHA-1 regardless of the key size + * + * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> + * @see <a href="https://issues.apache.org/jira/browse/SSHD-945">SSHD-945 issue</a> + */ +public class LegacyDSASigner extends java.security.Signature { + public static final String LEGACY_SIGNATURE = "LegacySHA1withDSA"; + + protected final MessageDigest md; + protected final Factory<Random> randomFactory; + protected BigInteger x; + protected BigInteger y; + protected DSAParams params; + + public LegacyDSASigner(Factory<Random> randomFactory) throws GeneralSecurityException { + super(LEGACY_SIGNATURE); + + this.randomFactory = randomFactory; + md = MessageDigest.getInstance("SHA-1"); + } + + @Override + @Deprecated + protected void engineSetParameter(String key, Object param) { + throw new InvalidParameterException("No parameter accepted"); + } + + @Override + @Deprecated + protected Object engineGetParameter(String key) { + return null; + } + + protected void initDSAParameters(DSAKey key) throws InvalidKeyException { + params = key.getParams(); + if (params == null) { + throw new InvalidKeyException("Missing DSA parameters in key"); + } + + md.reset(); + } + + @Override + protected void engineInitSign(PrivateKey key) throws InvalidKeyException { + if (!(key instanceof DSAPrivateKey)) { + throw new InvalidKeyException("Not a DSA private key"); + } + + DSAPrivateKey prvKey = (DSAPrivateKey) key; + initDSAParameters(prvKey); + x = prvKey.getX(); + y = null; + } + + @Override + protected byte[] engineSign() throws SignatureException { + try { + ValidateUtils.checkState(params != null, "Missing DSA parameters"); + + BigInteger q = params.getQ(); + BigInteger k = generateK(q); + + BigInteger r = generateR(params.getP(), q, params.getG(), k); + byte[] rEncoding; + try (DERWriter w = new DERWriter(SignatureDSA.MAX_SIGNATURE_VALUE_LENGTH + 4)) { // in case length > 0x7F + w.writeBigInteger(r); + rEncoding = w.toByteArray(); + } + + BigInteger s = generateS(x, q, r, k); + byte[] sEncoding; + try (DERWriter w = new DERWriter(SignatureDSA.MAX_SIGNATURE_VALUE_LENGTH + 4)) { // in case length > 0x7F + w.writeBigInteger(s); + sEncoding = w.toByteArray(); + } + + int length = rEncoding.length + sEncoding.length; + try (DERWriter w = new DERWriter(1 + length + 4)) { // in case length > 0x7F + w.write(0x30); // SEQUENCE + w.writeLength(length); + w.write(rEncoding); + w.write(sEncoding); + return w.toByteArray(); + } + } catch (RuntimeException | IOException e) { + throw new SignatureException(e.getMessage(), e); + } + } + + protected BigInteger generateK(BigInteger q) { + Random random; + if (appRandom == null) { + ValidateUtils.checkState(randomFactory != null, "No signing random factory provided"); + random = randomFactory.create(); + } else { + random = null; + } + + byte[] kValue = new byte[q.bitLength() / Byte.SIZE]; + + while (true) { + if (random == null) { + appRandom.nextBytes(kValue); + } else { + random.fill(kValue); + } + + BigInteger posK = new BigInteger(1, kValue); + BigInteger k = posK.mod(q); + if ((k.signum() > 0) && (k.compareTo(q) < 0)) { + return k; + } + } + } + + protected BigInteger generateR( + BigInteger p, BigInteger q, BigInteger g, BigInteger k) { + BigInteger temp = g.modPow(k, p); + return temp.mod(q); + } + + protected BigInteger generateS( + BigInteger x, BigInteger q, BigInteger r, BigInteger k) { + byte[] s2 = md.digest(); + + int nBytes = q.bitLength() / Byte.SIZE; + if (nBytes < s2.length) { + s2 = Arrays.copyOfRange(s2, 0, nBytes); + } + + BigInteger z = new BigInteger(1, s2); + BigInteger k1 = k.modInverse(q); + BigInteger mul1 = x.multiply(r); + BigInteger withZ = mul1.add(z); + BigInteger mul2 = withZ.multiply(k1); + return mul2.mod(q); + } + + @Override + protected void engineInitVerify(PublicKey key) throws InvalidKeyException { + if (!(key instanceof DSAPublicKey)) { + throw new InvalidKeyException("Not a DSA public key"); + } + + DSAPublicKey pubKey = (DSAPublicKey) key; + initDSAParameters(pubKey); + x = null; + y = pubKey.getY(); + } + + @Override + protected boolean engineVerify(byte[] signature) + throws SignatureException { + return engineVerify(signature, 0, signature.length); + } + + @Override + protected boolean engineVerify(byte[] signature, int offset, int length) + throws SignatureException { + ValidateUtils.checkState(params != null, "Missing DSA parameters"); + + try { + BigInteger r; + BigInteger s; + try (DERParser parser = new DERParser(signature, offset, length)) { + int type = parser.read(); + if (type != 0x30) { + throw new SignatureException( + "Invalid signature format - not a DER SEQUENCE: 0x" + Integer.toHexString(type)); + } + + // length of remaining encoding of the 2 integers + int remainLen = parser.readLength(); + /* + * There are supposed to be 2 INTEGERs, each encoded with: + * + * - one byte representing the fact that it is an INTEGER + * - one byte of the integer encoding length + * - at least one byte of integer data (zero length is not an option) + */ + if (remainLen < (2 * 3)) { + throw new SignatureException( + "Invalid signature format - not enough encoded data length: " + remainLen); + } + + r = parser.readBigInteger(); + s = parser.readBigInteger(); + } catch (IOException e) { + throw new SignatureException("Failed to parse DSA DER data", e); + } + + /* + * Some implementations do not correctly encode values in the ASN.1 + * 2's complement format - we need positive values for validation + */ + if (r.signum() < 0) { + r = new BigInteger(1, r.toByteArray()); + } + if (s.signum() < 0) { + s = new BigInteger(1, s.toByteArray()); + } + + BigInteger q = params.getQ(); + if ((r.compareTo(q) != -1) || (s.compareTo(q) != -1)) { + throw new IndexOutOfBoundsException("Out of range values in signature"); + } + + BigInteger p = params.getP(); + BigInteger g = params.getG(); + BigInteger w = generateW(p, q, g, s); + BigInteger v = generateV(y, p, q, g, w, r); + return v.equals(r); + } catch (RuntimeException e) { + throw new SignatureException(e.getMessage(), e); + } + } + + protected BigInteger generateW( + BigInteger p, BigInteger q, BigInteger g, BigInteger s) { + return s.modInverse(q); + } + + protected BigInteger generateV( + BigInteger y, BigInteger p, BigInteger q, BigInteger g, BigInteger w, BigInteger r) { + byte[] s2 = md.digest(); + int nBytes = q.bitLength() / Byte.SIZE; + if (nBytes < s2.length) { + s2 = Arrays.copyOfRange(s2, 0, nBytes); + } + + BigInteger z = new BigInteger(1, s2); + BigInteger u1 = z.multiply(w).mod(q); + BigInteger mul = r.multiply(w); + BigInteger u2 = mul.mod(q); + BigInteger t1 = g.modPow(u1, p); + BigInteger t2 = y.modPow(u2, p); + BigInteger t3 = t1.multiply(t2); + BigInteger t5 = t3.mod(p); + return t5.mod(q); + } + + @Override + protected void engineUpdate(byte b) { + md.update(b); + } + + @Override + protected void engineUpdate(byte[] data, int off, int len) { + md.update(data, off, len); + } + + @Override + protected void engineUpdate(ByteBuffer b) { + md.update(b); + } +} + diff --git a/sshd-contrib/src/test/java/org/apache/sshd/common/signature/LegacyDSASignerTest.java b/sshd-contrib/src/test/java/org/apache/sshd/common/signature/LegacyDSASignerTest.java new file mode 100644 index 0000000..fc54f85 --- /dev/null +++ b/sshd-contrib/src/test/java/org/apache/sshd/common/signature/LegacyDSASignerTest.java @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.sshd.common.signature; + +import java.io.IOException; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.KeyPair; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import org.apache.sshd.common.config.keys.KeyUtils; +import org.apache.sshd.common.config.keys.loader.pem.DSSPEMResourceKeyPairParser; +import org.apache.sshd.common.keyprovider.KeyPairProvider; +import org.apache.sshd.common.random.JceRandomFactory; +import org.apache.sshd.common.util.GenericUtils; +import org.apache.sshd.common.util.ValidateUtils; +import org.apache.sshd.common.util.security.SecurityUtils; +import org.apache.sshd.util.test.JUnit4ClassRunnerWithParametersFactory; +import org.apache.sshd.util.test.JUnitTestSupport; +import org.apache.sshd.util.test.NoIoTestCase; +import org.junit.Assume; +import org.junit.FixMethodOrder; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.MethodSorters; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import org.junit.runners.Parameterized.UseParametersRunnerFactory; + +/** + * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> + */ +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +@RunWith(Parameterized.class) // see https://github.com/junit-team/junit/wiki/Parameterized-tests +@UseParametersRunnerFactory(JUnit4ClassRunnerWithParametersFactory.class) +@Category({ NoIoTestCase.class }) +public class LegacyDSASignerTest extends JUnitTestSupport { + private final int keySize; + private final KeyPair kp; + + public LegacyDSASignerTest(int keySize) + throws IOException, GeneralSecurityException { + this.keySize = keySize; + + String resourceName = KeyPairProvider.SSH_DSS + "-" + keySize; + URL url = getClass().getResource(resourceName); + assertNotNull("Missing test key file " + resourceName, url); + + Collection<KeyPair> keys = + DSSPEMResourceKeyPairParser.INSTANCE.loadKeyPairs(null, url, null); + ValidateUtils.checkNotNullAndNotEmpty(keys, "No keys loaded from %s", resourceName); + kp = GenericUtils.head(keys); + + int l = KeyUtils.getKeySize(kp.getPublic()); + assertEquals("Mismatched read public key size", keySize, l); + + l = KeyUtils.getKeySize(kp.getPrivate()); + assertEquals("mismatched read private key size", keySize, l); + } + + @Parameters(name = "key-size={0}") + public static List<Object[]> parameters() { + return parameterize(Arrays.asList(1024, 2048)); + } + + @Test + public void testReflexiveSigning() throws GeneralSecurityException { + java.security.Signature signer = new LegacyDSASigner(JceRandomFactory.INSTANCE); + signer.initSign(kp.getPrivate()); + + byte[] data = (getClass().getName() + "#" + getCurrentTestName()) + .getBytes(StandardCharsets.UTF_8); + signer.update(data); + byte[] signature = signer.sign(); + + signer.initVerify(kp.getPublic()); + signer.update(data, 0, data.length); + assertTrue(signer.verify(signature)); + } + + @Test + public void testBuiltinVerifier() throws GeneralSecurityException { + Assume.assumeTrue("Skip SHA-1 with too large a key", keySize <= 1024); + + java.security.Signature signer = new LegacyDSASigner(JceRandomFactory.INSTANCE); + signer.initSign(kp.getPrivate()); + + byte[] data = (getClass().getName() + "#" + getCurrentTestName()) + .getBytes(StandardCharsets.UTF_8); + signer.update(data); + byte[] signature = signer.sign(); + + java.security.Signature verifier = + SecurityUtils.getSignature(SignatureDSA.DEFAULT_ALGORITHM); + verifier.initVerify(kp.getPublic()); + verifier.update(data); + assertTrue(verifier.verify(signature)); + } + + @Test + public void testBuiltinSigner() throws GeneralSecurityException { + Assume.assumeTrue("Skip SHA-1 with too large a key", keySize <= 1024); + + java.security.Signature signer = + SecurityUtils.getSignature(SignatureDSA.DEFAULT_ALGORITHM); + signer.initSign(kp.getPrivate()); + + byte[] data = (getClass().getName() + "#" + getCurrentTestName()) + .getBytes(StandardCharsets.UTF_8); + signer.update(data); + byte[] signature = signer.sign(); + + java.security.Signature verifier = new LegacyDSASigner(JceRandomFactory.INSTANCE); + verifier.initVerify(kp.getPublic()); + verifier.update(data); + assertTrue(verifier.verify(signature)); + } + + @Override + public String toString() { + return getClass().getSimpleName() + "[keySize=" + keySize + "]"; + } +} diff --git a/sshd-contrib/src/test/resources/org/apache/sshd/common/signature/ssh-dss-1024 b/sshd-contrib/src/test/resources/org/apache/sshd/common/signature/ssh-dss-1024 new file mode 100644 index 0000000..1d3ef24 --- /dev/null +++ b/sshd-contrib/src/test/resources/org/apache/sshd/common/signature/ssh-dss-1024 @@ -0,0 +1,12 @@ +-----BEGIN DSA PRIVATE KEY----- +MIIBvAIBAAKBgQDIPyMbBuQcZxeYDOyCqqkdK37cWQvp+RpWzvieB/oiG/ykfDQX +oZMRtwqwWTBfejNitbBBmC6G/t5OK+9aFmj7pfJ+a7fZKXfiUquIg9soDsoOindf +2AwR6MZ3os8uiP2xrC8IQAClnETa15mFShs4a4b2VjddgCQ6tphnY97MywIVAPtr +YyW11RIXsVTf/9KlbhYaNlt5AoGAX9JzbHykC/0xDKOyKU6xDIOVdEZ0ooAl9Psl +BEUuNhlv2XgmQScO6C9l2W7gbbut7zIw4FaZ2/dgXa3D4IyexBVug5XMnrssErZo +NcoF5g0dgEAsb9Hl9gkIK3VHM5kWteeUg1VE700JTtSMisdL8CgIdR+xN8iVH5Ew +CbLWxmECgYEAtv+cdRfNevYFkp55jVqazc8zRLvfb64jzgc5oSJVc64kFs4yx+ab +YpGX9WxNxDlG6g2WiY8voDBB0YnUJsn0kVRjBKX9OceROxrfT4K4dVbQZsdt+SLa +XWL4lGJFrFZL3LZqvySvq6xfhJfakQDDivW4hUOhFPXPHrE5/Ia3T7ACFQCE6flG +nmVCAbzo9YsbdJWBnxMnBA== +-----END DSA PRIVATE KEY----- \ No newline at end of file diff --git a/sshd-contrib/src/test/resources/org/apache/sshd/common/signature/ssh-dss-2048 b/sshd-contrib/src/test/resources/org/apache/sshd/common/signature/ssh-dss-2048 new file mode 100644 index 0000000..7ffb019 --- /dev/null +++ b/sshd-contrib/src/test/resources/org/apache/sshd/common/signature/ssh-dss-2048 @@ -0,0 +1,17 @@ +-----BEGIN DSA PRIVATE KEY----- +MIIDPwIBAAKCAQEA1ICHqL15SfQucYj2MQaiVQ6ysrYQdU4uJnhGVg63aMBQeET7Wt7gytsg9G8x +XEF66Awz33WusQKHNLRouawgn/4FjAzlOx17Hta+iT1VT1a/h1lWI7gH/kVpFkkpObpJtmdoAxeg +RrOIZ+46uFfVc9H8NVp9AtzLSccUIPe/PLJDxMsvhI8+T7CCXApyMbwDglrLp/5N8i0uSMQTIeLc +xdTMFbVd4nfb+QXOWLKpwbNln+Wjt5Z3llBLej+aQFXlF3r8qnSkah5UDHQY4bXyYhLEU4PZDuwz +MxkupYAvpNhXk40xe8zfYOd6+k6zUoBkPehIf6VHt+SSG6MwOowBCQIVAO0sDkcISNFnmchCzlyV +utRGd0pTAoIBAQCTuQ+r/gFK3GS4cR1hDN04hbQs+fngmmlvGKhc30lg0hL1blw7T4MrfWuYF9va +OQs5Zv04mOQixLxCC7J/t5EZUl5aXciJGyMC3qsYgAop7W3ktyqweenTL17X8xxeomK1TfuUSQRe +sr/uBi/ke8VMfZowh+YfMYOo6AeVf6dUQUNDhF/QGYvEW7Tm2WRIcmgITuCBSFJkU3fFmTaYylCT +9hfgBZOqIzALY2b8NrGtFccDKClQOCADUcmNRo5InpzeXBCNtW9O6J7UrEJxmQ5nlWXEEqaHRsYi +Ck/Hn25DyPuDoapPC77y8TXYrIiKIC9FaDH1DyGwvObTNspadBNSAoIBAFerYjYafy9LvQULc2We +m56Kssbhwc0kUPjrz+D4y+lzY+VDxoD5scXyeXXutNoZM8kK/paW95SqXJybmAcQg23PZ2EUSkMb +WAveNt1CM2J1cikVY3sJVmMlu0AGV+AM1JB3hrSzuanBY5DKqwSi/p30kIwVUTRlDjzOVtlg/7aW +z2fSya3JTukXagQacMeq9vDCQqTyJu2IgpocHNuKzmCUducjVhfhAx1vkz7JmLd/omDCAQ6ntN3o +RTMB8uMxpzBe/Obb9pJEO2P/qpbzpygMTjPP6ITQrpmk6ec56W+9hAPnS9qzgf3NrTgZJIoLEdwM +XymiNsgh3jKvHj7HZQYCFQCi5oxBB3/a/3QfzxOXWpHQ4PE5gA== +-----END DSA PRIVATE KEY-----