You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/10/03 06:29:39 UTC

[GitHub] [commons-net] tobias- opened a new pull request #63: Hostname used in hostname verification

tobias- opened a new pull request #63:
URL: https://github.com/apache/commons-net/pull/63


   Previously, the ip was used in hostname validation, causing it
   to always fail unless the ip was in the certificate making MitM
   easy.
   
   By adding the following two lines, the validation of certificate
   will be more like a browser.
   client.setEndpointCheckingEnabled(true);
   client.setTrustManager(TrustManagerUtils.getDefaultTrustManager(null))
   
   The old behaviour of validating by ip is still available by connecting
   using the ip (yet again, similar to browser).
   
   The ftp server certificate has had it's CN changed from "ftpserver"
   to "localhost" in order to match the hostname being connected to.


----------------------------------------------------------------
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



[GitHub] [commons-net] tobias- commented on pull request #63: Hostname used in hostname verification

Posted by GitBox <gi...@apache.org>.
tobias- commented on pull request #63:
URL: https://github.com/apache/commons-net/pull/63#issuecomment-703105797


   @garydgregory Uhmm.. I understand that you get shit PR's, but can you allow me more than 10 minutes to answer your question before closing my PR?


----------------------------------------------------------------
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



[GitHub] [commons-net] tobias- commented on pull request #63: Hostname used in hostname verification

Posted by GitBox <gi...@apache.org>.
tobias- commented on pull request #63:
URL: https://github.com/apache/commons-net/pull/63#issuecomment-703105613


   > Hi @tobias-
   > You issue description mentions:
   > 
   > ```
   > client.setTrustManager(TrustManagerUtils.getDefaultTrustManager(null))
   > ```
   > 
   > but I do not see that in this PR. What am I missing?
   
   Since there is no way of creating a valid certificate for localhost. I cannot add that line and get tests to pass.


----------------------------------------------------------------
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



[GitHub] [commons-net] garydgregory closed pull request #63: Hostname used in hostname verification

Posted by GitBox <gi...@apache.org>.
garydgregory closed pull request #63:
URL: https://github.com/apache/commons-net/pull/63


   


----------------------------------------------------------------
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



[GitHub] [commons-net] garydgregory commented on pull request #63: Hostname used in hostname verification

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #63:
URL: https://github.com/apache/commons-net/pull/63#issuecomment-703105508


   Thank you for your patch but please still reply to my query above.
   Committed with:
   ```
   commit 7827f0549526920b91aa2e62d2a1e7682c36e1e7
   Author: Gary Gregory <ga...@gmail.com> 2020-10-03 09:38:39
   Committer: Gary Gregory <ga...@gmail.com> 2020-10-03 09:38:39
   Parent: bcfb12594d1f9e7c2d784210a1e77bc402d5894f (Bump actions/setup-java from v1.4.2 to v1.4.3 #62.)
   Branches: master, origin/master
   
   [NET-689] Hostname is not set on the SSLSocket causing
   isEndpointCheckingEnabled to fail.
   The main side of this commit is from
   https://github.com/apache/commons-net/pull/63 but the test side has been
   rewritten to test endpoint checking as both enabled and disabled.
   ```
   Closing.
   


----------------------------------------------------------------
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



[GitHub] [commons-net] garydgregory commented on pull request #63: Hostname used in hostname verification

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #63:
URL: https://github.com/apache/commons-net/pull/63#issuecomment-703103559


   Hi @tobias- 
   You issue description mentions:
   ```
   client.setTrustManager(TrustManagerUtils.getDefaultTrustManager(null))
   ```
   but I do not see that in this PR. What am I missing?
   


----------------------------------------------------------------
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



[GitHub] [commons-net] tobias- commented on pull request #63: Hostname used in hostname verification

Posted by GitBox <gi...@apache.org>.
tobias- commented on pull request #63:
URL: https://github.com/apache/commons-net/pull/63#issuecomment-703107399


   @garydgregory Ok, now I understand your comment. You merged the PR in another system. Either way, my answer stands, It's impossible to add that line without a valid certificate, which you cannot distribute. I felt that particular part of the test was a bit outside of my scope, as there were no tests for any of the certificate validating functionality, and the trust manager wasn't involved in that particular part of validating the certificate.


----------------------------------------------------------------
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



[GitHub] [commons-net] tobias- commented on pull request #63: Hostname used in hostname verification

Posted by GitBox <gi...@apache.org>.
tobias- commented on pull request #63:
URL: https://github.com/apache/commons-net/pull/63#issuecomment-703090041


   This is for issue https://issues.apache.org/jira/browse/NET-689


----------------------------------------------------------------
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