add a version TXT record to adb secure mdns services

In the context of secure connect, allows adbd and host adb to reject
each other based on incompatible versions without even having to
actually connect (since it is a DNS TXT).

Bug: 111434128, 119490749

Test: N/A
Exempt-From-Owner-Approval: already approved
Change-Id: I54312d8b67370c397ba81ecdbca1b27e3ee58572
This commit is contained in:
Lingfeng Yang 2019-10-24 22:30:11 -07:00 committed by Joshua Duong
parent 615036f494
commit 39e54b8c5b
3 changed files with 144 additions and 16 deletions

View file

@ -27,12 +27,62 @@ const int kADBTransportServiceRefIndex = 0;
const int kADBSecurePairingServiceRefIndex = 1;
const int kADBSecureConnectServiceRefIndex = 2;
// Each ADB Secure service advertises with a TXT record indicating the version
// using a key/value pair per RFC 6763 (https://tools.ietf.org/html/rfc6763).
//
// The first key/value pair is always the version of the protocol.
// There may be more key/value pairs added after.
//
// The version is purposely represented as the single letter "v" due to the
// need to minimize DNS traffic. The version starts at 1. With each breaking
// protocol change, the version is incremented by 1.
//
// Newer adb clients/daemons need to recognize and either reject
// or be backward-compatible with older verseions if there is a mismatch.
//
// Relevant sections:
//
// """
// 6.4. Rules for Keys in DNS-SD Key/Value Pairs
//
// The key MUST be at least one character. DNS-SD TXT record strings
// beginning with an '=' character (i.e., the key is missing) MUST be
// silently ignored.
//
// ...
//
// 6.5. Rules for Values in DNS-SD Key/Value Pairs
//
// If there is an '=' in a DNS-SD TXT record string, then everything
// after the first '=' to the end of the string is the value. The value
// can contain any eight-bit values including '='.
// """
#define ADB_SECURE_SERVICE_VERSION_TXT_RECORD(ver) ("v=" #ver)
// Client/service versions are initially defined to be matching,
// but may go out of sync as different clients and services
// try to talk to each other.
#define ADB_SECURE_SERVICE_VERSION 1
#define ADB_SECURE_CLIENT_VERSION ADB_SECURE_SERVICE_VERSION
const char* kADBSecurePairingServiceTxtRecord =
ADB_SECURE_SERVICE_VERSION_TXT_RECORD(ADB_SECURE_SERVICE_VERSION);
const char* kADBSecureConnectServiceTxtRecord =
ADB_SECURE_SERVICE_VERSION_TXT_RECORD(ADB_SECURE_SERVICE_VERSION);
const char* kADBDNSServices[] = {
kADBServiceType,
kADBSecurePairingServiceType,
kADBSecureConnectServiceType,
};
const char* kADBDNSServiceTxtRecords[] = {
nullptr,
kADBSecurePairingServiceTxtRecord,
kADBSecureConnectServiceTxtRecord,
};
const int kNumADBDNSServices = arraysize(kADBDNSServices);
#endif

View file

@ -111,13 +111,14 @@ class ResolvedService : public AsyncServiceRef {
virtual ~ResolvedService() = default;
ResolvedService(std::string serviceName, std::string regType, uint32_t interfaceIndex,
const char* hosttarget, uint16_t port)
const char* hosttarget, uint16_t port, int version)
: serviceName_(serviceName),
regType_(regType),
hosttarget_(hosttarget),
port_(port),
sa_family_(0),
ip_addr_data_(NULL) {
ip_addr_data_(NULL),
serviceVersion_(version) {
memset(ip_addr_, 0, sizeof(ip_addr_));
/* TODO: We should be able to get IPv6 support by adding
@ -137,6 +138,8 @@ class ResolvedService : public AsyncServiceRef {
} else {
Initialize();
}
D("Client version: %d Service version: %d\n", clientVersion_, serviceVersion_);
}
void Connect(const sockaddr* address) {
@ -204,6 +207,7 @@ class ResolvedService : public AsyncServiceRef {
adb_secure_foreach_service_callback cb);
private:
int clientVersion_ = ADB_SECURE_CLIENT_VERSION;
std::string serviceName_;
std::string regType_;
std::string hosttarget_;
@ -211,6 +215,7 @@ class ResolvedService : public AsyncServiceRef {
int sa_family_;
const void* ip_addr_data_;
char ip_addr_[INET6_ADDRSTRLEN];
int serviceVersion_;
};
// static
@ -327,16 +332,55 @@ class DiscoveredService : public AsyncServiceRef {
std::string regType_;
};
static void DNSSD_API register_resolved_mdns_service(DNSServiceRef sdRef,
DNSServiceFlags flags,
uint32_t interfaceIndex,
DNSServiceErrorType errorCode,
const char* fullname,
const char* hosttarget,
uint16_t port,
uint16_t /*txtLen*/,
const unsigned char* /*txtRecord*/,
void* context) {
// Returns the version the device wanted to advertise,
// or -1 if parsing fails.
static int parse_version_from_txt_record(uint16_t txtLen, const unsigned char* txtRecord) {
if (!txtLen) return -1;
if (!txtRecord) return -1;
// https://tools.ietf.org/html/rfc6763
// """
// 6.1. General Format Rules for DNS TXT Records
//
// A DNS TXT record can be up to 65535 (0xFFFF) bytes long. The total
// length is indicated by the length given in the resource record header
// in the DNS message. There is no way to tell directly from the data
// alone how long it is (e.g., there is no length count at the start, or
// terminating NULL byte at the end).
// """
// Let's trust the TXT record's length byte
// Worst case, it wastes 255 bytes
std::vector<char> recordAsString(txtLen + 1, '\0');
char* str = recordAsString.data();
memcpy(str, txtRecord + 1 /* skip the length byte */, txtLen);
// Check if it's the version key
static const char* versionKey = "v=";
size_t versionKeyLen = strlen(versionKey);
if (strncmp(versionKey, str, versionKeyLen)) return -1;
auto valueStart = str + versionKeyLen;
long parsedNumber = strtol(valueStart, 0, 10);
// No valid conversion. Also, 0
// is not a valid version.
if (!parsedNumber) return -1;
// Outside bounds of long.
if (parsedNumber == LONG_MIN || parsedNumber == LONG_MAX) return -1;
// Possibly valid version
return static_cast<int>(parsedNumber);
}
static void DNSSD_API register_resolved_mdns_service(
DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t interfaceIndex,
DNSServiceErrorType errorCode, const char* fullname, const char* hosttarget, uint16_t port,
uint16_t txtLen, const unsigned char* txtRecord, void* context) {
D("Resolved a service.");
std::unique_ptr<DiscoveredService> discovered(
reinterpret_cast<DiscoveredService*>(context));
@ -346,8 +390,14 @@ static void DNSSD_API register_resolved_mdns_service(DNSServiceRef sdRef,
return;
}
// TODO: Reject certain combinations of invalid or mismatched client and
// service versions here before creating anything.
// At the moment, there is nothing to reject, so accept everything
// as an optimistic default.
auto serviceVersion = parse_version_from_txt_record(txtLen, txtRecord);
auto resolved = new ResolvedService(discovered->ServiceName(), discovered->RegType(),
interfaceIndex, hosttarget, ntohs(port));
interfaceIndex, hosttarget, ntohs(port), serviceVersion);
if (! resolved->Initialized()) {
delete resolved;

View file

@ -67,9 +67,37 @@ static void register_mdns_service(int index, int port) {
std::string hostname = "adb-";
hostname += android::base::GetProperty("ro.serialno", "unidentified");
auto error = DNSServiceRegister(&mdns_refs[index], 0, 0, hostname.c_str(),
kADBDNSServices[index], nullptr, nullptr,
htobe16((uint16_t)port), 0, nullptr, mdns_callback, nullptr);
// https://tools.ietf.org/html/rfc6763
// """
// The format of the data within a DNS TXT record is one or more
// strings, packed together in memory without any intervening gaps or
// padding bytes for word alignment.
//
// The format of each constituent string within the DNS TXT record is a
// single length byte, followed by 0-255 bytes of text data.
// """
//
// Therefore:
// 1. Begin with the string length
// 2. No null termination
std::vector<char> txtRecord;
if (kADBDNSServiceTxtRecords[index]) {
size_t txtRecordStringLength = strlen(kADBDNSServiceTxtRecords[index]);
txtRecord.resize(1 + // length byte
txtRecordStringLength // string bytes
);
txtRecord[0] = (char)txtRecordStringLength;
memcpy(txtRecord.data() + 1, kADBDNSServiceTxtRecords[index], txtRecordStringLength);
}
auto error = DNSServiceRegister(
&mdns_refs[index], 0, 0, hostname.c_str(), kADBDNSServices[index], nullptr, nullptr,
htobe16((uint16_t)port), (uint16_t)txtRecord.size(),
txtRecord.empty() ? nullptr : txtRecord.data(), mdns_callback, nullptr);
if (error != kDNSServiceErr_NoError) {
LOG(ERROR) << "Could not register mDNS service " << kADBDNSServices[index] << ", error ("