diff --git a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsMessageTransport.java b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsMessageTransport.java deleted file mode 100644 index 99e4347d056..00000000000 --- a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsMessageTransport.java +++ /dev/null @@ -1,134 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.dns.writer.dnsupdate; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Verify.verify; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.primitives.Ints; -import google.registry.config.RegistryConfig.Config; -import jakarta.inject.Inject; -import java.io.DataInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.net.InetAddress; -import java.net.Socket; -import java.nio.ByteBuffer; -import java.time.Duration; -import javax.net.SocketFactory; -import org.xbill.DNS.Message; -import org.xbill.DNS.Opcode; - -/** - * A transport for DNS messages. Sends/receives DNS messages over TCP using old-style {@link Socket} - * s and the message framing defined in RFC 1035. - * - *

TODO(b/463732345): now that we're no longer on AppEngine, see if we can use the dnsjava - * library's {@link org.xbill.DNS.SimpleResolver} class instead of this (that requires {@link - * java.nio.channels.SocketChannel} which is not supported on AppEngine). - */ -public class DnsMessageTransport { - - /** - * Size of message length field for DNS TCP transport. - * - * @see RFC 1035 - */ - static final int MESSAGE_LENGTH_FIELD_BYTES = 2; - private static final int MESSAGE_MAXIMUM_LENGTH = (1 << (MESSAGE_LENGTH_FIELD_BYTES * 8)) - 1; - - /** - * The standard DNS port number. - * - * @see RFC 1035 - */ - @VisibleForTesting static final int DNS_PORT = 53; - - private final SocketFactory factory; - private final String updateHost; - private final int updateTimeout; - - /** - * Class constructor. - * - * @param factory a factory for TCP sockets - * @param updateHost host name of the DNS server - * @param updateTimeout update I/O timeout - */ - @Inject - public DnsMessageTransport( - SocketFactory factory, - @Config("dnsUpdateHost") String updateHost, - @Config("dnsUpdateTimeout") Duration updateTimeout) { - this.factory = factory; - this.updateHost = updateHost; - this.updateTimeout = Ints.checkedCast(updateTimeout.toMillis()); - } - - /** - * Sends a DNS "query" message (most likely an UPDATE) and returns the response. The response is - * checked for matching ID and opcode. - * - * @param query a message to send - * @return the response received from the server - * @throws IOException if the Socket input/output streams throws one - * @throws IllegalArgumentException if the query is too large to be sent (> 65535 bytes) - */ - public Message send(Message query) throws IOException { - try (Socket socket = factory.createSocket(InetAddress.getByName(updateHost), DNS_PORT)) { - socket.setSoTimeout(updateTimeout); - writeMessage(socket.getOutputStream(), query); - Message response = readMessage(socket.getInputStream()); - checkValidResponse(query, response); - return response; - } - } - - private void checkValidResponse(Message query, Message response) { - verify( - response.getHeader().getID() == query.getHeader().getID(), - "response ID %s does not match query ID %s", - response.getHeader().getID(), - query.getHeader().getID()); - verify( - response.getHeader().getOpcode() == query.getHeader().getOpcode(), - "response opcode '%s' does not match query opcode '%s'", - Opcode.string(response.getHeader().getOpcode()), - Opcode.string(query.getHeader().getOpcode())); - } - - private void writeMessage(OutputStream outputStream, Message message) throws IOException { - byte[] messageData = message.toWire(); - checkArgument( - messageData.length <= MESSAGE_MAXIMUM_LENGTH, - "DNS request message larger than maximum of %s: %s", - MESSAGE_MAXIMUM_LENGTH, - messageData.length); - ByteBuffer buffer = ByteBuffer.allocate(messageData.length + MESSAGE_LENGTH_FIELD_BYTES); - buffer.putShort((short) messageData.length); - buffer.put(messageData); - outputStream.write(buffer.array()); - } - - private Message readMessage(InputStream inputStream) throws IOException { - DataInputStream stream = new DataInputStream(inputStream); - int length = stream.readUnsignedShort(); - byte[] messageData = new byte[length]; - stream.readFully(messageData); - return new Message(messageData); - } -} diff --git a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java index 0c1b1414764..0778d958c2e 100644 --- a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java +++ b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java @@ -50,6 +50,7 @@ import org.xbill.DNS.Name; import org.xbill.DNS.RRset; import org.xbill.DNS.Rcode; +import org.xbill.DNS.Resolver; import org.xbill.DNS.TextParseException; import org.xbill.DNS.Type; import org.xbill.DNS.Update; @@ -58,7 +59,7 @@ * A DnsWriter that implements the DNS UPDATE protocol as specified in RFC 2136. Publishes changes in the domain-registry * to a (capable) external DNS server, sometimes called a "hidden master". DNS UPDATE messages are - * sent via a supplied "transport" class. + * sent via a supplied {@link Resolver}. * *

On call to {@link #commit()}, a single UPDATE message is created containing the records * required to "synchronize" the DNS with the current (at the time of processing) state of the @@ -89,7 +90,7 @@ public class DnsUpdateWriter extends BaseDnsWriter { private final Duration dnsDefaultATtl; private final Duration dnsDefaultNsTtl; private final Duration dnsDefaultDsTtl; - private final DnsMessageTransport transport; + private final Resolver resolver; private final Clock clock; private final Update update; private final String zoneName; @@ -100,7 +101,7 @@ public class DnsUpdateWriter extends BaseDnsWriter { * @param dnsDefaultATtl TTL used for any created resource records * @param dnsDefaultNsTtl TTL used for any created nameserver records * @param dnsDefaultDsTtl TTL used for any created DS records - * @param transport the transport used to send/receive the UPDATE messages + * @param resolver the resolver used to send/receive the UPDATE messages * @param clock a source of time */ @Inject @@ -109,14 +110,14 @@ public DnsUpdateWriter( @Config("dnsDefaultATtl") Duration dnsDefaultATtl, @Config("dnsDefaultNsTtl") Duration dnsDefaultNsTtl, @Config("dnsDefaultDsTtl") Duration dnsDefaultDsTtl, - DnsMessageTransport transport, + Resolver resolver, Clock clock) { this.zoneName = zoneName; this.update = new Update(toAbsoluteName(zoneName)); this.dnsDefaultATtl = dnsDefaultATtl; this.dnsDefaultNsTtl = dnsDefaultNsTtl; this.dnsDefaultDsTtl = dnsDefaultDsTtl; - this.transport = transport; + this.resolver = resolver; this.clock = clock; } @@ -175,7 +176,7 @@ public void publishHost(String hostName) { @Override protected void commitUnchecked() { try { - Message response = transport.send(update); + Message response = resolver.send(update); verify( response.getRcode() == Rcode.NOERROR, "DNS server failed domain update for '%s' rcode: %s", diff --git a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java index 7fcca5b8e9e..9610b974337 100644 --- a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java +++ b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java @@ -20,17 +20,30 @@ import dagger.multibindings.IntoMap; import dagger.multibindings.IntoSet; import dagger.multibindings.StringKey; +import google.registry.config.RegistryConfig.Config; import google.registry.dns.writer.DnsWriter; import jakarta.inject.Named; -import javax.net.SocketFactory; +import java.net.UnknownHostException; +import java.time.Duration; +import org.xbill.DNS.Resolver; +import org.xbill.DNS.SimpleResolver; /** Dagger module that provides a DnsUpdateWriter. */ @Module public abstract class DnsUpdateWriterModule { @Provides - static SocketFactory provideSocketFactory() { - return SocketFactory.getDefault(); + static Resolver provideResolver( + @Config("dnsUpdateHost") String updateHost, + @Config("dnsUpdateTimeout") Duration updateTimeout) { + try { + SimpleResolver resolver = new SimpleResolver(updateHost); + resolver.setTCP(true); + resolver.setTimeout(updateTimeout); + return resolver; + } catch (UnknownHostException e) { + throw new RuntimeException(e); + } } @Binds diff --git a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsMessageTransportTest.java b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsMessageTransportTest.java deleted file mode 100644 index 78bf6f440c1..00000000000 --- a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsMessageTransportTest.java +++ /dev/null @@ -1,193 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.dns.writer.dnsupdate; - -import static com.google.common.io.BaseEncoding.base16; -import static com.google.common.truth.Truth.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import com.google.common.base.VerifyException; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.EOFException; -import java.io.InputStream; -import java.net.InetAddress; -import java.net.Socket; -import java.net.SocketTimeoutException; -import java.nio.ByteBuffer; -import java.time.Duration; -import java.util.Arrays; -import javax.net.SocketFactory; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; -import org.xbill.DNS.ARecord; -import org.xbill.DNS.DClass; -import org.xbill.DNS.Flags; -import org.xbill.DNS.Message; -import org.xbill.DNS.Name; -import org.xbill.DNS.Opcode; -import org.xbill.DNS.Rcode; -import org.xbill.DNS.Record; -import org.xbill.DNS.Type; -import org.xbill.DNS.Update; - -/** Unit tests for {@link DnsMessageTransport}. */ -class DnsMessageTransportTest { - - private static final String UPDATE_HOST = "127.0.0.1"; - - private final SocketFactory mockFactory = mock(SocketFactory.class); - private final Socket mockSocket = mock(Socket.class); - - private Message simpleQuery; - private Message expectedResponse; - private DnsMessageTransport resolver; - - @BeforeEach - @SuppressWarnings("AddressSelection") - void beforeEach() throws Exception { - simpleQuery = - Message.newQuery(Record.newRecord(Name.fromString("example.com."), Type.A, DClass.IN)); - expectedResponse = responseMessageWithCode(simpleQuery, Rcode.NOERROR); - when(mockFactory.createSocket(InetAddress.getByName(UPDATE_HOST), DnsMessageTransport.DNS_PORT)) - .thenReturn(mockSocket); - resolver = new DnsMessageTransport(mockFactory, UPDATE_HOST, Duration.ZERO); - } - - @Test - void testSentMessageHasCorrectLengthAndContent() throws Exception { - ByteArrayInputStream inputStream = - new ByteArrayInputStream(messageToBytesWithLength(expectedResponse)); - when(mockSocket.getInputStream()).thenReturn(inputStream); - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - when(mockSocket.getOutputStream()).thenReturn(outputStream); - - resolver.send(simpleQuery); - - ByteBuffer sentMessage = ByteBuffer.wrap(outputStream.toByteArray()); - int messageLength = sentMessage.getShort(); - byte[] messageData = new byte[messageLength]; - sentMessage.get(messageData); - assertThat(messageLength).isEqualTo(simpleQuery.toWire().length); - assertThat(base16().encode(messageData)).isEqualTo(base16().encode(simpleQuery.toWire())); - } - - @Test - void testReceivedMessageWithLengthHasCorrectContent() throws Exception { - ByteArrayInputStream inputStream = - new ByteArrayInputStream(messageToBytesWithLength(expectedResponse)); - when(mockSocket.getInputStream()).thenReturn(inputStream); - when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream()); - - Message actualResponse = resolver.send(simpleQuery); - - assertThat(base16().encode(actualResponse.toWire())) - .isEqualTo(base16().encode(expectedResponse.toWire())); - } - - @Test - void testEofReceivingResponse() throws Exception { - byte[] messageBytes = messageToBytesWithLength(expectedResponse); - ByteArrayInputStream inputStream = - new ByteArrayInputStream(Arrays.copyOf(messageBytes, messageBytes.length - 1)); - when(mockSocket.getInputStream()).thenReturn(inputStream); - when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream()); - assertThrows(EOFException.class, () -> resolver.send(new Message())); - } - - @Test - @Disabled("This test hangs and causes OOM when using Java 21") - // This test should never have passed. The socket is mocked and calling setSoTimeout() has no - // effect whatsoever. We should consider removing it. - void testTimeoutReceivingResponse() throws Exception { - InputStream mockInputStream = mock(InputStream.class); - when(mockInputStream.read()).thenThrow(new SocketTimeoutException("testing")); - when(mockSocket.getInputStream()).thenReturn(mockInputStream); - when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream()); - - Duration testTimeout = Duration.ofSeconds(1); - DnsMessageTransport resolver = new DnsMessageTransport(mockFactory, UPDATE_HOST, testTimeout); - Message expectedQuery = new Message(); - assertThrows(SocketTimeoutException.class, () -> resolver.send(expectedQuery)); - verify(mockSocket).setSoTimeout((int) testTimeout.toMillis()); - } - - @Test - void testSentMessageTooLongThrowsException() throws Exception { - Update oversize = new Update(Name.fromString("tld", Name.root)); - for (int i = 0; i < 2000; i++) { - oversize.add( - ARecord.newRecord( - Name.fromString("test-extremely-long-name-" + i + ".tld", Name.root), - Type.A, - DClass.IN)); - } - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - when(mockSocket.getOutputStream()).thenReturn(outputStream); - IllegalArgumentException thrown = - assertThrows(IllegalArgumentException.class, () -> resolver.send(oversize)); - assertThat(thrown).hasMessageThat().contains("message larger than maximum"); - } - - @Test - void testResponseIdMismatchThrowsException() throws Exception { - expectedResponse.getHeader().setID(1 + simpleQuery.getHeader().getID()); - when(mockSocket.getInputStream()) - .thenReturn(new ByteArrayInputStream(messageToBytesWithLength(expectedResponse))); - when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream()); - VerifyException thrown = assertThrows(VerifyException.class, () -> resolver.send(simpleQuery)); - assertThat(thrown) - .hasMessageThat() - .contains( - "response ID " - + expectedResponse.getHeader().getID() - + " does not match query ID " - + simpleQuery.getHeader().getID()); - } - - @Test - void testResponseOpcodeMismatchThrowsException() throws Exception { - simpleQuery.getHeader().setOpcode(Opcode.QUERY); - expectedResponse.getHeader().setOpcode(Opcode.STATUS); - when(mockSocket.getInputStream()) - .thenReturn(new ByteArrayInputStream(messageToBytesWithLength(expectedResponse))); - when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream()); - VerifyException thrown = assertThrows(VerifyException.class, () -> resolver.send(simpleQuery)); - assertThat(thrown) - .hasMessageThat() - .contains("response opcode 'STATUS' does not match query opcode 'QUERY'"); - } - - private Message responseMessageWithCode(Message query, int responseCode) { - Message message = new Message(query.getHeader().getID()); - message.getHeader().setOpcode(query.getHeader().getOpcode()); - message.getHeader().setFlag(Flags.QR); - message.getHeader().setRcode(responseCode); - return message; - } - - private byte[] messageToBytesWithLength(Message message) { - byte[] bytes = message.toWire(); - ByteBuffer buffer = - ByteBuffer.allocate(bytes.length + DnsMessageTransport.MESSAGE_LENGTH_FIELD_BYTES); - buffer.putShort((short) bytes.length); - buffer.put(bytes); - return buffer.array(); - } -} diff --git a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java index 2cdace40075..6dbea9cdb73 100644 --- a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java +++ b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.newDomain; import static google.registry.testing.DatabaseHelper.newHost; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistActiveHost; @@ -43,13 +44,20 @@ import google.registry.model.tld.Tld; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; -import google.registry.testing.DatabaseHelper; import google.registry.testing.FakeClock; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -66,12 +74,16 @@ import org.xbill.DNS.RRset; import org.xbill.DNS.Rcode; import org.xbill.DNS.Record; +import org.xbill.DNS.Resolver; import org.xbill.DNS.Section; +import org.xbill.DNS.SimpleResolver; import org.xbill.DNS.Type; import org.xbill.DNS.Update; +import org.xbill.DNS.WireParseException; /** Unit tests for {@link DnsUpdateWriter}. */ @ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) public class DnsUpdateWriterTest { @RegisterExtension @@ -81,7 +93,7 @@ public class DnsUpdateWriterTest { // Base16 encoded sha1 digest. Must have 40 chars (20 bytes after decoding) private static final String BASE16_SHA1_DIGEST = "0123".repeat(10); - @Mock private DnsMessageTransport mockResolver; + @Mock private Resolver mockResolver; @Captor private ArgumentCaptor updateCaptor; private final FakeClock clock = new FakeClock(Instant.parse("1971-01-01T00:00:00Z")); @@ -103,7 +115,7 @@ void testPublishDomainCreate_publishesNameServers() throws Exception { Host host1 = persistActiveHost("ns1.example.tld"); Host host2 = persistActiveHost("ns2.example.tld"); Domain domain = - persistActiveDomain("example.tld") + newDomain("example.tld") .asBuilder() .setNameservers(ImmutableSet.of(host1.createVKey(), host2.createVKey())) .build(); @@ -126,12 +138,11 @@ void testPublishDomainCreate_publishesNameServers() throws Exception { assertThatTotalUpdateSetsIs(update, 2); // The delete and NS sets } - @MockitoSettings(strictness = Strictness.LENIENT) @Test void testPublishAtomic_noCommit() { Host host1 = persistActiveHost("ns.example1.tld"); Domain domain1 = - persistActiveDomain("example1.tld") + newDomain("example1.tld") .asBuilder() .setNameservers(ImmutableSet.of(host1.createVKey())) .build(); @@ -139,7 +150,7 @@ void testPublishAtomic_noCommit() { Host host2 = persistActiveHost("ns.example2.tld"); Domain domain2 = - persistActiveDomain("example2.tld") + newDomain("example2.tld") .asBuilder() .setNameservers(ImmutableSet.of(host2.createVKey())) .build(); @@ -155,7 +166,7 @@ void testPublishAtomic_noCommit() { void testPublishAtomic_oneUpdate() throws Exception { Host host1 = persistActiveHost("ns.example1.tld"); Domain domain1 = - persistActiveDomain("example1.tld") + newDomain("example1.tld") .asBuilder() .setNameservers(ImmutableSet.of(host1.createVKey())) .build(); @@ -163,7 +174,7 @@ void testPublishAtomic_oneUpdate() throws Exception { Host host2 = persistActiveHost("ns.example2.tld"); Domain domain2 = - persistActiveDomain("example2.tld") + newDomain("example2.tld") .asBuilder() .setNameservers(ImmutableSet.of(host2.createVKey())) .build(); @@ -188,7 +199,7 @@ void testPublishAtomic_oneUpdate() throws Exception { @Test void testPublishDomainCreate_publishesDelegationSigner_usesDefaultTtl() throws Exception { Domain domain = - persistActiveDomain("example.tld") + newDomain("example.tld") .asBuilder() .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createVKey())) .setDsData( @@ -219,7 +230,7 @@ void testPublishDomainCreate_publishesDelegationSigner_usesTldConfiguredTtl() th .setDnsDsTtl(Duration.ofMillis(400)) .build()); Domain domain = - persistActiveDomain("example.tld") + newDomain("example.tld") .asBuilder() .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createVKey())) .setDsData( @@ -248,7 +259,7 @@ void testPublishDomainCreate_publishesDelegationSigner_usesTldConfiguredTtl() th @Test void testPublishDomainWhenNotActive_removesDnsRecords() throws Exception { Domain domain = - persistActiveDomain("example.tld") + newDomain("example.tld") .asBuilder() .addStatusValue(StatusValue.SERVER_HOLD) .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createVKey())) @@ -292,7 +303,7 @@ void testPublishHostCreate_publishesAddressRecords_usesDefaultTtl() throws Excep InetAddresses.forString("fd0e:a5c8:6dfb:6a5e:0:0:0:1"))) .build()); persistResource( - DatabaseHelper.newDomain("example.tld") + newDomain("example.tld") .asBuilder() .addSubordinateHost("ns1.example.tld") .addNameserver(host.createVKey()) @@ -338,7 +349,7 @@ void testPublishHostCreate_publishesAddressRecords_usesTldConfiguredTtl() throws InetAddresses.forString("fd0e:a5c8:6dfb:6a5e:0:0:0:1"))) .build()); persistResource( - DatabaseHelper.newDomain("example.tld") + newDomain("example.tld") .asBuilder() .addSubordinateHost("ns1.example.tld") .addNameserver(host.createVKey()) @@ -390,7 +401,7 @@ void testPublishHostDelete_removesDnsRecords() throws Exception { void testPublishHostDelete_removesGlueRecords() throws Exception { persistDeletedHost("ns1.example.tld", minusDays(clock.now(), 1)); persistResource( - persistActiveDomain("example.tld") + newDomain("example.tld") .asBuilder() .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.com").createVKey())) .build()); @@ -423,7 +434,7 @@ void testPublishDomainExternalAndInBailiwickNameServer() throws Exception { .build()); persistResource( - DatabaseHelper.newDomain("example.tld") + newDomain("example.tld") .asBuilder() .addSubordinateHost("ns1.example.tld") .addNameservers( @@ -471,7 +482,7 @@ void testPublishDomainDeleteOrphanGlues() throws Exception { .build()); persistResource( - DatabaseHelper.newDomain("example.tld") + newDomain("example.tld") .asBuilder() .addSubordinateHost("ns1.example.tld") .addSubordinateHost("foo.example.tld") @@ -500,29 +511,20 @@ void testPublishDomainDeleteOrphanGlues() throws Exception { assertThatTotalUpdateSetsIs(update, 6); } - @MockitoSettings(strictness = Strictness.LENIENT) - @SuppressWarnings("AssertThrowsMultipleStatements") @Test void testPublishDomainFails_whenDnsUpdateReturnsError() throws Exception { Domain domain = - persistActiveDomain("example.tld") + newDomain("example.tld") .asBuilder() .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createVKey())) .build(); persistResource(domain); when(mockResolver.send(any(Message.class))).thenReturn(messageWithResponseCode(Rcode.SERVFAIL)); - VerifyException thrown = - assertThrows( - VerifyException.class, - () -> { - writer.publishDomain("example.tld"); - writer.commit(); - }); + writer.publishDomain("example.tld"); + VerifyException thrown = assertThrows(VerifyException.class, () -> writer.commit()); assertThat(thrown).hasMessageThat().contains("SERVFAIL"); } - @MockitoSettings(strictness = Strictness.LENIENT) - @SuppressWarnings("AssertThrowsMultipleStatements") @Test void testPublishHostFails_whenDnsUpdateReturnsError() throws Exception { Host host = @@ -532,26 +534,136 @@ void testPublishHostFails_whenDnsUpdateReturnsError() throws Exception { .build(); persistResource(host); when(mockResolver.send(any(Message.class))).thenReturn(messageWithResponseCode(Rcode.SERVFAIL)); - VerifyException thrown = - assertThrows( - VerifyException.class, - () -> { - writer.publishHost("ns1.example.tld"); - writer.commit(); - }); + writer.publishHost("ns1.example.tld"); + VerifyException thrown = assertThrows(VerifyException.class, () -> writer.commit()); assertThat(thrown).hasMessageThat().contains("SERVFAIL"); } - private static void assertThatUpdatedZoneIs(Update update, String zoneName) { + @Test + void testPublishDomainFails_whenResolverThrowsIoException() throws Exception { + Domain domain = + newDomain("example.tld") + .asBuilder() + .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createVKey())) + .build(); + persistResource(domain); + when(mockResolver.send(any(Message.class))).thenThrow(new IOException("connection refused")); + writer.publishDomain("example.tld"); + RuntimeException thrown = assertThrows(RuntimeException.class, () -> writer.commit()); + assertThat(thrown).hasMessageThat().contains("publishDomain failed for zone: tld"); + assertThat(thrown).hasCauseThat().isInstanceOf(IOException.class); + } + + @Test + void testProvideResolver_configuresTcpAndTimeout() { + Resolver resolver = DnsUpdateWriterModule.provideResolver("127.0.0.1", Duration.ofSeconds(15)); + assertThat(resolver).isInstanceOf(SimpleResolver.class); + SimpleResolver simpleResolver = (SimpleResolver) resolver; + assertThat(simpleResolver.getTCP()).isTrue(); + assertThat(simpleResolver.getTimeout()).isEqualTo(Duration.ofSeconds(15)); + assertThat(simpleResolver.getAddress().getHostString()).isEqualTo("127.0.0.1"); + assertThat(simpleResolver.getPort()).isEqualTo(SimpleResolver.DEFAULT_PORT); + } + + @Test + void testPublishDomain_withRealSimpleResolverOverLoopbackTcp() throws Exception { + Host host1 = persistActiveHost("ns1.example.tld"); + Host host2 = persistActiveHost("ns2.example.tld"); + persistResource( + newDomain("example.tld") + .asBuilder() + .setNameservers(ImmutableSet.of(host1.createVKey(), host2.createVKey())) + .build()); + + try (ServerSocket serverSocket = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) { + CompletableFuture receivedUpdateFuture = + serveSingleDnsUpdate(serverSocket, Opcode.UPDATE); + DnsUpdateWriter realWriter = createLoopbackWriter(serverSocket); + + realWriter.publishDomain("example.tld"); + realWriter.commit(); + + Message receivedUpdate = receivedUpdateFuture.get(10, TimeUnit.SECONDS); + assertThatUpdatedZoneIs(receivedUpdate, "tld."); + assertThatUpdateDeletes(receivedUpdate, "example.tld.", Type.ANY); + assertThatUpdateAdds( + receivedUpdate, + "example.tld.", + Type.NS, + Duration.ZERO.toSeconds(), + "ns1.example.tld.", + "ns2.example.tld."); + assertThatTotalUpdateSetsIs(receivedUpdate, 2); + } + } + + @Test + void testPublishDomain_withRealSimpleResolverOverLoopbackTcp_responseOpcodeMismatchFails() + throws Exception { + persistResource( + newDomain("example.tld") + .asBuilder() + .setNameservers(ImmutableSet.of(persistActiveHost("ns1.example.tld").createVKey())) + .build()); + + try (ServerSocket serverSocket = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) { + CompletableFuture receivedUpdateFuture = + serveSingleDnsUpdate(serverSocket, Opcode.QUERY); + DnsUpdateWriter realWriter = createLoopbackWriter(serverSocket); + + realWriter.publishDomain("example.tld"); + RuntimeException thrown = assertThrows(RuntimeException.class, () -> realWriter.commit()); + receivedUpdateFuture.get(10, TimeUnit.SECONDS); + assertThat(thrown).hasCauseThat().isInstanceOf(WireParseException.class); + assertThat(thrown).hasCauseThat().hasMessageThat().contains("opcode response is not UPDATE"); + } + } + + private DnsUpdateWriter createLoopbackWriter(ServerSocket serverSocket) { + Resolver realResolver = + DnsUpdateWriterModule.provideResolver("127.0.0.1", Duration.ofSeconds(10)); + realResolver.setPort(serverSocket.getLocalPort()); + return new DnsUpdateWriter( + "tld", Duration.ZERO, Duration.ZERO, Duration.ZERO, realResolver, clock); + } + + private static CompletableFuture serveSingleDnsUpdate( + ServerSocket serverSocket, int responseOpcode) { + return CompletableFuture.supplyAsync( + () -> { + try (Socket client = serverSocket.accept(); + DataInputStream in = new DataInputStream(client.getInputStream()); + DataOutputStream out = new DataOutputStream(client.getOutputStream())) { + int length = in.readUnsignedShort(); + byte[] queryBytes = new byte[length]; + in.readFully(queryBytes); + Message receivedUpdate = new Message(queryBytes); + + Message response = new Message(receivedUpdate.getHeader().getID()); + response.getHeader().setOpcode(responseOpcode); + response.getHeader().setFlag(Flags.QR); + response.getHeader().setRcode(Rcode.NOERROR); + byte[] responseBytes = response.toWire(); + out.writeShort(responseBytes.length); + out.write(responseBytes); + out.flush(); + return receivedUpdate; + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } + + private static void assertThatUpdatedZoneIs(Message update, String zoneName) { List zoneRecords = update.getSection(Section.ZONE); assertThat(zoneRecords.get(0).getName().toString()).isEqualTo(zoneName); } - private static void assertThatTotalUpdateSetsIs(Update update, int count) { + private static void assertThatTotalUpdateSetsIs(Message update, int count) { assertThat(update.getSectionRRsets(Section.UPDATE)).hasSize(count); } - private static void assertThatUpdateDeletes(Update update, String resourceName, int recordType) { + private static void assertThatUpdateDeletes(Message update, String resourceName, int recordType) { ImmutableList deleted = findUpdateRecords(update, resourceName, recordType); // There's only an empty (i.e. "delete") record. assertThat(deleted.get(0).rdataToString()).hasLength(0); @@ -559,7 +671,7 @@ private static void assertThatUpdateDeletes(Update update, String resourceName, } private static void assertThatUpdateAdds( - Update update, String resourceName, int recordType, Long ttl, String... resourceData) { + Message update, String resourceName, int recordType, Long ttl, String... resourceData) { ArrayList expectedData = new ArrayList<>(); Collections.addAll(expectedData, resourceData); @@ -572,7 +684,7 @@ private static void assertThatUpdateAdds( } private static ImmutableList findUpdateRecords( - Update update, String resourceName, int recordType) { + Message update, String resourceName, int recordType) { for (RRset set : update.getSectionRRsets(Section.UPDATE)) { if (set.getName().toString().equals(resourceName) && set.getType() == recordType) { return ImmutableList.copyOf(set.rrs());