Skip to content

Commit cdd854f

Browse files
nodecenodece
authored
fix: Add strict hostname resolution and loopback check for advertised address (#4595)
* fix: Add strict hostname resolution and loopback check for advertised address * Fix style * Fix test
1 parent fabe44b commit cdd854f

File tree

6 files changed

+131
-34
lines changed

6 files changed

+131
-34
lines changed

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java‎

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -254,49 +254,56 @@ public static BookieId getBookieId(ServerConfiguration conf) throws UnknownHostE
254254
*/
255255
public static BookieSocketAddress getBookieAddress(ServerConfiguration conf)
256256
throws UnknownHostException {
257+
String hostAddress;
257258
// Advertised address takes precedence over the listening interface and the
258259
// useHostNameAsBookieID settings
259260
if (conf.getAdvertisedAddress() != null && conf.getAdvertisedAddress().trim().length() > 0) {
260-
String hostAddress = conf.getAdvertisedAddress().trim();
261-
return new BookieSocketAddress(hostAddress, conf.getBookiePort());
262-
}
263-
264-
String iface = conf.getListeningInterface();
265-
if (iface == null) {
266-
iface = "default";
267-
}
268-
269-
String hostAddress = DNS.getDefaultIP(iface);
270-
if (conf.getUseHostNameAsBookieID()) {
271-
try {
272-
hostAddress = InetAddress.getByName(hostAddress).getCanonicalHostName();
273-
} catch (Exception e) {
274-
UnknownHostException unknownHostException =
275-
new UnknownHostException("Unable to resolve hostname for interface: " + iface);
276-
unknownHostException.initCause(e);
277-
throw unknownHostException;
261+
hostAddress = conf.getAdvertisedAddress().trim();
262+
} else {
263+
String iface = conf.getListeningInterface();
264+
if (iface == null) {
265+
iface = "default";
278266
}
279-
if (conf.getUseShortHostName()) {
280-
/*
281-
* if short hostname is used, then FQDN is not used. Short
282-
* hostname is the hostname cut at the first dot.
283-
*/
284-
hostAddress = hostAddress.split("\\.", 2)[0];
267+
String defaultIP = DNS.getDefaultIP(iface);
268+
if (conf.getUseHostNameAsBookieID()) {
269+
try {
270+
hostAddress = InetAddress.getByName(defaultIP).getCanonicalHostName();
271+
} catch (Exception e) {
272+
UnknownHostException unknownHostException =
273+
new UnknownHostException("Unable to resolve hostname for interface: " + iface);
274+
unknownHostException.initCause(e);
275+
throw unknownHostException;
276+
}
277+
if (defaultIP.equals(hostAddress)) {
278+
throw new UnknownHostException("Unable to resolve hostname for ip address: " + defaultIP);
279+
}
280+
if (conf.getUseShortHostName()) {
281+
/*
282+
* if short hostname is used, then FQDN is not used. Short
283+
* hostname is the hostname cut at the first dot.
284+
*/
285+
hostAddress = hostAddress.split("\\.", 2)[0];
286+
}
287+
} else {
288+
hostAddress = defaultIP;
285289
}
286290
}
287291

288-
BookieSocketAddress addr =
292+
BookieSocketAddress bookieSocketAddress =
289293
new BookieSocketAddress(hostAddress, conf.getBookiePort());
290-
if (addr.getSocketAddress().getAddress().isLoopbackAddress()
291-
&& !conf.getAllowLoopback()) {
294+
InetAddress inetAddress = bookieSocketAddress.getSocketAddress().getAddress();
295+
if (inetAddress == null) {
296+
throw new UnknownHostException("Failed to resolve InetAddress for host address: " + hostAddress);
297+
}
298+
if (inetAddress.isLoopbackAddress() && !conf.getAllowLoopback()) {
292299
throw new UnknownHostException("Trying to listen on loopback address, "
293-
+ addr + " but this is forbidden by default "
300+
+ bookieSocketAddress + " but this is forbidden by default "
294301
+ "(see ServerConfiguration#getAllowLoopback()).\n"
295302
+ "If this happen, you can consider specifying the network interface"
296303
+ " to listen on (e.g. listeningInterface=eth0) or specifying the"
297304
+ " advertised address (e.g. advertisedAddress=172.x.y.z)");
298305
}
299-
return addr;
306+
return bookieSocketAddress;
300307
}
301308

302309
public LedgerDirsManager getLedgerDirsManager() {

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,16 @@ public ServerConfiguration setBookieId(String bookieId) {
12391239
return this;
12401240
}
12411241

1242+
/**
1243+
* Remove the configured BookieId for the bookie.
1244+
*
1245+
* @return server configuration
1246+
*/
1247+
public ServerConfiguration removeBookieId() {
1248+
this.setProperty(BOOKIE_ID, null);
1249+
return this;
1250+
}
1251+
12421252
/**
12431253
* Get the configured advertised address for the bookie.
12441254
*

‎bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieSocketAddressTest.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.bookkeeper.bookie;
19+
20+
import static org.apache.bookkeeper.bookie.BookieImpl.getBookieAddress;
21+
import static org.assertj.core.api.Assertions.assertThat;
22+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
23+
24+
import com.google.common.net.InetAddresses;
25+
import java.net.InetAddress;
26+
import java.net.UnknownHostException;
27+
import org.apache.bookkeeper.conf.ServerConfiguration;
28+
import org.apache.bookkeeper.net.BookieSocketAddress;
29+
import org.junit.Test;
30+
31+
public class BookieSocketAddressTest {
32+
33+
private void testAdvertisedWithLoopbackAddress(String address) throws UnknownHostException {
34+
ServerConfiguration conf = new ServerConfiguration();
35+
conf.setAdvertisedAddress(address);
36+
conf.setAllowLoopback(false);
37+
assertThatThrownBy(() -> getBookieAddress(conf)).isExactlyInstanceOf(UnknownHostException.class);
38+
39+
conf.setAllowLoopback(true);
40+
BookieSocketAddress bookieAddress = getBookieAddress(conf);
41+
assertThat(bookieAddress.getHostName()).isEqualTo(address);
42+
}
43+
44+
@Test
45+
public void testAdvertisedWithLoopbackAddress() throws UnknownHostException {
46+
testAdvertisedWithLoopbackAddress("localhost");
47+
testAdvertisedWithLoopbackAddress("127.0.0.1");
48+
}
49+
50+
@Test
51+
public void testAdvertisedWithNonLoopbackAddress() throws UnknownHostException {
52+
String hostAddress = InetAddress.getLocalHost().getHostAddress();
53+
if (hostAddress == null) {
54+
throw new UnknownHostException("Host address is null");
55+
}
56+
ServerConfiguration conf = new ServerConfiguration();
57+
conf.setAllowLoopback(false);
58+
conf.setAdvertisedAddress(hostAddress);
59+
BookieSocketAddress bookieAddress = getBookieAddress(conf);
60+
assertThat(bookieAddress.getHostName()).isEqualTo(hostAddress);
61+
}
62+
63+
@Test
64+
public void testBookieAddressIsIPAddressByDefault() throws UnknownHostException {
65+
ServerConfiguration conf = new ServerConfiguration();
66+
BookieSocketAddress bookieAddress = getBookieAddress(conf);
67+
assertThat(InetAddresses.isInetAddress(bookieAddress.getHostName())).isTrue();
68+
}
69+
70+
@Test
71+
public void testBookieAddressIsHostname() throws UnknownHostException {
72+
ServerConfiguration conf = new ServerConfiguration();
73+
conf.setUseHostNameAsBookieID(true);
74+
BookieSocketAddress bookieAddress = getBookieAddress(conf);
75+
assertThat(InetAddresses.isInetAddress(bookieAddress.getHostName())).isFalse();
76+
}
77+
}

‎bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -829,9 +829,10 @@ public void testRestartWithAdvertisedAddressAsBookieID() throws Exception {
829829
.setBookiePort(bookiePort)
830830
.setMetadataServiceUri(zkUtil.getMetadataServiceUri());
831831
conf.setUseHostNameAsBookieID(false);
832+
conf.setAllowLoopback(true);
832833
validateConfig(conf);
833834

834-
conf.setAdvertisedAddress("unknown");
835+
conf.setAdvertisedAddress("localhost");
835836
try {
836837
validateConfig(conf);
837838
fail("Should not start a bookie with ip if the bookie has been started with an ip");

‎bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/CookieValidationTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ private File initializedDir() throws Exception {
7575
private static ServerConfiguration serverConf(boolean stampMissingCookies) {
7676
ServerConfiguration conf = new ServerConfiguration();
7777
conf.setDataIntegrityStampMissingCookiesEnabled(stampMissingCookies);
78-
conf.setAdvertisedAddress("foobar");
78+
conf.setAdvertisedAddress("localhost");
79+
conf.setAllowLoopback(true);
7980
return conf;
8081
}
8182

@@ -255,7 +256,7 @@ public void testChangingBookieIdRaisesError() throws Exception {
255256
conf, regManager, new MockDataIntegrityCheck());
256257
v1.checkCookies(dirs); // stamp original cookies
257258

258-
conf.setAdvertisedAddress("barfoo");
259+
conf.setBookieId("barfoo");
259260
DataIntegrityCookieValidation v2 = new DataIntegrityCookieValidation(
260261
conf, regManager, new MockDataIntegrityCheck());
261262
try {
@@ -265,7 +266,7 @@ public void testChangingBookieIdRaisesError() throws Exception {
265266
// expected
266267
}
267268

268-
conf.setAdvertisedAddress("foobar");
269+
conf.removeBookieId();
269270
DataIntegrityCookieValidation v3 = new DataIntegrityCookieValidation(
270271
conf, regManager, new MockDataIntegrityCheck());
271272
v3.checkCookies(dirs); // should succeed as the cookie is same as before

‎bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/DataIntegrityCheckTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ public void teardown() throws Exception {
105105

106106
private static ServerConfiguration serverConf() {
107107
ServerConfiguration conf = new ServerConfiguration();
108-
conf.setAdvertisedAddress("foobar");
108+
conf.setAdvertisedAddress("localhost");
109+
conf.setAllowLoopback(true);
109110
return conf;
110111
}
111112

0 commit comments

Comments
 (0)