You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by GitBox <gi...@apache.org> on 2021/03/20 12:09:47 UTC
[GitHub] [mina-sshd] tomaswolf opened a new pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
tomaswolf opened a new pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184
Unconditionally announce that the client wants to get the server's
SSH_MSG_EXT_INFO. Otherwise the server will never send it. Add the
"ext-info-c" marker only on the very first key exchange proposal,
and make sure the client doesn't send by mistake "ext-info-s".
KexExtensions are available in all phases except PREKEX.
When server-sig-algs is received, reorder the client session's
signature factories such that algorithms the server announced as
supported come first, followed by those not announced, both in client
order.
The client determines the order and the server just says what it
supports.
Note that per RFC 8308 [1] it's possible that a server doesn't announce
_all_ the algorithms it supports, and a client is allowed to try
unsupported algorithms, but may face authentication penalties such
as back-off delays, authentication failures, or disconnections.
[1] https://tools.ietf.org/html/rfc8308
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] tomaswolf commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
tomaswolf commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599722159
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
I should have, soon. Finally got around to enable 2FA here on Github, accounts are linked, so now I'm waiting for the organizational invite. If I understood the process correctly, that should then also enable commit rights on master.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] tomaswolf commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
tomaswolf commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599714998
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
Done, though I'd still have preferred at least a method like `public boolean wasClientExtensionProposalMade(Session session)` instead of a public attribute. We don't have to make it easy for clients to shoot their own feet.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599703263
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
>> I know we have different ideas about interfaces. Not the first time this issue crops up :-)
Indeed :-)
>> If you really think this should be accessible, let's add an accessor method
How would this prevent misuse ?
>> If this is public, a client could reset this -- with might break rekeying? According to RFC 8308 the indicator is to be added only on the first key exchange.
Like I said - I prefer holding the users responsible for misuse rather than patronize them and "protect them from themselves"
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599753743
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
>> I had asked Guillaume a while ago about where I should commit/merge
Please ask him then how to merge the 2 PRs - in any case, if you need me to merge them let me know
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599724723
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
>> I'm waiting for the organizational invite
Note that the _master_ I am speaking about is not the _github_ one but rather the ASF one. Github is only a **mirror** of the ASF one - we never push to github...
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599717983
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
Do you have commit rights to _master_ or do you need me to merge your PRs ? If so, let me know when you think it is ready (since I have no more comments) and I will do so.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] tomaswolf commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
tomaswolf commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599733904
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
I had asked Guillaume a while ago about where I should commit/merge; he said he just pushed to the Github repo and that worked for him. There's also a number of commits at gitbox that are PR merges. So I gathered that indeed simply merging PRs the usual Github way would work.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599716781
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
>> Done, though I'd still have preferred at least a method like public boolean wasClientExtensionProposalMade(Session session) instead of a public attribute. We don't have to make it easy for clients to shoot their own feet.
Like you said - we differ in our approach/view on this matter :-))
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599703263
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
>> I know we have different ideas about interfaces. Not the first time this issue crops up :-)
Indeed :-)
>> If you really think this should be accessible, let's add an accessor method
How would this prevent misuse ?
>> If this is public, a client could reset this -- with might break rekeying? According to RFC 8308 the indicator is to be added only on the first key exchange.
Like I said - I prefer holding the users responsible for misuse rather than patronize them and "protect them from themselves"
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] tomaswolf merged pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
tomaswolf merged pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] tomaswolf commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
tomaswolf commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599700726
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
I know we have different ideas about interfaces. Not the first time this issue crops up :-) If you really think this should be accessible, let's add an accessor method.
If this is public, a client could reset this -- with might break rekeying? According to RFC 8308 the indicator is to be added only on the _first_ key exchange.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599687095
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
I don't see why this should be `private` - it is immutable and someone might find it useful. In general, I try not to "hide" anything from the users (within reason) - even if they can cause harm by misuse - since one never knows how our users might need to extend/use our 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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #184: [SSHD-1141] Fix client-side server-sig-algs handling
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #184:
URL: https://github.com/apache/mina-sshd/pull/184#discussion_r599687095
##########
File path: sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
##########
@@ -52,247 +43,111 @@
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ private static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
Review comment:
I don't see why this should be `private` - it is immutable and someone might find it useful. In general, I try not to "hide" anything from the users (within reason) - even if they can cause harm by misuse - since one never knows how our users might need to extend/our 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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org