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

Change-Id: I542061b862cf8d2296dacccd6171bbb6119c6b1e

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: [

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;
     }

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,

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__;
 

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));
+}

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