Certificate Transparency DNS log client

components/certificate_transparency/log_dns_client_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/certificate_transparency/log_dns_client.h"
+
+#include <algorithm>
+#include <numeric>
+#include <utility>
+#include <vector>
+
+#include "base/big_endian.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/run_loop.h"
+#include "base/sys_byteorder.h"
+#include "base/test/simple_test_clock.h"
+#include "base/test/test_timeouts.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "crypto/sha2.h"
+#include "net/base/net_errors.h"
+#include "net/cert/merkle_audit_proof.h"
+#include "net/cert/merkle_tree_leaf.h"
+#include "net/cert/signed_certificate_timestamp.h"
+#include "net/dns/dns_client.h"
+#include "net/dns/dns_config_service.h"
+#include "net/dns/dns_protocol.h"
+#include "net/log/test_net_log.h"
+#include "net/socket/socket_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace certificate_transparency {
+namespace {
+
+using ::testing::AllOf;
+using ::testing::DeleteArg;
+using ::testing::IsNull;
+using ::testing::Not;
+using ::testing::Pointee;
+using ::testing::Field;
+using ::testing::SizeIs;
+
+constexpr char kLeafHash[] =
+    "\x1f\x25\xe1\xca\xba\x4f\xf9\xb8\x27\x24\x83\x0f\xca\x60\xe4\xc2\xbe\xa8"
+    "\xc3\xa9\x44\x1c\x27\xb0\xb4\x3e\x6a\x96\x94\xc7\xb8\x04";
+
+MATCHER(IsNetOk, "") {
+  if (arg <= net::OK)
+    *result_listener << net::ErrorToString(arg);
+  return arg == net::OK;
+}
+
+MATCHER_P(IsNetError, expected, "") {
+  if (arg <= net::OK)
+    *result_listener << net::ErrorToString(arg);
+  return arg == expected;
+}
+
+// Always return min, to simplify testing.
+// This should result in the DNS query ID always being 0.
+int FakeRandInt(int min, int max) {
+  return min;
+}
+
+std::vector<char> CreateDnsTxtRequest(base::StringPiece qname) {
+  std::string encoded_qname;
+  EXPECT_TRUE(net::DNSDomainFromDot(qname, &encoded_qname));
+
+  const size_t query_section_size = encoded_qname.size() + 4;
+
+  std::vector<char> request(sizeof(net::dns_protocol::Header) +
+                            query_section_size);
+  base::BigEndianWriter writer(request.data(), request.size());
+
+  // Header
+  net::dns_protocol::Header header = {};
+  header.flags = base::HostToNet16(net::dns_protocol::kFlagRD);
+  header.qdcount = base::HostToNet16(1);
+  EXPECT_TRUE(writer.WriteBytes(&header, sizeof(header)));
+  // Query section
+  EXPECT_TRUE(writer.WriteBytes(encoded_qname.data(), encoded_qname.size()));
+  EXPECT_TRUE(writer.WriteU16(net::dns_protocol::kTypeTXT));
+  EXPECT_TRUE(writer.WriteU16(net::dns_protocol::kClassIN));
+  EXPECT_EQ(0, writer.remaining());
+
+  return request;
+}
+
+std::vector<char> CreateDnsTxtResponse(const std::vector<char>& request,
+                                       base::StringPiece answer) {
+  const size_t answers_section_size = 12 + answer.size();
+  constexpr uint32_t ttl = 86400;  // seconds
+
+  std::vector<char> response(request.size() + answers_section_size);
+  std::copy(request.begin(), request.end(), response.begin());
+  // Modify the header
+  net::dns_protocol::Header* header =
+      reinterpret_cast<net::dns_protocol::Header*>(response.data());
+  header->ancount = base::HostToNet16(1);
+  header->flags |= base::HostToNet16(net::dns_protocol::kFlagResponse);
+
+  // Write the answer section
+  base::BigEndianWriter writer(response.data() + request.size(),
+                               response.size() - request.size());
+  EXPECT_TRUE(writer.WriteU8(0xc0));  // qname is a pointer
+  EXPECT_TRUE(writer.WriteU8(
+      sizeof(*header)));  // address of qname (start of query section)
+  EXPECT_TRUE(writer.WriteU16(net::dns_protocol::kTypeTXT));
+  EXPECT_TRUE(writer.WriteU16(net::dns_protocol::kClassIN));
+  EXPECT_TRUE(writer.WriteU32(ttl));
+  EXPECT_TRUE(writer.WriteU16(answer.size()));
+  EXPECT_TRUE(writer.WriteBytes(answer.data(), answer.size()));
+  EXPECT_EQ(0, writer.remaining());
+
+  return response;
+}
+
+std::vector<char> CreateDnsErrorResponse(const std::vector<char>& request,
+                                         uint8_t rcode) {
+  std::vector<char> response(request);
+  // Modify the header
+  net::dns_protocol::Header* header =
+      reinterpret_cast<net::dns_protocol::Header*>(response.data());
+  header->ancount = base::HostToNet16(1);
+  header->flags |= base::HostToNet16(net::dns_protocol::kFlagResponse | rcode);
+
+  return response;
+}
+
+std::vector<std::string> GetSampleAuditProof(size_t length) {
+  std::vector<std::string> audit_proof(length);
+  // Makes each node of the audit proof different, so that tests are able to
+  // confirm that the audit proof is reconstructed in the correct order.
+  for (size_t i = 0; i < length; ++i) {
+    std::string node(crypto::kSHA256Length, '\0');
+    // Each node is 32 bytes, with each byte having a different value.
+    for (size_t j = 0; j < crypto::kSHA256Length; ++j) {
+      node[j] = static_cast<char>((-127 + i + j) % 128);
+    }
+    audit_proof[i].assign(std::move(node));
+  }
+
+  return audit_proof;
+}
+
+class MockLeafIndexCallback {
+ public:
+  MOCK_METHOD2(Run, void(int net_error, uint64_t leaf_index));
+};
+
+class MockAuditProofCallback {
+ public:
+  MOCK_METHOD2(Run_, void(int net_error, net::ct::MerkleAuditProof* proof));
+
+  // This proxy function is required because Chromium's GMock is not built with
+  // C++11 support (required for handling std::unique_ptr parameters).
+  // Remove it once this is rectified.
+  void Run(int net_error, std::unique_ptr<net::ct::MerkleAuditProof> proof) {
+    Run_(net_error, proof.release());
+  }
+};
+
+// A container for all of the data we need to keep alive for a mock socket.
+// This is useful because Mock{Read,Write}, SequencedSocketData and
+// MockClientSocketFactory all do not take ownership of or copy their arguments,
+// so we have to manage the lifetime of those arguments ourselves. Wrapping all
+// of that up in a single class simplifies this.
+class MockSocket {

[mmenke, 2016/06/27 17:01:08]

These aren't sockets.  Call them MockSocketData or somesuch?

[Rob Percival, 2016/06/27 19:01:18]

Done.

+ public:
+  // A socket that expects one write and one read operation.
+  MockSocket(const std::vector<char>& write, const std::vector<char>& read)
+      : expected_write_payload_(write),
+        expected_read_payload_(read),
+        expected_write_(net::SYNCHRONOUS,

[mmenke, 2016/06/27 17:01:08]

You should test net::ASYNC as well.  For reads, especially, it's the more common
result.

[Rob Percival, 2016/06/27 19:01:18]

Isn't that beyond the concern of LogDnsClient? It's using a DnsClient to handle
all the socket interaction, so the DnsClient tests (and tests of classes it
uses) should have that covered, shouldn't they? DnsClient shouldn't change its
external behaviour based on the whether the reads/writes are async or not.

[mmenke, 2016/06/27 19:26:20]

If DnsClient always returns asynchronously, regardless of whether reads/writes
return sync/async, you're completely correct, it doesn't matter.

+                        expected_write_payload_.data(),
+                        expected_write_payload_.size(),
+                        0),
+        expected_read_(net::SYNCHRONOUS,
+                       expected_read_payload_.data(),
+                       expected_read_payload_.size(),
+                       1),

[mmenke, 2016/06/27 17:01:07]

You can also simulate network errors for both reads and writes - I think you
should test both cases.  Not as likely as with TCP, but should be sure it works.

[Rob Percival, 2016/06/28 21:44:54]

Done.

+        socket_data_(&expected_read_, 1, &expected_write_, 1) {}
+
+  void AddToFactory(net::MockClientSocketFactory* socket_factory) {
+    socket_factory->AddSocketDataProvider(&socket_data_);
+  }
+
+ private:
+  const std::vector<char> expected_write_payload_;
+  const std::vector<char> expected_read_payload_;
+  net::MockWrite expected_write_;
+  net::MockRead expected_read_;
+  net::SequencedSocketData socket_data_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockSocket);
+};
+
+class LogDnsClientTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    thread_bundle_.reset(new content::TestBrowserThreadBundle(
+        content::TestBrowserThreadBundle::IO_MAINLOOP));
+    net_log_.reset(new net::BoundTestNetLog());
+    clock_.reset(new base::SimpleTestClock());
+
+    socket_factory_.reset(new net::MockClientSocketFactory());
+    mock_sockets_.clear();

[mmenke, 2016/06/27 17:01:07]

Not needed.  For each test, the test fixture is created from scratch, so this
does nothing.

Given that, I'd recommend you remove this line, and move all this stuff to the
constructor, in an initializer list (And make them no longer unique_ptrs).

[Rob Percival, 2016/06/27 19:01:18]

Done.

+  }
+
+  std::unique_ptr<net::DnsClient> CreateDnsClient() {
+    net::DnsConfig config;
+    // Use an invalid nameserver address. This prevents the tests accidentally
+    // sending real DNS queries. The mock sockets don't care that the address
+    // is invalid.
+    config.nameservers.push_back(net::IPEndPoint());
+    // Don't attempt retransmissions - just fail.
+    config.attempts = 1;
+    // This ensures timeouts are long enough for memory tests.
+    config.timeout = TestTimeouts::action_timeout();
+    // Simplify testing - don't require random numbers for the source port.
+    // This means our FakeRandInt function should only be called to get query
+    // IDs.
+    config.randomize_ports = false;
+
+    std::unique_ptr<net::DnsClient> client =
+        net::DnsClient::CreateClientForTesting(net_log_->bound().net_log(),
+                                               socket_factory_.get(),
+                                               base::Bind(&FakeRandInt));
+    client->SetConfig(config);
+    return client;
+  }
+
+  void ExpectRequestAndResponse(base::StringPiece qname,
+                                base::StringPiece answer) {
+    std::vector<char> request = CreateDnsTxtRequest(qname);
+    std::vector<char> response = CreateDnsTxtResponse(request, answer);
+
+    mock_sockets_.emplace_back(new MockSocket(request, response));
+    mock_sockets_.back()->AddToFactory(socket_factory_.get());
+  }
+
+  void ExpectRequestAndErrorResponse(base::StringPiece qname, uint8_t rcode) {
+    std::vector<char> request = CreateDnsTxtRequest(qname);
+    std::vector<char> response = CreateDnsErrorResponse(request, rcode);
+
+    mock_sockets_.emplace_back(new MockSocket(request, response));
+    mock_sockets_.back()->AddToFactory(socket_factory_.get());
+  }
+
+  void ExpectLeafIndexRequestAndResponse(base::StringPiece qname,
+                                         base::StringPiece leaf_index) {
+    // Prepend size to leaf_index to create the query answer (rdata)
+    ASSERT_LE(leaf_index.size(), 0xFFul);  // size must fit into a single byte
+    std::string answer = leaf_index.as_string();
+    answer.insert(answer.begin(), static_cast<char>(leaf_index.size()));
+
+    ExpectRequestAndResponse(qname, answer);
+  }
+
+  void ExpectAuditProofRequestAndResponse(
+      base::StringPiece qname,
+      std::vector<std::string>::const_iterator audit_path_start,
+      std::vector<std::string>::const_iterator audit_path_end) {
+    // Join nodes in the audit path into a single string.
+    std::string proof =
+        std::accumulate(audit_path_start, audit_path_end, std::string());
+
+    // Prepend size to proof to create the query answer (rdata)
+    ASSERT_LE(proof.size(), 0xFFul);  // size must fit into a single byte
+    proof.insert(proof.begin(), static_cast<char>(proof.size()));
+
+    ExpectRequestAndResponse(qname, proof);
+  }
+
+  void QueryLeafIndex(base::StringPiece log_domain,
+                      base::StringPiece leaf_hash,
+                      MockLeafIndexCallback* callback) {
+    std::unique_ptr<net::DnsClient> dns_client = CreateDnsClient();
+    LogDnsClient log_client(std::move(dns_client), net_log_->bound(),
+                            clock_.get());
+
+    LogDnsClient::LeafIndexCallback callback_func =
+        base::Bind(&MockLeafIndexCallback::Run, base::Unretained(callback));
+
+    log_client.QueryLeafIndex(log_domain, leaf_hash, callback_func);
+    base::RunLoop().RunUntilIdle();

[mmenke, 2016/06/27 17:01:08]

I don't think it's a great idea, rather than waiting until the callback function
is invoked.  I suggest using RunLoop::Run / RunLoop::Quit (Can bind the RunLoop
as an argument to MockLeafIndexCallback::Run).

[Rob Percival, 2016/06/27 19:01:18]

Why's that? With your approach, if a bug causes the callback to not be invoked,
each test case will hang until it times out. This approach should be reliable
(this is all run on a single thread). It also seems to be in line with what a
lot of other tests do (at least the ones I've come across).

[mmenke, 2016/06/27 19:26:20]

Just because tests do it, doesn't mean they should.

RunUntilIdle often adds implicit dependencies on behaviors that are not part of
the public API.  DnsClient makes no promises about doing all its work on the
current thread, for instance.  Changing the code to do stuff on another thread
would cause issues.  Unlikely here, but I've run into the issue when refactoring
other net/ APIs that also had seemed unlikely to switch to doing stuff on
another thread.

Also, code is often unable to fully validate APIs when it uses RunUntilIdle - it
can only check the state of objects after the callback is invoked, and then all
pending tasks are executed.  net/ has a ton of cache tests, for instance, that
only pass because they spin the message loop after the callback the test is
actually using is complete.  Is the state of the world consistent during the
time the callback is actually invoked?  The tests can't actually validate that,
because of the extra spinning.  It looks like gmock helps you get around some of
this, in these tests, but...see other comment on gmock.  :)

Also worth noting that RunUntilIdle can also never terminate, if the message
loop never becomes idle (Less likely than Run hanging, but it doesn't eliminate
the issue).  Note that if a number of tests hang, a tryjob will be terminated
early, so you don't have all bots hung for the next 2 days (This didn't used to
be the case, and it caused a lot of issues at one point).

[Rob Percival, 2016/06/27 20:12:11]

It's also the way TestBrowserThreadBundle gives as an example
(https://cs.chromium.org/chromium/src/content/public/test/test_browser_thread_...).
Is it possible for DnsClient to do anything on another thread? There are no
other threads in existence. TestBrowserThreadBundle runs all tasks, regardless
of what browser thread they were posted to, on the thread that calls
RunLoop().RunUntilIdle(). I can't imagine DnsClient ever creating its own
threads either, but maybe I just lack imagination ;)

It seems like this is good advice for cases where there are multiple threads
and/or sub-optimal callback testing. However, in this single-threaded case with
GMock, it should be redundant and just unnecessarily complicate the tests. I
suppose there's an argument for doing it here just so that others won't copy
this technique and use it in those situations where it isn't as suitable
though..

[mmenke, 2016/06/27 20:24:45]

I think it's unlikely, but remember that the WorkerPool is a global thread pool.
 SimpleUrlRequestJob used to be a single-threaded class, but then it turned out
consumers were using its "memcpy" to copy data from memory mapped files, which
caused it to do file I/O on the IO thread, so we switched to using
WorkerPools...breaking unittests that really didn't need to care if it was done
synchronously or not.

So modifying the API to behave in a way such that it still adheres to spec would
break this code.  Meaning the hapless network stack team member doing that would
have to dig into this test and fix it.
If you want to use this pattern, feel free - I've said why I think it's a bad
pattern, but not going to push back any more on it.

[Rob Percival, 2016/06/28 21:44:54]

Done.

+  }
+
+  void QueryAuditProof(base::StringPiece log_domain,
+                       uint64_t leaf_index,
+                       uint64_t tree_size,
+                       MockAuditProofCallback* callback) {
+    std::unique_ptr<net::DnsClient> dns_client = CreateDnsClient();
+    LogDnsClient log_client(std::move(dns_client), net_log_->bound(),
+                            clock_.get());
+
+    LogDnsClient::AuditProofCallback callback_func =
+        base::Bind(&MockAuditProofCallback::Run, base::Unretained(callback));
+
+    log_client.QueryAuditProof(log_domain, leaf_index, tree_size,
+                               callback_func);
+    base::RunLoop().RunUntilIdle();
+  }
+
+  std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle_;
+  std::unique_ptr<net::BoundTestNetLog> net_log_;

[mmenke, 2016/06/27 17:04:09]

Also, you don't need this - just pass in a net::BoundNetLog(), and everything
will work fine.

BoundTestNetLog is just needed if you want to peer at logged network events.

[mmenke, 2016/06/27 17:05:02]

(Also, pass in nullptr when you need a NetLog instead of a BoundNetLog)

[Rob Percival, 2016/06/27 19:01:18]

Done.

+  std::unique_ptr<base::SimpleTestClock> clock_;
+  std::vector<std::unique_ptr<MockSocket>> mock_sockets_;
+  std::unique_ptr<net::MockClientSocketFactory> socket_factory_;
+};
+
+TEST_F(LogDnsClientTest, QueryLeafIndex) {
+  ExpectLeafIndexRequestAndResponse(
+      "D4S6DSV2J743QJZEQMH4UYHEYK7KRQ5JIQOCPMFUHZVJNFGHXACA.hash.ct.test.",
+      "123456");
+
+  MockLeafIndexCallback callback;

[mmenke, 2016/06/27 17:01:08]

The chrome wide preference is not to use gmock, to make hacking on Chrome easier
for new developers.  Looks like it's fairly straightforward to just switch to
something else for all of these.

[Rob Percival, 2016/06/27 19:01:18]

As it happens, I used to have some hand-rolled mocks until patch set 6
(https://codereview.chromium.org/2066553002/diff2/80001:100001/components/cert...).
GMock is definitely more concise in this case though and I don't think it's
unreasonable to expect new developers to learn the basics. Are GMock matchers
included in that preference, or is it just mocks themselves?

[mmenke, 2016/06/27 19:26:20]

Clarity is generally better than being concise.  Here's the discussion on the
topic:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/-KH_IP0rIWQ

Consensus may be a bit weaker than I indicated, but I've continued to see a
bunch of reviewers call it out, and I also find it much less readable.  The lack
of support for unique_ptrs also makes things look leaky.

[Rob Percival, 2016/06/27 20:12:11]

That lack of support for unique_ptrs is indeed unfortunate. GMock does support
them but some of the build bots are using versions of libc++ with incomplete
C++11 support (missing some type traits), so GMock with C++11 support enabled
fails to compile :(

Very well, if you dislike GMock that much then I'll revert to using the
hand-rolled mocks I had before.

[Rob Percival, 2016/06/28 21:44:54]

Done.

+  EXPECT_CALL(callback, Run(IsNetOk(), 123456));
+
+  QueryLeafIndex("ct.test", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsThatLogDomainDoesNotExist) {
+  ExpectRequestAndErrorResponse(
+      "D4S6DSV2J743QJZEQMH4UYHEYK7KRQ5JIQOCPMFUHZVJNFGHXACA.hash.ct.test.",
+      net::dns_protocol::kRcodeNXDOMAIN);
+
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_NAME_NOT_RESOLVED), 0));
+
+  QueryLeafIndex("ct.test", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsServerFailure) {
+  ExpectRequestAndErrorResponse(
+      "D4S6DSV2J743QJZEQMH4UYHEYK7KRQ5JIQOCPMFUHZVJNFGHXACA.hash.ct.test.",
+      net::dns_protocol::kRcodeSERVFAIL);
+
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_DNS_SERVER_FAILED), 0));
+
+  QueryLeafIndex("ct.test", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsServerRefusal) {
+  ExpectRequestAndErrorResponse(
+      "D4S6DSV2J743QJZEQMH4UYHEYK7KRQ5JIQOCPMFUHZVJNFGHXACA.hash.ct.test.",
+      net::dns_protocol::kRcodeREFUSED);
+
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_DNS_SERVER_FAILED), 0));
+
+  QueryLeafIndex("ct.test", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest,
+       QueryLeafIndexReportsMalformedResponseIfLeafIndexIsNotNumeric) {
+  ExpectLeafIndexRequestAndResponse(
+      "D4S6DSV2J743QJZEQMH4UYHEYK7KRQ5JIQOCPMFUHZVJNFGHXACA.hash.ct.test.",
+      "foo");
+
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_DNS_MALFORMED_RESPONSE), 0));
+
+  QueryLeafIndex("ct.test", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest,
+       QueryLeafIndexReportsMalformedResponseIfLeafIndexIsFloatingPoint) {
+  ExpectLeafIndexRequestAndResponse(
+      "D4S6DSV2J743QJZEQMH4UYHEYK7KRQ5JIQOCPMFUHZVJNFGHXACA.hash.ct.test.",
+      "123456.0");
+
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_DNS_MALFORMED_RESPONSE), 0));
+
+  QueryLeafIndex("ct.test", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest,
+       QueryLeafIndexReportsMalformedResponseIfLeafIndexIsEmpty) {
+  ExpectLeafIndexRequestAndResponse(
+      "D4S6DSV2J743QJZEQMH4UYHEYK7KRQ5JIQOCPMFUHZVJNFGHXACA.hash.ct.test.", "");
+
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_DNS_MALFORMED_RESPONSE), 0));
+
+  QueryLeafIndex("ct.test", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsInvalidArgIfLogDomainIsEmpty) {
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_INVALID_ARGUMENT), 0));
+
+  QueryLeafIndex("", kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsInvalidArgIfLogDomainIsNull) {
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_INVALID_ARGUMENT), 0));
+
+  QueryLeafIndex(nullptr, kLeafHash, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsInvalidArgIfLeafHashIsInvalid) {
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_INVALID_ARGUMENT), 0));
+
+  QueryLeafIndex("ct.test", "foo", &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsInvalidArgIfLeafHashIsEmpty) {
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_INVALID_ARGUMENT), 0));
+
+  QueryLeafIndex("ct.test", "", &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryLeafIndexReportsInvalidArgIfLeafHashIsNull) {
+  MockLeafIndexCallback callback;
+  EXPECT_CALL(callback, Run(IsNetError(net::ERR_INVALID_ARGUMENT), 0));
+
+  QueryLeafIndex("ct.test", nullptr, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryAuditProof) {
+  const std::vector<std::string> audit_proof = GetSampleAuditProof(20);
+
+  // It should require 3 queries to collect the entire audit proof, as there is
+  // only space for 7 nodes per UDP packet.
+  ExpectAuditProofRequestAndResponse("0.123456.999999.tree.ct.test.",
+                                     audit_proof.begin(),
+                                     audit_proof.begin() + 7);
+  ExpectAuditProofRequestAndResponse("7.123456.999999.tree.ct.test.",
+                                     audit_proof.begin() + 7,
+                                     audit_proof.begin() + 14);
+  ExpectAuditProofRequestAndResponse("14.123456.999999.tree.ct.test.",
+                                     audit_proof.begin() + 14,
+                                     audit_proof.end());
+
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback,
+              Run_(IsNetOk(),
+                   Pointee(AllOf(
+                       Field(&net::ct::MerkleAuditProof::nodes, audit_proof),
+                       Field(&net::ct::MerkleAuditProof::leaf_index, 123456)))))
+      .WillOnce(DeleteArg<1>());
+
+  QueryAuditProof("ct.test", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest,
+       QueryAuditProofHandlesResponsesWithEmptyAndShortAuditPaths) {
+  const std::vector<std::string> audit_proof = GetSampleAuditProof(20);
+
+  // Make some of the responses empty or contain fewer audit proof nodes than
+  // they can hold.
+  ExpectAuditProofRequestAndResponse("0.123456.999999.tree.ct.test.",
+                                     audit_proof.begin(), audit_proof.begin());
+  ExpectAuditProofRequestAndResponse("0.123456.999999.tree.ct.test.",
+                                     audit_proof.begin(),
+                                     audit_proof.begin() + 1);
+  ExpectAuditProofRequestAndResponse("1.123456.999999.tree.ct.test.",
+                                     audit_proof.begin() + 1,
+                                     audit_proof.begin() + 3);
+  ExpectAuditProofRequestAndResponse("3.123456.999999.tree.ct.test.",
+                                     audit_proof.begin() + 3,
+                                     audit_proof.begin() + 6);
+  ExpectAuditProofRequestAndResponse("6.123456.999999.tree.ct.test.",
+                                     audit_proof.begin() + 6,
+                                     audit_proof.begin() + 10);
+  ExpectAuditProofRequestAndResponse("10.123456.999999.tree.ct.test.",
+                                     audit_proof.begin() + 10,
+                                     audit_proof.begin() + 13);
+  ExpectAuditProofRequestAndResponse("13.123456.999999.tree.ct.test.",
+                                     audit_proof.begin() + 13,
+                                     audit_proof.end());
+
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback,
+              Run_(IsNetOk(),
+                   Pointee(AllOf(
+                       Field(&net::ct::MerkleAuditProof::nodes, audit_proof),
+                       Field(&net::ct::MerkleAuditProof::leaf_index, 123456)))))
+      .WillOnce(DeleteArg<1>());
+
+  QueryAuditProof("ct.test", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryAuditProofReportsThatLogDomainDoesNotExist) {
+  ExpectRequestAndErrorResponse("0.123456.999999.tree.ct.test.",
+                                net::dns_protocol::kRcodeNXDOMAIN);
+
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback, Run_(IsNetError(net::ERR_NAME_NOT_RESOLVED), nullptr));
+
+  QueryAuditProof("ct.test", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryAuditProofReportsServerFailure) {
+  ExpectRequestAndErrorResponse("0.123456.999999.tree.ct.test.",
+                                net::dns_protocol::kRcodeSERVFAIL);
+
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback, Run_(IsNetError(net::ERR_DNS_SERVER_FAILED), nullptr));
+
+  QueryAuditProof("ct.test", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryAuditProofReportsServerRefusal) {
+  ExpectRequestAndErrorResponse("0.123456.999999.tree.ct.test.",
+                                net::dns_protocol::kRcodeREFUSED);
+
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback, Run_(IsNetError(net::ERR_DNS_SERVER_FAILED), nullptr));
+
+  QueryAuditProof("ct.test", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest,
+       QueryAuditProofReportsResponseMalformedIfNodeTooShort) {
+  // node is shorter than a SHA-256 hash (31 vs 32 bytes)
+  const std::vector<std::string> audit_proof(1, std::string(31, 'a'));
+
+  ExpectAuditProofRequestAndResponse("0.123456.999999.tree.ct.test.",
+                                     audit_proof.begin(), audit_proof.end());
+
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback,
+              Run_(IsNetError(net::ERR_DNS_MALFORMED_RESPONSE), nullptr));
+
+  QueryAuditProof("ct.test", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryAuditProofReportsResponseMalformedIfNodeTooLong) {
+  // node is longer than a SHA-256 hash (33 vs 32 bytes)
+  const std::vector<std::string> audit_proof(1, std::string(33, 'a'));
+
+  ExpectAuditProofRequestAndResponse("0.123456.999999.tree.ct.test.",
+                                     audit_proof.begin(), audit_proof.end());
+
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback,
+              Run_(IsNetError(net::ERR_DNS_MALFORMED_RESPONSE), nullptr));
+
+  QueryAuditProof("ct.test", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryAuditProofReportsInvalidArgIfLogDomainIsEmpty) {
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback, Run_(IsNetError(net::ERR_INVALID_ARGUMENT), nullptr));
+
+  QueryAuditProof("", 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest, QueryAuditProofReportsInvalidArgIfLogDomainIsNull) {
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback, Run_(IsNetError(net::ERR_INVALID_ARGUMENT), nullptr));
+
+  QueryAuditProof(nullptr, 123456, 999999, &callback);
+}
+
+TEST_F(LogDnsClientTest,
+       QueryAuditProofReportInvalidArgIfLeafIndexEqualToTreeSize) {
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback, Run_(IsNetError(net::ERR_INVALID_ARGUMENT), nullptr));
+
+  QueryAuditProof("ct.test", 123456, 123456, &callback);
+}
+
+TEST_F(LogDnsClientTest,
+       QueryAuditProofReportInvalidArgIfLeafIndexGreaterThanTreeSize) {
+  MockAuditProofCallback callback;
+  EXPECT_CALL(callback, Run_(IsNetError(net::ERR_INVALID_ARGUMENT), nullptr));
+
+  QueryAuditProof("ct.test", 999999, 123456, &callback);
+}
+
+}  // namespace
+}  // namespace certificate_transparency