Merge "adb: change mdns tls service names (RFC 6763)."

6 files changed, 129 insertions, 12 deletions

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 <android-base/macros.h>
 
-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 <gtest/gtest.h>
+
+#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