Bill commented on a change in pull request #7119:
URL: https://github.com/apache/geode/pull/7119#discussion_r754715790
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
##########
@@ -192,7 +196,7 @@
*/
private final String conduitIdStr;
- private InternalDistributedMember remoteAddr;
+ private InternalDistributedMember remoteMember;
Review comment:
Big improvement in naming here!
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
##########
@@ -373,14 +373,12 @@ void configureSSLEngine(SSLEngine engine, String
hostName, int port, boolean cli
}
engine.setUseClientMode(clientSocket);
- if (!clientSocket) {
- engine.setNeedClientAuth(sslConfig.isRequireAuth());
- }
-
if (clientSocket) {
if (checkAndEnableHostnameValidation(parameters)) {
updateEngineWithParameters = true;
}
+ } else {
+ engine.setNeedClientAuth(sslConfig.isRequireAuth());
Review comment:
This refactor improves readability.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
##########
@@ -1798,6 +1794,46 @@ private void createIoFilter(SocketChannel channel,
boolean clientSocket) throws
}
}
+ private SSLEngine createSslEngine(final InetSocketAddress remoteAddress) {
+ final String hostName;
+ final boolean isSender = remoteMember != null;
+ if (isSender) {
+ hostName = convertToFqdnIfNeeded(remoteMember.getHostName(),
remoteMember.getVersion());
+ } else {
+ hostName = SocketCreator.getHostName(remoteAddress.getAddress());
+ }
+ return getConduit().getSocketCreator().createSSLEngine(hostName,
remoteAddress.getPort(),
+ isSender);
+ }
+
+ /**
+ * In older versions of Geode {@link
InternalDistributedMember#getHostName()} can return an IP
+ * address if network partition detection is enabled. If SSL endpoint
identification is enabled,
+ * those product versions supply the result of a reverse lookup to the TLS
handshake API.
+ * Endpoint identification will fail if e.g. the lookup returned a
fully-qualified name but
+ * the certificate had just a (non-fully-qualified) hostname in a Subject
Alternate Name field.
+ *
+ * In version 1.15.0 member identifiers were changed so that if a bind
address is specified,
+ * that exact string will be carried as the host name. That gives the
administrator better control
+ * over endpoint identification. When connecting to earlier versions we
convert any IP numbers
+ * to hostnames via reverse lookup here.
+ *
+ * This method should be removed once pre-1.15.0 versions get out of support
(i.e. along with
+ * the removal of fromDataPre_GEODE_1_15_0_0 and toDataPre_GEODE_1_15_0_0
methods)
+ */
+ protected String convertToFqdnIfNeeded(final String hostNameOrIP, final
Version version) {
+ if (version.isOlderThan(KnownVersion.GEODE_1_15_0)
+ && owner.getDM().getConfig().getSSLEndPointIdentificationEnabled()
+ && InetAddressValidator.getInstance().isValid(hostNameOrIP)) {
+ try {
+ return InetAddress.getByName(hostNameOrIP).getCanonicalHostName();
+ } catch (UnknownHostException e) {
+ // ignore
+ }
+ }
+ return hostNameOrIP;
+ }
+
Review comment:
The crux of this PR. This looks good to me. Thanks for the clear
comment. I feel like whoever (eventually) deprecates the 1.15 line will notice
this when they grep for "1.15". Not quite as elegant as a version-specific
deserialization method on MemberIdentifier, but this approach avoids problems
with our client code (using MemberIdentifiers in region names).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
