tomaswolf commented on pull request #119: URL: https://github.com/apache/mina-sshd/pull/119#issuecomment-836265703
This is definitely going in the right direction, but will need more work before it could be incorporated into the main code. As for testing: I think testing against a real OpenSSH server would be great. Testing only against an Apache MINA sshd server is better than nothing, but it's inbreeding and actually only shows that both our client and server side can talk to each other. It doesn't show that either implements the SSH protocol correctly; they might both implement their own completely different protocol. But I don't know if the containers used for the CI builds on Travis do actually have OpenSSH, nor do I know what the best way to configure, create and tear down an OpenSSH server during tests would be. We do run tests on Windows; I suspect there at least we might not have an OpenSSH server available. Perhaps @lgoldstein has an idea... Some detail comments on your code: (can't comment directly on the preview diff) - Comparing -cert keys should probably also compare the signatures. - There really needs to be some mapping from the -cert signature algorithm names to the names of the underlying real signature. But I'd try to avoid simply stripping off the [email protected] suffix. A explicit mapping table might be clearer. - OpenSSH has some restrictions on which signature algorithms it accepts for which -cert key; see [ssh-rsa.c, line 274](https://github.com/openssh/openssh-portable/blob/48f54b9/ssh-rsa.c#L270). In general it must be the same as used for the certificate, except for [email protected], where it may be any of ssh-rsa, rsa-sha2-256, or rsa-sha2-512. But for a rsa-sha2-256-cert-v01 key, for instance, it _must_ be rsa-sha2-256. I suspect the determination of eligible algorithms in [UserAuthPublicKey.sendAuthDataRquest()](https://github.com/apache/mina-sshd/blob/master/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java#L165) might also need to be changed. I suspect with the current "alias"-based way, we'd try for an rsa-sha2-256-cert-v01 key first rsa-sha2-512, get a failure, and only then re-try with rsa-sha2-256. -- 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]
