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";>&quot;rsa-sha2-256/512&quot;</A> 
signature
  * factories (if not already added).
  *
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
  * @author <a href="mailto:[email protected]";>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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to