tomaswolf commented on a change in pull request #176:
URL: https://github.com/apache/mina-sshd/pull/176#discussion_r632393865
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/BaseBuilder.java
##########
@@ -70,6 +70,7 @@
*/
public static final List<BuiltinCiphers> DEFAULT_CIPHERS_PREFERENCE =
Collections.unmodifiableList(
Arrays.asList(
+ // BuiltinCiphers.cc20p1305_openssh, // TODO: enable by
default when BouncyCastle available
Review comment:
I may be missing something here... but this BuiltinCiphers code already
can have ciphers in there that have isSupported() == false. So I wonder why
we'd need to have a ChaCha implementation in sshd itself. Either some provider
provides it, or not.
I would expect the ChaCha cipher never to be proposed or used if it isn't
supported. Having it supported only if BC is available would be fine by me. (We
seem to require BC >= 1.68. If there's any such BC version that doesn't provide
ChaCha, then the feature test would need to be more precise.)
Don't know what the stance of the other maintainers is on having more crypto
code inside sshd. AFAIK the only crypto code in sshd is the OpenBSD bcrypt
implementation needed to handle OpenSSH encrypted keys. That one was copied
because it was felt that having another dependency on an external artifact just
for one class was to be avoided, and the inclusion was cleared beforehand here
at Apache _and_ with the Eclipse legal team. (The latter only because I wanted
it to be used in Eclipse; including it here if then Eclipse legal had objected
would have only complicated matters.)
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
