diff --git a/adb/Android.bp b/adb/Android.bp index 6386a78ed..19e921f8c 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -695,6 +695,7 @@ cc_test { "daemon/shell_service_test.cpp", "shell_service_protocol.cpp", "shell_service_protocol_test.cpp", + "mdns_test.cpp", ], shared_libs: [ diff --git a/adb/adb.cpp b/adb/adb.cpp index 08d3904c7..06fdb69a1 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -149,7 +149,7 @@ void print_packet(const char *label, apacket *p) case A_WRTE: tag = "WRTE"; break; case A_AUTH: tag = "AUTH"; break; case A_STLS: - tag = "ATLS"; + tag = "STLS"; break; default: tag = "????"; break; } diff --git a/adb/adb_mdns.h b/adb/adb_mdns.h index 6b373553a..31112485b 100644 --- a/adb/adb_mdns.h +++ b/adb/adb_mdns.h @@ -19,9 +19,14 @@ #include -const char* kADBServiceType = "_adb._tcp"; -const char* kADBSecurePairingServiceType = "_adb_secure_pairing._tcp"; -const char* kADBSecureConnectServiceType = "_adb_secure_connect._tcp"; +// The rules for Service Names [RFC6335] state that they may be no more +// than fifteen characters long (not counting the mandatory underscore), +// consisting of only letters, digits, and hyphens, must begin and end +// with a letter or digit, must not contain consecutive hyphens, and +// must contain at least one letter. +#define ADB_MDNS_SERVICE_TYPE "adb" +#define ADB_MDNS_TLS_PAIRING_TYPE "adb-tls-pairing" +#define ADB_MDNS_TLS_CONNECT_TYPE "adb-tls-connect" const int kADBTransportServiceRefIndex = 0; const int kADBSecurePairingServiceRefIndex = 1; @@ -71,11 +76,10 @@ const char* kADBSecurePairingServiceTxtRecord = const char* kADBSecureConnectServiceTxtRecord = ADB_SECURE_SERVICE_VERSION_TXT_RECORD(ADB_SECURE_SERVICE_VERSION); -const char* kADBDNSServices[] = { - kADBServiceType, - kADBSecurePairingServiceType, - kADBSecureConnectServiceType, -}; +#define ADB_FULL_MDNS_SERVICE_TYPE(atype) ("_" atype "._tcp") +const char* kADBDNSServices[] = {ADB_FULL_MDNS_SERVICE_TYPE(ADB_MDNS_SERVICE_TYPE), + ADB_FULL_MDNS_SERVICE_TYPE(ADB_MDNS_TLS_PAIRING_TYPE), + ADB_FULL_MDNS_SERVICE_TYPE(ADB_MDNS_TLS_CONNECT_TYPE)}; const char* kADBDNSServiceTxtRecords[] = { nullptr, diff --git a/adb/client/auth.cpp b/adb/client/auth.cpp index 4b2fa0423..b674a8161 100644 --- a/adb/client/auth.cpp +++ b/adb/client/auth.cpp @@ -504,6 +504,12 @@ void adb_auth_tls_handshake(atransport* t) { }).detach(); } +// Callback given to SSL_set_cert_cb to select a certificate when server requests +// for a certificate. This is where the server will give us a CA-issuer list, and +// figure out if the server knows any of our public keys. We currently always return +// 1 here to indicate success, since we always try a key here (in the case of no auth). +// See https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_set_cert_cb +// for more details. int adb_tls_set_certificate(SSL* ssl) { LOG(INFO) << __func__; diff --git a/adb/mdns_test.cpp b/adb/mdns_test.cpp new file mode 100644 index 000000000..1f662c1ca --- /dev/null +++ b/adb/mdns_test.cpp @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * 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. + */ + +#include + +#include "adb_mdns.h" + +static bool isValidMdnsServiceName(std::string_view name) { + // The rules for Service Names [RFC6335] state that they may be no more + // than fifteen characters long (not counting the mandatory underscore), + // consisting of only letters, digits, and hyphens, must begin and end + // with a letter or digit, must not contain consecutive hyphens, and + // must contain at least one letter. + + // No more than 15 characters long + if (name.empty() || name.size() > 15) { + return false; + } + + bool hasAtLeastOneLetter = false; + bool sawHyphen = false; + for (size_t i = 0; i < name.size(); ++i) { + // Must contain at least one letter + // Only contains letters, digits and hyphens + if (name[i] == '-') { + // Cannot be at beginning or end + if (i == 0 || i == name.size() - 1) { + return false; + } + if (sawHyphen) { + // Consecutive hyphen found + return false; + } + sawHyphen = true; + continue; + } + + sawHyphen = false; + if ((name[i] >= 'a' && name[i] <= 'z') || (name[i] >= 'A' && name[i] <= 'Z')) { + hasAtLeastOneLetter = true; + continue; + } + + if (name[i] >= '0' && name[i] <= '9') { + continue; + } + + // Invalid character + return false; + } + + return hasAtLeastOneLetter; +} + +TEST(mdns, test_isValidMdnsServiceName) { + // Longer than 15 characters + EXPECT_FALSE(isValidMdnsServiceName("abcd1234abcd1234")); + + // Contains invalid characters + EXPECT_FALSE(isValidMdnsServiceName("a*a")); + EXPECT_FALSE(isValidMdnsServiceName("a_a")); + EXPECT_FALSE(isValidMdnsServiceName("_a")); + + // Does not begin or end with letter or digit + EXPECT_FALSE(isValidMdnsServiceName("")); + EXPECT_FALSE(isValidMdnsServiceName("-")); + EXPECT_FALSE(isValidMdnsServiceName("-a")); + EXPECT_FALSE(isValidMdnsServiceName("-1")); + EXPECT_FALSE(isValidMdnsServiceName("a-")); + EXPECT_FALSE(isValidMdnsServiceName("1-")); + + // Contains consecutive hyphens + EXPECT_FALSE(isValidMdnsServiceName("a--a")); + + // Does not contain at least one letter + EXPECT_FALSE(isValidMdnsServiceName("1")); + EXPECT_FALSE(isValidMdnsServiceName("12")); + EXPECT_FALSE(isValidMdnsServiceName("1-2")); + + // Some valid names + EXPECT_TRUE(isValidMdnsServiceName("a")); + EXPECT_TRUE(isValidMdnsServiceName("a1")); + EXPECT_TRUE(isValidMdnsServiceName("1A")); + EXPECT_TRUE(isValidMdnsServiceName("aZ")); + EXPECT_TRUE(isValidMdnsServiceName("a-Z")); + EXPECT_TRUE(isValidMdnsServiceName("a-b-Z")); + EXPECT_TRUE(isValidMdnsServiceName("abc-def-123-456")); +} + +TEST(mdns, ServiceName_RFC6335) { + EXPECT_TRUE(isValidMdnsServiceName(ADB_MDNS_SERVICE_TYPE)); + EXPECT_TRUE(isValidMdnsServiceName(ADB_MDNS_TLS_PAIRING_TYPE)); + EXPECT_TRUE(isValidMdnsServiceName(ADB_MDNS_TLS_CONNECT_TYPE)); +} diff --git a/adb/transport.cpp b/adb/transport.cpp index 963c3c194..25ed36654 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -505,11 +505,10 @@ bool FdConnection::DoTlsHandshake(RSA* key, std::string* auth_key) { int osh = cast_handle_to_int(adb_get_os_handle(fd_)); #if ADB_HOST - tls_ = TlsConnection::Create(TlsConnection::Role::Client, + tls_ = TlsConnection::Create(TlsConnection::Role::Client, x509_str, evp_str, osh); #else - tls_ = TlsConnection::Create(TlsConnection::Role::Server, + tls_ = TlsConnection::Create(TlsConnection::Role::Server, x509_str, evp_str, osh); #endif - x509_str, evp_str, osh); CHECK(tls_); #if ADB_HOST // TLS 1.3 gives the client no message if the server rejected the