Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

Commit 2dc4cab

Browse files
jmgaojmgao
committed
adb: stop using adbkey.pub.
An adbkey/adbkey.pub pair that doesn't match up results in a hard-to-diagnose scenario where "Always allow from this computer" doesn't work. The private key contains all of the information that's in the public key, so just extract the public key out of the private key instead of storing them separately. Bug: http://b/119634232 Test: rm ~/.android/adbkey.pub; adb kill-server; adb shell true Test: rm ~/.android/adbkey*; adb kill-server; adb shell true Change-Id: I0ae9033dbcd119c12cfb2b3977f1f1954ac800c1
1 parent c55fab4 commit 2dc4cab

File tree

3 files changed

+57
-75
lines changed

3 files changed

+57
-75
lines changed

‎adb/adb_auth.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
void adb_auth_init();
3737

3838
int adb_auth_keygen(const char* filename);
39+
int adb_auth_pubkey(const char* filename);
3940
std::string adb_auth_get_userkey();
4041
std::deque<std::shared_ptr<RSA>> adb_auth_get_private_keys();
4142

‎adb/client/auth.cpp‎

Lines changed: 49 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343

4444
#include "adb.h"
4545
#include "adb_auth.h"
46+
#include "adb_io.h"
4647
#include "adb_utils.h"
4748
#include "sysdeps.h"
4849
#include "transport.h"
@@ -52,30 +53,7 @@ static std::map<std::string, std::shared_ptr<RSA>>& g_keys =
5253
*new std::map<std::string, std::shared_ptr<RSA>>;
5354
static std::map<int, std::string>& g_monitored_paths = *new std::map<int, std::string>;
5455

55-
static std::string get_user_info() {
56-
LOG(INFO) << "get_user_info...";
57-
58-
std::string hostname;
59-
if (getenv("HOSTNAME")) hostname = getenv("HOSTNAME");
60-
#if !defined(_WIN32)
61-
char buf[64];
62-
if (hostname.empty() && gethostname(buf, sizeof(buf)) != -1) hostname = buf;
63-
#endif
64-
if (hostname.empty()) hostname = "unknown";
65-
66-
std::string username;
67-
if (getenv("LOGNAME")) username = getenv("LOGNAME");
68-
#if !defined _WIN32 && !defined ADB_HOST_ON_TARGET
69-
if (username.empty() && getlogin()) username = getlogin();
70-
#endif
71-
if (username.empty()) hostname = "unknown";
72-
73-
return " " + username + "@" + hostname;
74-
}
75-
76-
static bool write_public_keyfile(RSA* private_key, const std::string& private_key_path) {
77-
LOG(INFO) << "write_public_keyfile...";
78-
56+
static bool calculate_public_key(std::string* out, RSA* private_key) {
7957
uint8_t binary_key_data[ANDROID_PUBKEY_ENCODED_SIZE];
8058
if (!android_pubkey_encode(private_key, binary_key_data, sizeof(binary_key_data))) {
8159
LOG(ERROR) << "Failed to convert to public key";
@@ -88,20 +66,10 @@ static bool write_public_keyfile(RSA* private_key, const std::string& private_ke
8866
return false;
8967
}
9068

91-
std::string content;
92-
content.resize(expected_length);
93-
size_t actual_length = EVP_EncodeBlock(reinterpret_cast<uint8_t*>(&content[0]), binary_key_data,
69+
out->resize(expected_length);
70+
size_t actual_length = EVP_EncodeBlock(reinterpret_cast<uint8_t*>(out->data()), binary_key_data,
9471
sizeof(binary_key_data));
95-
content.resize(actual_length);
96-
97-
content += get_user_info();
98-
99-
std::string path(private_key_path + ".pub");
100-
if (!android::base::WriteStringToFile(content, path)) {
101-
PLOG(ERROR) << "Failed to write public key to '" << path << "'";
102-
return false;
103-
}
104-
72+
out->resize(actual_length);
10573
return true;
10674
}
10775

@@ -140,11 +108,6 @@ static int generate_key(const std::string& file) {
140108
goto out;
141109
}
142110

143-
if (!write_public_keyfile(rsa, file)) {
144-
D("Failed to write public key");
145-
goto out;
146-
}
147-
148111
ret = 1;
149112

150113
out:
@@ -170,36 +133,41 @@ static std::string hash_key(RSA* key) {
170133
return result;
171134
}
172135

173-
static bool read_key_file(const std::string& file) {
174-
LOG(INFO) << "read_key_file '" << file << "'...";
175-
136+
static std::shared_ptr<RSA> read_key_file(const std::string& file) {
176137
std::unique_ptr<FILE, decltype(&fclose)> fp(fopen(file.c_str(), "r"), fclose);
177138
if (!fp) {
178139
PLOG(ERROR) << "Failed to open '" << file << "'";
179-
return false;
140+
return nullptr;
180141
}
181142

182143
RSA* key = RSA_new();
183144
if (!PEM_read_RSAPrivateKey(fp.get(), &key, nullptr, nullptr)) {
184145
LOG(ERROR) << "Failed to read key";
185146
RSA_free(key);
147+
return nullptr;
148+
}
149+
150+
return std::shared_ptr<RSA>(key, RSA_free);
151+
}
152+
153+
static bool load_key(const std::string& file) {
154+
std::shared_ptr<RSA> key = read_key_file(file);
155+
if (!key) {
186156
return false;
187157
}
188158

189159
std::lock_guard<std::mutex> lock(g_keys_mutex);
190-
std::string fingerprint = hash_key(key);
160+
std::string fingerprint = hash_key(key.get());
191161
if (g_keys.find(fingerprint) != g_keys.end()) {
192162
LOG(INFO) << "ignoring already-loaded key: " << file;
193-
RSA_free(key);
194163
} else {
195-
g_keys[fingerprint] = std::shared_ptr<RSA>(key, RSA_free);
164+
g_keys[fingerprint] = std::move(key);
196165
}
197-
198166
return true;
199167
}
200168

201-
static bool read_keys(const std::string& path, bool allow_dir = true) {
202-
LOG(INFO) << "read_keys '" << path << "'...";
169+
static bool load_keys(const std::string& path, bool allow_dir = true) {
170+
LOG(INFO) << "load_keys '" << path << "'...";
203171

204172
struct stat st;
205173
if (stat(path.c_str(), &st) != 0) {
@@ -208,7 +176,7 @@ static bool read_keys(const std::string& path, bool allow_dir = true) {
208176
}
209177

210178
if (S_ISREG(st.st_mode)) {
211-
return read_key_file(path);
179+
return load_key(path);
212180
} else if (S_ISDIR(st.st_mode)) {
213181
if (!allow_dir) {
214182
// inotify isn't recursive. It would break expectations to load keys in nested
@@ -237,7 +205,7 @@ static bool read_keys(const std::string& path, bool allow_dir = true) {
237205
continue;
238206
}
239207

240-
result |= read_key_file((path + OS_PATH_SEPARATOR + name));
208+
result |= load_key((path + OS_PATH_SEPARATOR + name));
241209
}
242210
return result;
243211
}
@@ -250,7 +218,7 @@ static std::string get_user_key_path() {
250218
return adb_get_android_dir_path() + OS_PATH_SEPARATOR + "adbkey";
251219
}
252220

253-
static bool get_user_key() {
221+
static bool generate_userkey() {
254222
std::string path = get_user_key_path();
255223
if (path.empty()) {
256224
PLOG(ERROR) << "Error getting user key filename";
@@ -266,7 +234,7 @@ static bool get_user_key() {
266234
}
267235
}
268236

269-
return read_key_file(path);
237+
return load_key(path);
270238
}
271239

272240
static std::set<std::string> get_vendor_keys() {
@@ -320,26 +288,42 @@ static std::string adb_auth_sign(RSA* key, const char* token, size_t token_size)
320288
return result;
321289
}
322290

291+
static bool pubkey_from_privkey(std::string* out, const std::string& path) {
292+
std::shared_ptr<RSA> privkey = read_key_file(path);
293+
if (!privkey) {
294+
return false;
295+
}
296+
return calculate_public_key(out, privkey.get());
297+
}
298+
323299
std::string adb_auth_get_userkey() {
324300
std::string path = get_user_key_path();
325301
if (path.empty()) {
326302
PLOG(ERROR) << "Error getting user key filename";
327303
return "";
328304
}
329-
path += ".pub";
330305

331-
std::string content;
332-
if (!android::base::ReadFileToString(path, &content)) {
333-
PLOG(ERROR) << "Can't load '" << path << "'";
306+
std::string result;
307+
if (!pubkey_from_privkey(&result, path)) {
334308
return "";
335309
}
336-
return content;
310+
return result;
337311
}
338312

339313
int adb_auth_keygen(const char* filename) {
340314
return (generate_key(filename) == 0);
341315
}
342316

317+
int adb_auth_pubkey(const char* filename) {
318+
std::string pubkey;
319+
if (!pubkey_from_privkey(&pubkey, filename)) {
320+
return 1;
321+
}
322+
pubkey.push_back('\n');
323+
324+
return WriteFdExactly(STDOUT_FILENO, pubkey.data(), pubkey.size()) ? 0 : 1;
325+
}
326+
343327
#if defined(__linux__)
344328
static void adb_auth_inotify_update(int fd, unsigned fd_event, void*) {
345329
LOG(INFO) << "adb_auth_inotify_update called";
@@ -380,7 +364,7 @@ static void adb_auth_inotify_update(int fd, unsigned fd_event, void*) {
380364
LOG(INFO) << "ignoring new directory at '" << path << "'";
381365
} else {
382366
LOG(INFO) << "observed new file at '" << path << "'";
383-
read_keys(path, false);
367+
load_keys(path, false);
384368
}
385369
} else {
386370
LOG(WARNING) << "unmonitored event for " << path << ": 0x" << std::hex
@@ -420,8 +404,8 @@ static void adb_auth_inotify_init(const std::set<std::string>& paths) {
420404
void adb_auth_init() {
421405
LOG(INFO) << "adb_auth_init...";
422406

423-
if (!get_user_key()) {
424-
LOG(ERROR) << "Failed to get user key";
407+
if (!generate_userkey()) {
408+
LOG(ERROR) << "Failed to generate user key";
425409
return;
426410
}
427411

@@ -432,7 +416,7 @@ void adb_auth_init() {
432416
#endif
433417

434418
for (const std::string& path : key_paths) {
435-
read_keys(path.c_str());
419+
load_keys(path.c_str());
436420
}
437421
}
438422

‎adb/client/commandline.cpp‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,6 @@ static void help() {
185185
" enable-verity re-enable dm-verity checking on userdebug builds\n"
186186
" keygen FILE\n"
187187
" generate adb public/private key; private key stored in FILE,\n"
188-
" public key stored in FILE.pub (existing files overwritten)\n"
189188
"\n"
190189
"scripting:\n"
191190
" wait-for[-TRANSPORT]-STATE\n"
@@ -1756,14 +1755,14 @@ int adb_commandline(int argc, const char** argv) {
17561755
// Always print key generation information for keygen command.
17571756
adb_trace_enable(AUTH);
17581757
return adb_auth_keygen(argv[1]);
1759-
}
1760-
else if (!strcmp(argv[0], "jdwp")) {
1758+
} else if (!strcmp(argv[0], "pubkey")) {
1759+
if (argc != 2) error_exit("pubkey requires an argument");
1760+
return adb_auth_pubkey(argv[1]);
1761+
} else if (!strcmp(argv[0], "jdwp")) {
17611762
return adb_connect_command("jdwp");
1762-
}
1763-
else if (!strcmp(argv[0], "track-jdwp")) {
1763+
} else if (!strcmp(argv[0], "track-jdwp")) {
17641764
return adb_connect_command("track-jdwp");
1765-
}
1766-
else if (!strcmp(argv[0], "track-devices")) {
1765+
} else if (!strcmp(argv[0], "track-devices")) {
17671766
return adb_connect_command("host:track-devices");
17681767
} else if (!strcmp(argv[0], "raw")) {
17691768
if (argc != 2) {
@@ -1772,13 +1771,11 @@ int adb_commandline(int argc, const char** argv) {
17721771
return adb_connect_command(argv[1]);
17731772
}
17741773

1775-
17761774
/* "adb /?" is a common idiom under Windows */
17771775
else if (!strcmp(argv[0], "--help") || !strcmp(argv[0], "help") || !strcmp(argv[0], "/?")) {
17781776
help();
17791777
return 0;
1780-
}
1781-
else if (!strcmp(argv[0], "--version") || !strcmp(argv[0], "version")) {
1778+
} else if (!strcmp(argv[0], "--version") || !strcmp(argv[0], "version")) {
17821779
fprintf(stdout, "%s", adb_version().c_str());
17831780
return 0;
17841781
} else if (!strcmp(argv[0], "features")) {

0 commit comments

Comments
 (0)