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/24 19:38:38 UTC
[GitHub] [mina-sshd] tomaswolf opened a new pull request #186: [SSHD-1141] Pubkey auth: handle failure with known signature algorithm
tomaswolf opened a new pull request #186:
URL: https://github.com/apache/mina-sshd/pull/186
If the server had announced via server-sig-algs which signature
algorithms it supports, UserAuthPubKey tries the known algorithms
first. If authentication with such a known signature algorithm fails,
there is no point trying other algorithms.
Trying different signature algorithms may make sense only if the client
doesn't know which algorithms the server supports.
--
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 pull request #186: [SSHD-1141] Pubkey auth: handle failure with known signature algorithm
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on pull request #186:
URL: https://github.com/apache/mina-sshd/pull/186#issuecomment-812111007
Looks fine to me
--
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 #186: [SSHD-1141] Pubkey auth: handle failure with known signature algorithm
Posted by GitBox <gi...@apache.org>.
tomaswolf commented on a change in pull request #186:
URL: https://github.com/apache/mina-sshd/pull/186#discussion_r605879641
##########
File path: sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java
##########
@@ -105,6 +106,19 @@ protected boolean sendAuthDataRequest(ClientSession session, String service) thr
chosenAlgorithm = null;
} else if (!currentAlgorithms.isEmpty()) {
currentAlgorithm = currentAlgorithms.poll();
+ if (chosenAlgorithm != null) {
+ Set<String> knownServerAlgorithms = session.getAttribute(
+ DefaultClientKexExtensionHandler.SERVER_ALGORITHMS);
+ if (knownServerAlgorithms != null
+ && knownServerAlgorithms.contains(chosenAlgorithm)) {
+ // We've tried key 'current' with 'chosenAlgorithm', but it
+ // failed. However, the server had told us it supported
+ // 'chosenAlgorithm'. Thus it makes no sense to continue
+ // with this key and other signature algorithms. Skip to the
+ // next key, if any.
+ currentAlgorithm = null;
Review comment:
Done.
--
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 pull request #186: [SSHD-1141] Pubkey auth: handle failure with known signature algorithm
Posted by GitBox <gi...@apache.org>.
tomaswolf commented on pull request #186:
URL: https://github.com/apache/mina-sshd/pull/186#issuecomment-806112579
@lgoldstein : a final tweak.
--
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 #186: [SSHD-1141] Pubkey auth: handle failure with known signature algorithm
Posted by GitBox <gi...@apache.org>.
tomaswolf merged pull request #186:
URL: https://github.com/apache/mina-sshd/pull/186
--
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 #186: [SSHD-1141] Pubkey auth: handle failure with known signature algorithm
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #186:
URL: https://github.com/apache/mina-sshd/pull/186#discussion_r605868466
##########
File path: sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java
##########
@@ -105,6 +106,19 @@ protected boolean sendAuthDataRequest(ClientSession session, String service) thr
chosenAlgorithm = null;
} else if (!currentAlgorithms.isEmpty()) {
currentAlgorithm = currentAlgorithms.poll();
+ if (chosenAlgorithm != null) {
+ Set<String> knownServerAlgorithms = session.getAttribute(
+ DefaultClientKexExtensionHandler.SERVER_ALGORITHMS);
+ if (knownServerAlgorithms != null
+ && knownServerAlgorithms.contains(chosenAlgorithm)) {
+ // We've tried key 'current' with 'chosenAlgorithm', but it
+ // failed. However, the server had told us it supported
+ // 'chosenAlgorithm'. Thus it makes no sense to continue
+ // with this key and other signature algorithms. Skip to the
+ // next key, if any.
+ currentAlgorithm = null;
Review comment:
Let's add a DEBUG level log message summarizing this behavior so that user can tell when it occurred.
--
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 pull request #186: [SSHD-1141] Pubkey auth: handle failure with known signature algorithm
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on pull request #186:
URL: https://github.com/apache/mina-sshd/pull/186#issuecomment-812359931
Hi @tomaswolf , thanks for the great work. I do have one minor request - before merging into `master` please run a **full** `mvn clean install` - this is required due to the fact that during the build process the sources may be (re-)formatted, which might change them. We need to make sure that if someone pulls the code and compiles it, the `git status` command will not show any changes to our committed files. In this case, there were 2 files that were not properly formatted and they turned out when I was working on some issue and built the code - I was surprised to see 2 extra files that I have not touched marked as modified.
Again, no real harm - but let's try and avoid these potential pitfalls.
--
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