[GitHub] [geode] bschuchardt commented on a change in pull request #5131: GEODE-8144: endpoint identification in servers is not working

2020-05-22 Thread GitBox


bschuchardt commented on a change in pull request #5131:
URL: https://github.com/apache/geode/pull/5131#discussion_r429282471



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
##
@@ -791,7 +792,19 @@ private boolean setServerNames(SSLParameters 
modifiedParams, HostAndPort addr) {
   return false;
 }
 
-serverNames.add(new SNIHostName(addr.getHostName()));
+String hostName = addr.getHostName();
+if (this.sslConfig.doEndpointIdentification()
+&& InetAddressValidator.getInstance().isValid(hostName)) {
+  // endpoint validation typically uses a hostname in the sniServer 
parameter that the handshake
+  // will compare against the subject alternative addresses in the 
server's certificate. Here
+  // we attempt to get a hostname instead of the proffered numeric address
+  try {
+hostName = InetAddress.getByName(hostName).getCanonicalHostName();

Review comment:
   @pivotal-jbarrett if you will look at the implementation of 
getCanonicalHostName, I think you will find that it already addresses your 
concerns.  Also, this is just setting the sniServerName field, not redirecting 
the socket to connect to a different address.





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:
us...@infra.apache.org




[GitHub] [geode] bschuchardt commented on a change in pull request #5131: GEODE-8144: endpoint identification in servers is not working

2020-05-22 Thread GitBox


bschuchardt commented on a change in pull request #5131:
URL: https://github.com/apache/geode/pull/5131#discussion_r429282471



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
##
@@ -791,7 +792,19 @@ private boolean setServerNames(SSLParameters 
modifiedParams, HostAndPort addr) {
   return false;
 }
 
-serverNames.add(new SNIHostName(addr.getHostName()));
+String hostName = addr.getHostName();
+if (this.sslConfig.doEndpointIdentification()
+&& InetAddressValidator.getInstance().isValid(hostName)) {
+  // endpoint validation typically uses a hostname in the sniServer 
parameter that the handshake
+  // will compare against the subject alternative addresses in the 
server's certificate. Here
+  // we attempt to get a hostname instead of the proffered numeric address
+  try {
+hostName = InetAddress.getByName(hostName).getCanonicalHostName();

Review comment:
   @pivotal-jbarrett if you will look at the implementation of 
getCanonicalHostName, I think you will find that it already addresses your 
concerns.  Also, this is just setting the sniServerName field, not redirecting 
the socket to connect to a different address.





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:
us...@infra.apache.org




[GitHub] [geode] bschuchardt commented on a change in pull request #5131: GEODE-8144: endpoint identification in servers is not working

2020-05-22 Thread GitBox


bschuchardt commented on a change in pull request #5131:
URL: https://github.com/apache/geode/pull/5131#discussion_r429282471



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
##
@@ -791,7 +792,19 @@ private boolean setServerNames(SSLParameters 
modifiedParams, HostAndPort addr) {
   return false;
 }
 
-serverNames.add(new SNIHostName(addr.getHostName()));
+String hostName = addr.getHostName();
+if (this.sslConfig.doEndpointIdentification()
+&& InetAddressValidator.getInstance().isValid(hostName)) {
+  // endpoint validation typically uses a hostname in the sniServer 
parameter that the handshake
+  // will compare against the subject alternative addresses in the 
server's certificate. Here
+  // we attempt to get a hostname instead of the proffered numeric address
+  try {
+hostName = InetAddress.getByName(hostName).getCanonicalHostName();

Review comment:
   @pivotal-jbarrett if you will look at the implementation of 
getCanonicalHostName, I think you will find that it already addresses your 
concerns.





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:
us...@infra.apache.org