You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2020/05/22 13:00:37 UTC

[mina-sshd] 01/05: Fixed some code format and definition issues

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 cea78ef4b8cc35dedfddaa2924e8eeafe70e86ba
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Fri May 1 10:54:57 2020 +0300

    Fixed some code format and definition issues
---
 README.md                                          |  2 +
 .../keys/impl/OpenSSHCertificateDecoder.java       |  6 +-
 .../loader/pem/DSSPEMResourceKeyPairParser.java    |  8 +--
 .../loader/pem/ECDSAPEMResourceKeyPairParser.java  | 72 ++++++++++++++++++----
 .../keys/loader/pem/PEMResourceParserUtils.java    |  4 ++
 .../loader/pem/RSAPEMResourceKeyPairParser.java    |  8 +--
 .../signature/AbstractSecurityKeySignature.java    |  6 +-
 .../org/apache/sshd/common/util/GenericUtils.java  |  7 ++-
 8 files changed, 80 insertions(+), 33 deletions(-)

diff --git a/README.md b/README.md
index 2a67a34..e3dc9ed 100644
--- a/README.md
+++ b/README.md
@@ -19,8 +19,10 @@ based applications requiring SSH support.
 * [RFC 4345 - Improved Arcfour Modes for the Secure Shell (SSH) Transport Layer Protocol](https://tools.ietf.org/html/rfc4345)
 * [RFC 4419 - Diffie-Hellman Group Exchange for the Secure Shell (SSH) Transport Layer Protocol](https://tools.ietf.org/html/rfc4419)
 * [RFC 4716 - The Secure Shell (SSH) Public Key File Format](https://tools.ietf.org/html/rfc4716)
+* [RFC 5208 - Public-Key Cryptography Standards (PKCS) #8 - version 1.2](https://tools.ietf.org/html/rfc5208)
 * [RFC 5480 - Elliptic Curve Cryptography Subject Public Key Information](https://tools.ietf.org/html/rfc5480)
 * [RFC 5656 - Elliptic Curve Algorithm Integration in the Secure Shell Transport Layer](https://tools.ietf.org/html/rfc5656)
+* [RFC 5915 - Elliptic Curve Private Key Structure](https://tools.ietf.org/html/rfc5915)
 * [RFC 6668 - SHA-2 Data Integrity Verification for the Secure Shell (SSH) Transport Layer Protocol](https://tools.ietf.org/html/rfc6668)
 * [RFC 8160 - IUTF8 Terminal Mode in Secure Shell (SSH)](https://tools.ietf.org/html/rfc8160)
 * [RFC 8268 - More Modular Exponentiation (MODP) Diffie-Hellman (DH) Key Exchange (KEX) Groups for Secure Shell (SSH)](https://tools.ietf.org/html/rfc8268)
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/OpenSSHCertificateDecoder.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/OpenSSHCertificateDecoder.java
index f73c64f..d8d947f 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/OpenSSHCertificateDecoder.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/OpenSSHCertificateDecoder.java
@@ -63,13 +63,9 @@ public class OpenSSHCertificateDecoder extends AbstractPublicKeyEntryDecoder<Ope
     public OpenSshCertificate decodePublicKey(
             SessionContext session, String keyType, InputStream keyData, Map<String, String> headers)
             throws IOException, GeneralSecurityException {
-
         byte[] bytes = IoUtils.toByteArray(keyData);
-
         ByteArrayBuffer buffer = new ByteArrayBuffer(bytes);
-
-        OpenSshCertificate cert = (OpenSshCertificate) OpenSSHCertPublicKeyParser.INSTANCE.getRawPublicKey(keyType, buffer);
-
+        OpenSshCertificate cert = OpenSSHCertPublicKeyParser.INSTANCE.getRawPublicKey(keyType, buffer);
         if (cert.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) {
             throw new GeneralSecurityException("The provided certificate is not a Host certificate.");
         }
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/DSSPEMResourceKeyPairParser.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/DSSPEMResourceKeyPairParser.java
index f1b6b77..ca4f04a 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/DSSPEMResourceKeyPairParser.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/DSSPEMResourceKeyPairParser.java
@@ -47,6 +47,7 @@ import org.apache.sshd.common.util.security.SecurityUtils;
 
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ * @see <a href="https://tools.ietf.org/html/rfc3279#section-2.3.2">RFC-3279 section 2.3.2</a>
  */
 public class DSSPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParser {
     // Not exactly according to standard but good enough
@@ -56,9 +57,6 @@ public class DSSPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParse
     public static final String END_MARKER = "END DSA PRIVATE KEY";
     public static final List<String> ENDERS = Collections.unmodifiableList(Collections.singletonList(END_MARKER));
 
-    /**
-     * @see <A HREF="https://tools.ietf.org/html/rfc3279#section-2.3.2">RFC-3279 section 2.3.2</A>
-     */
     public static final String DSS_OID = "1.2.840.10040.4.1";
 
     public static final DSSPEMResourceKeyPairParser INSTANCE = new DSSPEMResourceKeyPairParser();
@@ -82,7 +80,7 @@ public class DSSPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParse
      * <p>
      * The ASN.1 syntax for the private key:
      * </P>
-     * 
+     *
      * <pre>
      * <code>
      * DSAPrivateKey ::= SEQUENCE {
@@ -95,7 +93,7 @@ public class DSSPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParse
      * }
      * </code>
      * </pre>
-     * 
+     *
      * @param  kf                       The {@link KeyFactory} To use to generate the keys
      * @param  s                        The {@link InputStream} containing the encoded bytes
      * @param  okToClose                <code>true</code> if the method may close the input stream regardless of success
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/ECDSAPEMResourceKeyPairParser.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/ECDSAPEMResourceKeyPairParser.java
index 907efba..12b74f9 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/ECDSAPEMResourceKeyPairParser.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/ECDSAPEMResourceKeyPairParser.java
@@ -51,6 +51,7 @@ import org.apache.sshd.common.util.security.SecurityUtils;
 
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ * @see <a href="https://tools.ietf.org/html/rfc5915">RFC 5915</a>
  */
 public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParser {
     public static final String BEGIN_MARKER = "BEGIN EC PRIVATE KEY";
@@ -91,10 +92,10 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
 
     /**
      * <P>
-     * ASN.1 syntax according to rfc5915 is:
+     * ASN.1 syntax according to <A HREF="https://tools.ietf.org/html/rfc5915">RFC 5915</A> is:
      * </P>
      * </BR>
-     * 
+     *
      * <PRE>
      * <CODE>
      * ECPrivateKey ::= SEQUENCE {
@@ -109,7 +110,7 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
      * <I>ECParameters</I> syntax according to RFC5480:
      * </P>
      * </BR>
-     * 
+     *
      * <PRE>
      * <CODE>
      * ECParameters ::= CHOICE {
@@ -119,7 +120,7 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
      * }
      * </CODE>
      * </PRE>
-     * 
+     *
      * @param  inputStream The {@link InputStream} containing the DER encoded data
      * @param  okToClose   {@code true} if OK to close the DER stream once parsing complete
      * @return             The decoded {@link SimpleImmutableEntry} of {@link ECPublicKeySpec} and
@@ -134,11 +135,16 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
             sequence = parser.readObject();
         }
 
-        if (!ASN1Type.SEQUENCE.equals(sequence.getObjType())) {
-            throw new IOException("Invalid DER: not a sequence: " + sequence.getObjType());
+        return decodeECPrivateKeySpec(sequence);
+    }
+
+    public static SimpleImmutableEntry<ECPublicKeySpec, ECPrivateKeySpec> decodeECPrivateKeySpec(ASN1Object sequence)
+            throws IOException {
+        ASN1Type objType = (sequence == null) ? null : sequence.getObjType();
+        if (!ASN1Type.SEQUENCE.equals(objType)) {
+            throw new IOException("Invalid DER: not a sequence: " + objType);
         }
 
-        // Parse inside the sequence
         try (DERParser parser = sequence.createParser()) {
             ECPrivateKeySpec prvSpec = decodeECPrivateKeySpec(parser);
             ECCurves curve = ECCurves.fromCurveParameters(prvSpec.getParams());
@@ -146,20 +152,45 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
                 throw new StreamCorruptedException("Unknown curve");
             }
 
+            /*
+             * According to https://tools.ietf.org/html/rfc5915 - section 3
+             *
+             * ECPrivateKey ::= SEQUENCE {
+             *      version        INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
+             *      privateKey     OCTET STRING,
+             *      parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
+             *      publicKey  [1] BIT STRING OPTIONAL
+             * }
+             */
             ECPoint w = decodeECPublicKeyValue(curve, parser);
             ECPublicKeySpec pubSpec = new ECPublicKeySpec(w, prvSpec.getParams());
             return new SimpleImmutableEntry<>(pubSpec, prvSpec);
         }
     }
 
+    /*
+     * According to https://tools.ietf.org/html/rfc5915 - section 3
+     *
+     * ECPrivateKey ::= SEQUENCE {
+     *      version        INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
+     *      privateKey     OCTET STRING,
+     *      parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
+     *      publicKey  [1] BIT STRING OPTIONAL
+     * }
+     */
     public static final ECPrivateKeySpec decodeECPrivateKeySpec(DERParser parser) throws IOException {
         // see openssl asn1parse -inform PEM -in ...file... -dump
-        ASN1Object versionObject = parser.readObject(); // Skip version
+        ASN1Object versionObject = parser.readObject();
         if (versionObject == null) {
             throw new StreamCorruptedException("No version");
         }
 
-        // as per RFC-5915 section 3
+        /*
+         * According to https://tools.ietf.org/html/rfc5915 - section 3
+         *
+         *      For this version of the document, it SHALL be set to ecPrivkeyVer1,
+         *      which is of type INTEGER and whose value is one (1)
+         */
         BigInteger version = versionObject.asInteger();
         if (!BigInteger.ONE.equals(version)) {
             throw new StreamCorruptedException("Bad version value: " + version);
@@ -175,6 +206,18 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
             throw new StreamCorruptedException("Non-matching private key object type: " + objType);
         }
 
+        /*
+         * According to https://tools.ietf.org/html/rfc5915 - section 3
+         *
+         *      parameters specifies the elliptic curve domain parameters
+         *      associated to the private key.  The type ECParameters is discussed
+         *      in [RFC5480].  As specified in [RFC5480], only the namedCurve
+         *      CHOICE is permitted.  namedCurve is an object identifier that
+         *      fully identifies the required values for a particular set of
+         *      elliptic curve domain parameters.  Though the ASN.1 indicates that
+         *      the parameters field is OPTIONAL, implementations that conform to
+         *      this document MUST always include the parameters field.
+         */
         ASN1Object paramsObject = parser.readObject();
         if (paramsObject == null) {
             throw new StreamCorruptedException("No parameters value");
@@ -206,13 +249,13 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
      * ASN.1 syntax according to rfc5915 is:
      * </P>
      * </BR>
-     * 
+     *
      * <pre>
      * <code>
      *      publicKey  [1] BIT STRING OPTIONAL
      * </code>
      * </pre>
-     * 
+     *
      * @param  curve       The {@link ECCurves} curve
      * @param  parser      The {@link DERParser} assumed to be positioned at the start of the data
      * @return             The encoded {@link ECPoint}
@@ -225,6 +268,13 @@ public class ECDSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairPar
             throw new StreamCorruptedException("No public key data bytes");
         }
 
+        /*
+         * According to https://tools.ietf.org/html/rfc5915
+         *
+         *      Though the ASN.1 indicates publicKey is OPTIONAL,
+         *      implementations that conform to this document SHOULD
+         *      always include the publicKey field
+         */
         try (DERParser dataParser = dataObject.createParser()) {
             ASN1Object pointData = dataParser.readObject();
             if (pointData == null) {
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/PEMResourceParserUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/PEMResourceParserUtils.java
index 3b28890..df7a72f 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/PEMResourceParserUtils.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/PEMResourceParserUtils.java
@@ -91,6 +91,10 @@ public final class PEMResourceParserUtils {
         }
     }
 
+    public static KeyPairPEMResourceParser getPEMResourceParserByOidValues(Collection<? extends Number> oid) {
+        return getPEMResourceParserByOid(GenericUtils.join(oid, '.'));
+    }
+
     public static KeyPairPEMResourceParser getPEMResourceParserByOid(String oid) {
         if (GenericUtils.isEmpty(oid)) {
             return null;
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/RSAPEMResourceKeyPairParser.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/RSAPEMResourceKeyPairParser.java
index f091dba..d599afe 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/RSAPEMResourceKeyPairParser.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/RSAPEMResourceKeyPairParser.java
@@ -48,6 +48,7 @@ import org.apache.sshd.common.util.security.SecurityUtils;
 
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ * @see <a href="https://tools.ietf.org/html/rfc3279#section-2.3.1">RFC-3279 section 2.3.1</a>
  */
 public class RSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParser {
     // Not exactly according to standard but good enough
@@ -57,9 +58,6 @@ public class RSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParse
     public static final String END_MARKER = "END RSA PRIVATE KEY";
     public static final List<String> ENDERS = Collections.unmodifiableList(Collections.singletonList(END_MARKER));
 
-    /**
-     * @see <A HREF="https://tools.ietf.org/html/rfc3279#section-2.3.1">RFC-3279 section 2.3.1</A>
-     */
     public static final String RSA_OID = "1.2.840.113549.1.1.1";
 
     public static final RSAPEMResourceKeyPairParser INSTANCE = new RSAPEMResourceKeyPairParser();
@@ -83,7 +81,7 @@ public class RSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParse
      * <p>
      * The ASN.1 syntax for the private key as per RFC-3447 section A.1.2:
      * </P>
-     * 
+     *
      * <pre>
      * <code>
      * RSAPrivateKey ::= SEQUENCE {
@@ -100,7 +98,7 @@ public class RSAPEMResourceKeyPairParser extends AbstractPEMResourceKeyPairParse
      * }
      * </code>
      * </pre>
-     * 
+     *
      * @param  kf                       The {@link KeyFactory} To use to generate the keys
      * @param  s                        The {@link InputStream} containing the encoded bytes
      * @param  okToClose                <code>true</code> if the method may close the input stream regardless of success
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java b/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java
index 5f50c1d..8745656 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java
@@ -30,12 +30,10 @@ import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
 import org.apache.sshd.common.util.security.SecurityUtils;
 
 public abstract class AbstractSecurityKeySignature implements Signature {
-
     private static final int FLAG_USER_PRESENCE = 0x01;
 
     private final String keyType;
-
-    private SecurityKeyPublicKey publicKey;
+    private SecurityKeyPublicKey<?> publicKey;
     private MessageDigest challengeDigest;
 
     protected AbstractSecurityKeySignature(String keyType) {
@@ -47,7 +45,7 @@ public abstract class AbstractSecurityKeySignature implements Signature {
         if (!(key instanceof SecurityKeyPublicKey)) {
             throw new IllegalArgumentException("Only instances of SecurityKeyPublicKey can be used");
         }
-        this.publicKey = (SecurityKeyPublicKey) key;
+        this.publicKey = (SecurityKeyPublicKey<?>) key;
         this.challengeDigest = SecurityUtils.getMessageDigest("SHA-256");
     }
 
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/GenericUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/GenericUtils.java
index e5da0d5..4ed0514 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/GenericUtils.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/GenericUtils.java
@@ -873,7 +873,7 @@ public final class GenericUtils {
 
     /**
      * Wraps a value into a {@link Supplier}
-     * 
+     *
      * @param  <T>   Type of value being supplied
      * @param  value The value to be supplied
      * @return       The supplier wrapper
@@ -1014,11 +1014,12 @@ public final class GenericUtils {
 
     /**
      * The delegate Suppliers get() method is called exactly once and the result is cached.
-     * 
+     *
+     * @param <T> Generic type of supplied value
      * @param  delegate The actual Supplier
      * @return          The memoized Supplier
      */
-    public static <T> Supplier<T> memoizeLock(Supplier<T> delegate) {
+    public static <T> Supplier<T> memoizeLock(Supplier<? extends T> delegate) {
         AtomicReference<T> value = new AtomicReference<>();
         return () -> {
             T val = value.get();