osxkeychain: replace deprecated SecKeychain API
The SecKeychain API was deprecated in macOS 10.10, nearly 10 years ago. The replacement SecItem API however is available as far back as macOS 10.6. While supporting older macOS was perhaps prevously a concern, git-credential-osxkeychain already requires a minimum of macOS 10.7 since 5747c8072b (contrib/credential: avoid fixed-size buffer in osxkeychain, 2023-05-01) so using the newer API should not regress the range of macOS versions supported. Adapting to use the newer SecItem API also happens to fix two test failures in osxkeychain: 8 - helper (osxkeychain) overwrites on store 9 - helper (osxkeychain) can forget host The new API is compatible with credentials saved with the older API. Signed-off-by: Bo Anderson <mail@boanderson.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bo Anderson committed
Feb 17, 2024 at 23:34 UTC
9abe31f5f161be4d69118bdfae00103cd6efa510
2 files changed
+199
-69
contrib/credential/osxkeychain/Makefile
+2
-1
@@ -8,7 +8,8 @@ CFLAGS = -g -O2 -Wall
8
-include ../../../config.mak
9
10
git-credential-osxkeychain: git-credential-osxkeychain.o
11
- $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) -Wl,-framework -Wl,Security
11
+ $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) \
12
+ -framework Security -framework CoreFoundation
13
14
git-credential-osxkeychain.o: git-credential-osxkeychain.c
15
$(CC) -c $(CFLAGS) $<
contrib/credential/osxkeychain/git-credential-osxkeychain.c
+197
-68
@@ -3,14 +3,39 @@
3
#include <stdlib.h>
4
#include <Security/Security.h>
5
6
-static SecProtocolType protocol;
7
-static char *host;
8
-static char *path;
9
-static char *username;
10
-static char *password;
11
-static UInt16 port;
12
-
13
-__attribute__((format (printf, 1, 2)))
6
+#define ENCODING kCFStringEncodingUTF8
7
+static CFStringRef protocol; /* Stores constant strings - not memory managed */
8
+static CFStringRef host;
9
+static CFStringRef path;
10
+static CFStringRef username;
11
+static CFDataRef password;
12
+static CFNumberRef port;
13
+
14
+static void clear_credential(void)
15
+{
16
+ if (host) {
17
+ CFRelease(host);
18
+ host = NULL;
19
+ }
20
+ if (path) {
21
+ CFRelease(path);
22
+ path = NULL;
23
+ }
24
+ if (username) {
25
+ CFRelease(username);
26
+ username = NULL;
27
+ }
28
+ if (password) {
29
+ CFRelease(password);
30
+ password = NULL;
31
+ }
32
+ if (port) {
33
+ CFRelease(port);
34
+ port = NULL;
35
+ }
36
+}
37
+
38
+__attribute__((format (printf, 1, 2), __noreturn__))
39
static void die(const char *err, ...)
40
{
41
char msg[4096];
@@ -19,70 +44,135 @@ static void die(const char *err, ...)
44
vsnprintf(msg, sizeof(msg), err, params);
45
fprintf(stderr, "%s\n", msg);
46
va_end(params);
47
+ clear_credential();
48
exit(1);
49
}
50
25
-static void *xstrdup(const char *s1)
51
+static void *xmalloc(size_t len)
52
{
27
- void *ret = strdup(s1);
53
+ void *ret = malloc(len);
54
if (!ret)
55
die("Out of memory");
56
return ret;
57
}
58
33
-#define KEYCHAIN_ITEM(x) (x ? strlen(x) : 0), x
34
-#define KEYCHAIN_ARGS \
35
- NULL, /* default keychain */ \
36
- KEYCHAIN_ITEM(host), \
37
- 0, NULL, /* account domain */ \
38
- KEYCHAIN_ITEM(username), \
39
- KEYCHAIN_ITEM(path), \
40
- port, \
41
- protocol, \
42
- kSecAuthenticationTypeDefault
43
-
44
-static void write_item(const char *what, const char *buf, int len)
59
+static CFDictionaryRef create_dictionary(CFAllocatorRef allocator, ...)
60
+{
61
+ va_list args;
62
+ const void *key;
63
+ CFMutableDictionaryRef result;
64
+
65
+ result = CFDictionaryCreateMutable(allocator,
66
+ 0,
67
+ &kCFTypeDictionaryKeyCallBacks,
68
+ &kCFTypeDictionaryValueCallBacks);
69
+
70
+
71
+ va_start(args, allocator);
72
+ while ((key = va_arg(args, const void *)) != NULL) {
73
+ const void *value;
74
+ value = va_arg(args, const void *);
75
+ if (value)
76
+ CFDictionarySetValue(result, key, value);
77
+ }
78
+ va_end(args);
79
+
80
+ return result;
81
+}
82
+
83
+#define CREATE_SEC_ATTRIBUTES(...) \
84
+ create_dictionary(kCFAllocatorDefault, \
85
+ kSecClass, kSecClassInternetPassword, \
86
+ kSecAttrServer, host, \
87
+ kSecAttrAccount, username, \
88
+ kSecAttrPath, path, \
89
+ kSecAttrPort, port, \
90
+ kSecAttrProtocol, protocol, \
91
+ kSecAttrAuthenticationType, \
92
+ kSecAttrAuthenticationTypeDefault, \
93
+ __VA_ARGS__);
94
+
95
+static void write_item(const char *what, const char *buf, size_t len)
96
{
97
printf("%s=", what);
98
fwrite(buf, 1, len, stdout);
99
putchar('\n');
100
}
101
51
-static void find_username_in_item(SecKeychainItemRef item)
102
+static void find_username_in_item(CFDictionaryRef item)
103
{
53
- SecKeychainAttributeList list;
54
- SecKeychainAttribute attr;
104
+ CFStringRef account_ref;
105
+ char *username_buf;
106
+ CFIndex buffer_len;
107
56
- list.count = 1;
57
- list.attr = &attr;
58
- attr.tag = kSecAccountItemAttr;
108
+ account_ref = CFDictionaryGetValue(item, kSecAttrAccount);
109
+ if (!account_ref)
110
+ {
111
+ write_item("username", "", 0);
112
+ return;
113
+ }
114
60
- if (SecKeychainItemCopyContent(item, NULL, &list, NULL, NULL))
115
+ username_buf = (char *)CFStringGetCStringPtr(account_ref, ENCODING);
116
+ if (username_buf)
117
+ {
118
+ write_item("username", username_buf, strlen(username_buf));
119
return;
120
+ }
121
63
- write_item("username", attr.data, attr.length);
64
- SecKeychainItemFreeContent(&list, NULL);
122
+ /* If we can't get a CString pointer then
123
+ * we need to allocate our own buffer */
124
+ buffer_len = CFStringGetMaximumSizeForEncoding(
125
+ CFStringGetLength(account_ref), ENCODING) + 1;
126
+ username_buf = xmalloc(buffer_len);
127
+ if (CFStringGetCString(account_ref,
128
+ username_buf,
129
+ buffer_len,
130
+ ENCODING)) {
131
+ write_item("username", username_buf, buffer_len - 1);
132
+ }
133
+ free(username_buf);
134
}
135
67
-static void find_internet_password(void)
136
+static OSStatus find_internet_password(void)
137
{
69
- void *buf;
70
- UInt32 len;
71
- SecKeychainItemRef item;
138
+ CFDictionaryRef attrs;
139
+ CFDictionaryRef item;
140
+ CFDataRef data;
141
+ OSStatus result;
142
73
- if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS, &len, &buf, &item))
74
- return;
143
+ attrs = CREATE_SEC_ATTRIBUTES(kSecMatchLimit, kSecMatchLimitOne,
144
+ kSecReturnAttributes, kCFBooleanTrue,
145
+ kSecReturnData, kCFBooleanTrue,
146
+ NULL);
147
+ result = SecItemCopyMatching(attrs, (CFTypeRef *)&item);
148
+ if (result) {
149
+ goto out;
150
+ }
151
76
- write_item("password", buf, len);
152
+ data = CFDictionaryGetValue(item, kSecValueData);
153
+
154
+ write_item("password",
155
+ (const char *)CFDataGetBytePtr(data),
156
+ CFDataGetLength(data));
157
if (!username)
158
find_username_in_item(item);
159
80
- SecKeychainItemFreeContent(NULL, buf);
160
+ CFRelease(item);
161
+
162
+out:
163
+ CFRelease(attrs);
164
+
165
+ /* We consider not found to not be an error */
166
+ if (result == errSecItemNotFound)
167
+ result = errSecSuccess;
168
+
169
+ return result;
170
}
171
83
-static void delete_internet_password(void)
172
+static OSStatus delete_internet_password(void)
173
{
85
- SecKeychainItemRef item;
174
+ CFDictionaryRef attrs;
175
+ OSStatus result;
176
177
/*
178
* Require at least a protocol and host for removal, which is what git
@@ -90,25 +180,42 @@ static void delete_internet_password(void)
180
* Keychain manager.
181
*/
182
if (!protocol || !host)
93
- return;
183
+ return -1;
184
95
- if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS, 0, NULL, &item))
96
- return;
185
+ attrs = CREATE_SEC_ATTRIBUTES(NULL);
186
+ result = SecItemDelete(attrs);
187
+ CFRelease(attrs);
188
+
189
+ /* We consider not found to not be an error */
190
+ if (result == errSecItemNotFound)
191
+ result = errSecSuccess;
192
98
- SecKeychainItemDelete(item);
193
+ return result;
194
}
195
101
-static void add_internet_password(void)
196
+static OSStatus add_internet_password(void)
197
{
198
+ CFDictionaryRef attrs;
199
+ OSStatus result;
200
+
201
/* Only store complete credentials */
202
if (!protocol || !host || !username || !password)
105
- return;
203
+ return -1;
204
107
- if (SecKeychainAddInternetPassword(
108
- KEYCHAIN_ARGS,
109
- KEYCHAIN_ITEM(password),
110
- NULL))
111
- return;
205
+ attrs = CREATE_SEC_ATTRIBUTES(kSecValueData, password,
206
+ NULL);
207
+
208
+ result = SecItemAdd(attrs, NULL);
209
+ if (result == errSecDuplicateItem) {
210
+ CFDictionaryRef query;
211
+ query = CREATE_SEC_ATTRIBUTES(NULL);
212
+ result = SecItemUpdate(query, attrs);
213
+ CFRelease(query);
214
+ }
215
+
216
+ CFRelease(attrs);
217
+
218
+ return result;
219
}
220
221
static void read_credential(void)
@@ -131,36 +238,52 @@ static void read_credential(void)
238
239
if (!strcmp(buf, "protocol")) {
240
if (!strcmp(v, "imap"))
134
- protocol = kSecProtocolTypeIMAP;
241
+ protocol = kSecAttrProtocolIMAP;
242
else if (!strcmp(v, "imaps"))
136
- protocol = kSecProtocolTypeIMAPS;
243
+ protocol = kSecAttrProtocolIMAPS;
244
else if (!strcmp(v, "ftp"))
138
- protocol = kSecProtocolTypeFTP;
245
+ protocol = kSecAttrProtocolFTP;
246
else if (!strcmp(v, "ftps"))
140
- protocol = kSecProtocolTypeFTPS;
247
+ protocol = kSecAttrProtocolFTPS;
248
else if (!strcmp(v, "https"))
142
- protocol = kSecProtocolTypeHTTPS;
249
+ protocol = kSecAttrProtocolHTTPS;
250
else if (!strcmp(v, "http"))
144
- protocol = kSecProtocolTypeHTTP;
251
+ protocol = kSecAttrProtocolHTTP;
252
else if (!strcmp(v, "smtp"))
146
- protocol = kSecProtocolTypeSMTP;
147
- else /* we don't yet handle other protocols */
253
+ protocol = kSecAttrProtocolSMTP;
254
+ else {
255
+ /* we don't yet handle other protocols */
256
+ clear_credential();
257
exit(0);
258
+ }
259
}
260
else if (!strcmp(buf, "host")) {
261
char *colon = strchr(v, ':');
262
if (colon) {
263
+ UInt16 port_i;
264
*colon++ = '\0';
154
- port = atoi(colon);
265
+ port_i = atoi(colon);
266
+ port = CFNumberCreate(kCFAllocatorDefault,
267
+ kCFNumberShortType,
268
+ &port_i);
269
}
156
- host = xstrdup(v);
270
+ host = CFStringCreateWithCString(kCFAllocatorDefault,
271
+ v,
272
+ ENCODING);
273
}
274
else if (!strcmp(buf, "path"))
159
- path = xstrdup(v);
275
+ path = CFStringCreateWithCString(kCFAllocatorDefault,
276
+ v,
277
+ ENCODING);
278
else if (!strcmp(buf, "username"))
161
- username = xstrdup(v);
279
+ username = CFStringCreateWithCString(
280
+ kCFAllocatorDefault,
281
+ v,
282
+ ENCODING);
283
else if (!strcmp(buf, "password"))
163
- password = xstrdup(v);
284
+ password = CFDataCreate(kCFAllocatorDefault,
285
+ (UInt8 *)v,
286
+ strlen(v));
287
/*
288
* Ignore other lines; we don't know what they mean, but
289
* this future-proofs us when later versions of git do
@@ -173,6 +296,7 @@ static void read_credential(void)
296
297
int main(int argc, const char **argv)
298
{
299
+ OSStatus result = 0;
300
const char *usage =
301
"usage: git credential-osxkeychain <get|store|erase>";
302
@@ -182,12 +306,17 @@ int main(int argc, const char **argv)
306
read_credential();
307
308
if (!strcmp(argv[1], "get"))
185
- find_internet_password();
309
+ result = find_internet_password();
310
else if (!strcmp(argv[1], "store"))
187
- add_internet_password();
311
+ result = add_internet_password();
312
else if (!strcmp(argv[1], "erase"))
189
- delete_internet_password();
313
+ result = delete_internet_password();
314
/* otherwise, ignore unknown action */
315
316
+ if (result)
317
+ die("failed to %s: %d", argv[1], (int)result);
318
+
319
+ clear_credential();
320
+
321
return 0;
322
}