Skip to content

Commit c4726a9

Browse files
bbeaudreaultbbeaudreault
committed
add the ability to lookup by hostname, if exists. add tests. remove todo
1 parent 7a572d3 commit c4726a9

File tree

3 files changed

+122
-68
lines changed

3 files changed

+122
-68
lines changed

‎hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseHostnameVerifier.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
@InterfaceAudience.Private
5959
final class HBaseHostnameVerifier implements HostnameVerifier {
6060

61-
private final Logger LOG = LoggerFactory.getLogger(HBaseHostnameVerifier.class);
61+
private static final Logger LOG = LoggerFactory.getLogger(HBaseHostnameVerifier.class);
6262

6363
/**
6464
* Note: copied from Apache httpclient with some minor modifications. We want host verification,
@@ -269,9 +269,11 @@ private static List<SubjectName> getSubjectAltNames(final X509Certificate cert)
269269
if (type != null) {
270270
if (type == SubjectName.DNS || type == SubjectName.IP) {
271271
final Object o = entry.get(1);
272-
// TODO ASN.1 DER encoded form when instanceof byte[]
273272
if (o instanceof String) {
274273
result.add(new SubjectName((String) o, type));
274+
} else {
275+
LOG.debug("non-string Subject Alt Name type detected, not currently supported: {}",
276+
o);
275277
}
276278
}
277279
}

‎hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseTrustManager.java‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,17 +142,31 @@ private void performHostVerification(InetAddress inetAddress, X509Certificate ce
142142
hostAddress = inetAddress.getHostAddress();
143143
hostnameVerifier.verify(hostAddress, certificate);
144144
} catch (SSLException addressVerificationException) {
145-
if (!allowReverseDnsLookup) {
145+
// If we fail with hostAddress, we should try the hostname.
146+
// The inetAddress may have been created with a hostname, in which case getHostName() will
147+
// return quickly below. If not, a reverse lookup will happen, which can be expensive.
148+
// We provide the option to skip the reverse lookup if preferring to fail fast.
149+
150+
// Handle logging here to aid debugging. The easiest way to check for an existing
151+
// hostname is through toString, see javadoc.
152+
String inetAddressString = inetAddress.toString();
153+
if (!inetAddressString.startsWith("/")) {
154+
LOG.debug(
155+
"Failed to verify host address: {}, but inetAddress {} has a hostname, trying that",
156+
hostAddress, inetAddressString, addressVerificationException);
157+
} else if (allowReverseDnsLookup) {
158+
LOG.debug(
159+
"Failed to verify host address: {}, attempting to verify host name with reverse dns",
160+
hostAddress, addressVerificationException);
161+
} else {
146162
LOG.debug("Failed to verify host address: {}, but reverse dns lookup is disabled",
147163
hostAddress, addressVerificationException);
148-
throw new CertificateException("Failed to verify host address",
164+
throw new CertificateException(
165+
"Failed to verify host address, and reverse lookup is disabled",
149166
addressVerificationException);
150167
}
151168

152169
try {
153-
LOG.debug(
154-
"Failed to verify host address: {} attempting to verify host name with reverse dns",
155-
hostAddress, addressVerificationException);
156170
hostName = inetAddress.getHostName();
157171
hostnameVerifier.verify(hostName, certificate);
158172
} catch (SSLException hostnameVerificationException) {

‎hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestHBaseTrustManager.java‎

Lines changed: 99 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import org.junit.ClassRule;
6060
import org.junit.Test;
6161
import org.junit.experimental.categories.Category;
62-
import org.mockito.invocation.InvocationOnMock;
6362
import org.mockito.stubbing.Answer;
6463

6564
//
@@ -83,8 +82,10 @@ public class TestHBaseTrustManager {
8382
private static final String IP_ADDRESS = "127.0.0.1";
8483
private static final String HOSTNAME = "localhost";
8584

86-
private InetAddress mockInetAddress;
87-
private Socket mockSocket;
85+
private InetAddress mockInetAddressWithoutHostname;
86+
private InetAddress mockInetAddressWithHostname;
87+
private Socket mockSocketWithoutHostname;
88+
private Socket mockSocketWithHostname;
8889

8990
@BeforeClass
9091
public static void createKeyPair() throws Exception {
@@ -104,28 +105,27 @@ public static void removeBouncyCastleProvider() throws Exception {
104105
public void setup() throws Exception {
105106
mockX509ExtendedTrustManager = mock(X509ExtendedTrustManager.class);
106107

107-
mockInetAddress = mock(InetAddress.class);
108-
when(mockInetAddress.getHostAddress()).thenAnswer(new Answer() {
109-
@Override
110-
public Object answer(InvocationOnMock invocationOnMock) throws Throwable {
111-
return IP_ADDRESS;
112-
}
113-
});
114-
115-
when(mockInetAddress.getHostName()).thenAnswer(new Answer() {
116-
@Override
117-
public Object answer(InvocationOnMock invocationOnMock) throws Throwable {
118-
return HOSTNAME;
119-
}
120-
});
121-
122-
mockSocket = mock(Socket.class);
123-
when(mockSocket.getInetAddress()).thenAnswer(new Answer() {
124-
@Override
125-
public Object answer(InvocationOnMock invocationOnMock) throws Throwable {
126-
return mockInetAddress;
127-
}
128-
});
108+
mockInetAddressWithoutHostname = mock(InetAddress.class);
109+
when(mockInetAddressWithoutHostname.getHostAddress())
110+
.thenAnswer((Answer<?>) invocationOnMock -> IP_ADDRESS);
111+
when(mockInetAddressWithoutHostname.toString())
112+
.thenAnswer((Answer<?>) invocationOnMock -> "/" + IP_ADDRESS);
113+
114+
mockInetAddressWithHostname = mock(InetAddress.class);
115+
when(mockInetAddressWithHostname.getHostAddress())
116+
.thenAnswer((Answer<?>) invocationOnMock -> IP_ADDRESS);
117+
when(mockInetAddressWithHostname.getHostName())
118+
.thenAnswer((Answer<?>) invocationOnMock -> HOSTNAME);
119+
when(mockInetAddressWithHostname.toString())
120+
.thenAnswer((Answer<?>) invocationOnMock -> HOSTNAME + "/" + IP_ADDRESS);
121+
122+
mockSocketWithoutHostname = mock(Socket.class);
123+
when(mockSocketWithoutHostname.getInetAddress())
124+
.thenAnswer((Answer<?>) invocationOnMock -> mockInetAddressWithoutHostname);
125+
126+
mockSocketWithHostname = mock(Socket.class);
127+
when(mockSocketWithHostname.getInetAddress())
128+
.thenAnswer((Answer<?>) invocationOnMock -> mockInetAddressWithHostname);
129129
}
130130

131131
@SuppressWarnings("JavaUtilDate")
@@ -173,13 +173,13 @@ public void testServerHostnameVerificationWithHostnameVerificationDisabled() thr
173173
new HBaseTrustManager(mockX509ExtendedTrustManager, false, false);
174174

175175
X509Certificate[] certificateChain = createSelfSignedCertificateChain(IP_ADDRESS, HOSTNAME);
176-
trustManager.checkServerTrusted(certificateChain, null, mockSocket);
176+
trustManager.checkServerTrusted(certificateChain, null, mockSocketWithHostname);
177177

178-
verify(mockInetAddress, times(0)).getHostAddress();
179-
verify(mockInetAddress, times(0)).getHostName();
178+
verify(mockInetAddressWithHostname, times(0)).getHostAddress();
179+
verify(mockInetAddressWithHostname, times(0)).getHostName();
180180

181181
verify(mockX509ExtendedTrustManager, times(1)).checkServerTrusted(certificateChain, null,
182-
mockSocket);
182+
mockSocketWithHostname);
183183
}
184184

185185
@SuppressWarnings("checkstyle:linelength")
@@ -189,13 +189,13 @@ public void testServerTrustedWithHostnameVerificationDisabled() throws Exception
189189
new HBaseTrustManager(mockX509ExtendedTrustManager, false, false);
190190

191191
X509Certificate[] certificateChain = createSelfSignedCertificateChain(IP_ADDRESS, HOSTNAME);
192-
trustManager.checkServerTrusted(certificateChain, null, mockSocket);
192+
trustManager.checkServerTrusted(certificateChain, null, mockSocketWithHostname);
193193

194-
verify(mockInetAddress, times(0)).getHostAddress();
195-
verify(mockInetAddress, times(0)).getHostName();
194+
verify(mockInetAddressWithHostname, times(0)).getHostAddress();
195+
verify(mockInetAddressWithHostname, times(0)).getHostName();
196196

197197
verify(mockX509ExtendedTrustManager, times(1)).checkServerTrusted(certificateChain, null,
198-
mockSocket);
198+
mockSocketWithHostname);
199199
}
200200

201201
@Test
@@ -204,13 +204,13 @@ public void testServerTrustedWithHostnameVerificationEnabled() throws Exception
204204
new HBaseTrustManager(mockX509ExtendedTrustManager, true, true);
205205

206206
X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME);
207-
trustManager.checkServerTrusted(certificateChain, null, mockSocket);
207+
trustManager.checkServerTrusted(certificateChain, null, mockSocketWithHostname);
208208

209-
verify(mockInetAddress, times(1)).getHostAddress();
210-
verify(mockInetAddress, times(1)).getHostName();
209+
verify(mockInetAddressWithHostname, times(1)).getHostAddress();
210+
verify(mockInetAddressWithHostname, times(1)).getHostName();
211211

212212
verify(mockX509ExtendedTrustManager, times(1)).checkServerTrusted(certificateChain, null,
213-
mockSocket);
213+
mockSocketWithHostname);
214214
}
215215

216216
@Test
@@ -219,13 +219,13 @@ public void testServerTrustedWithHostnameVerificationEnabledUsingIpAddress() thr
219219
new HBaseTrustManager(mockX509ExtendedTrustManager, true, true);
220220

221221
X509Certificate[] certificateChain = createSelfSignedCertificateChain(IP_ADDRESS, null);
222-
trustManager.checkServerTrusted(certificateChain, null, mockSocket);
222+
trustManager.checkServerTrusted(certificateChain, null, mockSocketWithHostname);
223223

224-
verify(mockInetAddress, times(1)).getHostAddress();
225-
verify(mockInetAddress, times(0)).getHostName();
224+
verify(mockInetAddressWithHostname, times(1)).getHostAddress();
225+
verify(mockInetAddressWithHostname, times(0)).getHostName();
226226

227227
verify(mockX509ExtendedTrustManager, times(1)).checkServerTrusted(certificateChain, null,
228-
mockSocket);
228+
mockSocketWithHostname);
229229
}
230230

231231
@Test
@@ -239,13 +239,32 @@ public void testServerTrustedWithHostnameVerificationEnabledNoReverseLookup() th
239239
// This mismatch would succeed if reverse lookup is enabled, but here fails since it's
240240
// not enabled.
241241
assertThrows(CertificateException.class,
242-
() -> trustManager.checkServerTrusted(certificateChain, null, mockSocket));
242+
() -> trustManager.checkServerTrusted(certificateChain, null, mockSocketWithoutHostname));
243243

244-
verify(mockInetAddress, times(1)).getHostAddress();
245-
verify(mockInetAddress, times(0)).getHostName();
244+
verify(mockInetAddressWithoutHostname, times(1)).getHostAddress();
245+
verify(mockInetAddressWithoutHostname, times(0)).getHostName();
246246

247247
verify(mockX509ExtendedTrustManager, times(1)).checkServerTrusted(certificateChain, null,
248-
mockSocket);
248+
mockSocketWithoutHostname);
249+
}
250+
251+
@Test
252+
public void testServerTrustedWithHostnameVerificationEnabledWithHostnameNoReverseLookup()
253+
throws Exception {
254+
HBaseTrustManager trustManager =
255+
new HBaseTrustManager(mockX509ExtendedTrustManager, true, false);
256+
257+
X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME);
258+
259+
// since the socket inetAddress already has a hostname, we don't need reverse lookup.
260+
// so this succeeds
261+
trustManager.checkServerTrusted(certificateChain, null, mockSocketWithHostname);
262+
263+
verify(mockInetAddressWithHostname, times(1)).getHostAddress();
264+
verify(mockInetAddressWithHostname, times(1)).getHostName();
265+
266+
verify(mockX509ExtendedTrustManager, times(1)).checkServerTrusted(certificateChain, null,
267+
mockSocketWithHostname);
249268
}
250269

251270
@Test
@@ -254,13 +273,13 @@ public void testClientTrustedWithHostnameVerificationDisabled() throws Exception
254273
new HBaseTrustManager(mockX509ExtendedTrustManager, false, false);
255274

256275
X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME);
257-
trustManager.checkClientTrusted(certificateChain, null, mockSocket);
276+
trustManager.checkClientTrusted(certificateChain, null, mockSocketWithHostname);
258277

259-
verify(mockInetAddress, times(0)).getHostAddress();
260-
verify(mockInetAddress, times(0)).getHostName();
278+
verify(mockInetAddressWithHostname, times(0)).getHostAddress();
279+
verify(mockInetAddressWithHostname, times(0)).getHostName();
261280

262281
verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null,
263-
mockSocket);
282+
mockSocketWithHostname);
264283
}
265284

266285
@Test
@@ -269,13 +288,13 @@ public void testClientTrustedWithHostnameVerificationEnabled() throws Exception
269288
new HBaseTrustManager(mockX509ExtendedTrustManager, true, true);
270289

271290
X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME);
272-
trustManager.checkClientTrusted(certificateChain, null, mockSocket);
291+
trustManager.checkClientTrusted(certificateChain, null, mockSocketWithHostname);
273292

274-
verify(mockInetAddress, times(1)).getHostAddress();
275-
verify(mockInetAddress, times(1)).getHostName();
293+
verify(mockInetAddressWithHostname, times(1)).getHostAddress();
294+
verify(mockInetAddressWithHostname, times(1)).getHostName();
276295

277296
verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null,
278-
mockSocket);
297+
mockSocketWithHostname);
279298
}
280299

281300
@Test
@@ -284,13 +303,13 @@ public void testClientTrustedWithHostnameVerificationEnabledUsingIpAddress() thr
284303
new HBaseTrustManager(mockX509ExtendedTrustManager, true, true);
285304

286305
X509Certificate[] certificateChain = createSelfSignedCertificateChain(IP_ADDRESS, null);
287-
trustManager.checkClientTrusted(certificateChain, null, mockSocket);
306+
trustManager.checkClientTrusted(certificateChain, null, mockSocketWithHostname);
288307

289-
verify(mockInetAddress, times(1)).getHostAddress();
290-
verify(mockInetAddress, times(0)).getHostName();
308+
verify(mockInetAddressWithHostname, times(1)).getHostAddress();
309+
verify(mockInetAddressWithHostname, times(0)).getHostName();
291310

292311
verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null,
293-
mockSocket);
312+
mockSocketWithHostname);
294313
}
295314

296315
@Test
@@ -305,13 +324,32 @@ public void testClientTrustedWithHostnameVerificationEnabledWithoutReverseLookup
305324
// This mismatch would succeed if reverse lookup is enabled, but here fails since it's
306325
// not enabled.
307326
assertThrows(CertificateException.class,
308-
() -> trustManager.checkClientTrusted(certificateChain, null, mockSocket));
327+
() -> trustManager.checkClientTrusted(certificateChain, null, mockSocketWithoutHostname));
328+
329+
verify(mockInetAddressWithoutHostname, times(1)).getHostAddress();
330+
verify(mockInetAddressWithoutHostname, times(0)).getHostName();
331+
332+
verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null,
333+
mockSocketWithoutHostname);
334+
}
335+
336+
@Test
337+
public void testClientTrustedWithHostnameVerificationEnabledWithHostnameNoReverseLookup()
338+
throws Exception {
339+
HBaseTrustManager trustManager =
340+
new HBaseTrustManager(mockX509ExtendedTrustManager, true, false);
341+
342+
X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME);
343+
344+
// since the socket inetAddress already has a hostname, we don't need reverse lookup.
345+
// so this succeeds
346+
trustManager.checkClientTrusted(certificateChain, null, mockSocketWithHostname);
309347

310-
verify(mockInetAddress, times(1)).getHostAddress();
311-
verify(mockInetAddress, times(0)).getHostName();
348+
verify(mockInetAddressWithHostname, times(1)).getHostAddress();
349+
verify(mockInetAddressWithHostname, times(1)).getHostName();
312350

313351
verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null,
314-
mockSocket);
352+
mockSocketWithHostname);
315353
}
316354

317355
}

0 commit comments

Comments
 (0)