mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (Jira)" <j...@apache.org>
Subject [jira] [Work logged] (SSHD-1017) Add support for chacha20-poly1305@openssh.com
Date Sun, 25 Oct 2020 16:37:00 GMT

     [ https://issues.apache.org/jira/browse/SSHD-1017?focusedWorklogId=504577&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-504577
]

ASF GitHub Bot logged work on SSHD-1017:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 25/Oct/20 16:36
            Start Date: 25/Oct/20 16:36
    Worklog Time Spent: 10m 
      Work Description: lgoldstein commented on a change in pull request #176:
URL: https://github.com/apache/mina-sshd/pull/176#discussion_r511619354



##########
File path: sshd-common/src/test/java/org/apache/sshd/common/cipher/ChaChaCipherTest.java
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.cipher;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.sshd.common.util.buffer.BufferUtils;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class ChaChaCipherTest {
+    public ChaChaCipherTest() {
+        // empty
+    }

Review comment:
       Please use `extends JUnitTestSupport` and remove `import static`

##########
File path: sshd-common/src/main/java/org/apache/sshd/common/cipher/ChaChaCipher.java
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.cipher;
+
+import java.util.Arrays;
+
+import javax.crypto.AEADBadTagException;
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.sshd.common.mac.BaseMac;
+import org.apache.sshd.common.mac.Mac;
+import org.apache.sshd.common.util.buffer.BufferUtils;
+import org.apache.sshd.common.util.security.SecurityUtils;
+
+/**
+ * AEAD cipher based on the
+ * <a href="https://github.com/openbsd/src/blob/master/usr.bin/ssh/PROTOCOL.chacha20poly1305">OpenSSH
ChaCha20-Poly1305</a>
+ * cipher extension.
+ */
+public class ChaChaCipher extends BaseCipher {
+    protected Mode mode;
+    protected SecretKey bodyKey;
+    protected SecretKey headerKey;
+    protected Cipher bodyCipher;
+    protected Cipher headerCipher;
+    protected boolean initialized;
+    protected PacketSequenceNumberParameterSpec parameters;
+
+    protected final Mac poly1305 = new BaseMac("Poly1305", 16, 32, true);
+    protected final byte[] macKey = new byte[32];
+
+    public ChaChaCipher() {
+        super(8, 16, 64, "ChaCha", 256, "ChaCha", 8);
+    }
+
+    @Override
+    protected Cipher createCipherInstance(Mode mode, byte[] key, byte[] iv) throws Exception
{
+        this.mode = mode;

Review comment:
       Recommend adding `this.mode = Objects.requireNotNull(mode, "No cipher mode provided
   );`

##########
File path: sshd-common/src/test/java/org/apache/sshd/common/cipher/ChaChaCipherTest.java
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.cipher;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.sshd.common.util.buffer.BufferUtils;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class ChaChaCipherTest {
+    public ChaChaCipherTest() {
+        // empty

Review comment:
       Please call `super()` explicitly

##########
File path: sshd-common/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
##########
@@ -102,6 +104,31 @@ public Cipher create() {
      */
     @Deprecated
     blowfishcbc(Constants.BLOWFISH_CBC, 8, 0, 16, "Blowfish", 128, "Blowfish/CBC/NoPadding",
8),
+    cc20p1305_openssh(Constants.CC20P1305_OPENSSH, 8, 16, 64, "ChaCha", 256, "ChaCha", 8)
{
+        private volatile Boolean supported;
+
+        @Override
+        public boolean isSupported() {
+            if (supported == null) {
+                synchronized (this) {
+                    if (supported == null) {
+                        try {
+                            SecurityUtils.getCipher("ChaCha");
+                            supported = true;
+                        } catch (GeneralSecurityException ignored) {
+                            supported = false;
+                        }
+                    }
+                }
+            }
+            return supported;
+        }

Review comment:
       If this works only with BC then you can replace/add `SecurityUtils.isBouncyCastleRegistered()`
call here

##########
File path: sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
##########
@@ -1757,14 +1764,18 @@ protected void receiveNewKeys() throws Exception {
                     this, inCipher, outCipher, recommendedByteRekeyBlocks, maxRekeyBlocks);
         }
 
+        resetCounters();
+        lastKeyTimeValue.set(Instant.now());
+        firstKexPacketFollows = null;
+    }
+
+    protected void resetCounters() {
         inBytesCount.set(0L);
         outBytesCount.set(0L);
         inPacketsCount.set(0L);
         outPacketsCount.set(0L);
         inBlocksCount.set(0L);
         outBlocksCount.set(0L);
-        lastKeyTimeValue.set(Instant.now());
-        firstKexPacketFollows = null;

Review comment:
       Why were these deleted ? They are used in the code...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 504577)
    Time Spent: 20m  (was: 10m)

> Add support for chacha20-poly1305@openssh.com
> ---------------------------------------------
>
>                 Key: SSHD-1017
>                 URL: https://issues.apache.org/jira/browse/SSHD-1017
>             Project: MINA SSHD
>          Issue Type: New Feature
>            Reporter: Matt Sicker
>            Priority: Major
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> See [protocol details|https://github.com/openbsd/src/blob/master/usr.bin/ssh/PROTOCOL.chacha20poly1305].
> * [RFC 7539|https://tools.ietf.org/html/rfc7539] describes the ChaCha20-Poly1305 algorithm.
> * [Dropbear implementation|https://github.com/mkj/dropbear/blob/master/chachapoly.c]
> * [OpenSSH implementation|https://github.com/openbsd/src/blob/master/usr.bin/ssh/cipher-chachapoly-libcrypto.c]
> The cipher is provided by Bouncycastle.
> As a bonus, this could potentially be adapted to propose an equivalent AES/GCM cipher
encoding to how OpenSSH implements this ChaCha20-Poly1305 cipher.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org


Mime
View raw message