tomaswolf commented on PR #449:
URL: https://github.com/apache/mina-sshd/pull/449#issuecomment-1877851047
> Looks OK to me (have not gone over the tests in details - I trust you
checked them). I do feel I need to insist on a `CoreModuleProperties` property
that controls this feature. As I have said previously, I am fine with its
default being "enabled", but I strongly feel we should have it...
I think we should minimize "kill switches". They obfuscate the issue
permanently (removing the property in the future would be an API-breaking
change). In this particular case:
- This is a fairly important modification of a core sub-protocol in the SSH
protocols.
- It is backwards compatible with older implementations that don't know
about it.
- It is tested.
- If we did it wrong and people can't use it because it breaks something,
they'll just stay on 2.11.0 and we'll have to have a 2.12.1 with a fix.
Finally, if someone really wants to shut off strict KEX usage, here's one
way of doing so:
```
SshClient client = ...;
SessionFactory factory = new NoStrictKexSessionFactory(client);
client.setSessionFactory(factory);
client.start();
```
and
```
class NoStrictKexSessionFactory extends SessionFactory {
NoStrictKexSessionFactory(ClientFactoryManager client) {
super(client);
}
@Override
protected ClientSessionImpl doCreateSession(IoSession ioSession) throws
Exception {
return new NoStrictKexSession(getClient(), ioSession);
}
}
class NoStrictKexSession extends ClientSessionImpl {
NoStrictKexSession(ClientFactoryManager client, IoSession ioSession)
throws Exception {
super(client, ioSession);
}
@Override
protected Map<KexProposalOption, String>
doStrictKexProposal(Map<KexProposalOption, String> proposal) {
return proposal;
}
}
```
(Or likewise for a `ServerSession`.)
Yes, it's a little bit of code to write, but it's IMO about the right level
of difficulty for switching off a security feature. I don't want to make it too
easy for users to shoot themselves in the foot by inadvertently switching
strict kex off. And yes, a boolean property in `CoreModuleProperties` fits my
definition of "too easy" in this case.
--
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.
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
