Skip to content

Commit 861312b

Browse files
gitstergitster
committed
Merge branch 'kn/osxkeychain-idempotent-store-fix'
An earlier check added to osx keychain credential helper to avoid storing the credential itself supplied was overeager and rejected credential material supplied by other helper backends that it would have wanted to store, which has been corrected. * kn/osxkeychain-idempotent-store-fix: osxkeychain: avoid incorrectly skipping store operation
2 parents aa934e0 + 4580bcd commit 861312b

File tree

3 files changed

+132
-30
lines changed

3 files changed

+132
-30
lines changed

‎contrib/credential/osxkeychain/Makefile‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,65 @@
11
# The default target of this Makefile is...
22
all:: git-credential-osxkeychain
33

4+
include ../../../config.mak.uname
45
-include ../../../config.mak.autogen
56
-include ../../../config.mak
67

8+
ifdef ZLIB_NG
9+
BASIC_CFLAGS += -DHAVE_ZLIB_NG
10+
ifdef ZLIB_NG_PATH
11+
BASIC_CFLAGS += -I$(ZLIB_NG_PATH)/include
12+
EXTLIBS += $(call libpath_template,$(ZLIB_NG_PATH)/$(lib))
13+
endif
14+
EXTLIBS += -lz-ng
15+
else
16+
ifdef ZLIB_PATH
17+
BASIC_CFLAGS += -I$(ZLIB_PATH)/include
18+
EXTLIBS += $(call libpath_template,$(ZLIB_PATH)/$(lib))
19+
endif
20+
EXTLIBS += -lz
21+
endif
22+
ifndef NO_ICONV
23+
ifdef NEEDS_LIBICONV
24+
ifdef ICONVDIR
25+
BASIC_CFLAGS += -I$(ICONVDIR)/include
26+
ICONV_LINK = $(call libpath_template,$(ICONVDIR)/$(lib))
27+
else
28+
ICONV_LINK =
29+
endif
30+
ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
31+
ICONV_LINK += -lintl
32+
endif
33+
EXTLIBS += $(ICONV_LINK) -liconv
34+
endif
35+
endif
36+
ifndef LIBC_CONTAINS_LIBINTL
37+
EXTLIBS += -lintl
38+
endif
39+
740
prefix ?= /usr/local
841
gitexecdir ?= $(prefix)/libexec/git-core
942

1043
CC ?= gcc
11-
CFLAGS ?= -g -O2 -Wall
44+
CFLAGS ?= -g -O2 -Wall -I../../.. $(BASIC_CFLAGS)
45+
LDFLAGS ?= $(BASIC_LDFLAGS) $(EXTLIBS)
1246
INSTALL ?= install
1347
RM ?= rm -f
1448

1549
%.o: %.c
1650
$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $<
1751

18-
git-credential-osxkeychain: git-credential-osxkeychain.o
52+
git-credential-osxkeychain: git-credential-osxkeychain.o ../../../libgit.a
1953
$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) \
2054
-framework Security -framework CoreFoundation
2155

2256
install: git-credential-osxkeychain
2357
$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
2458
$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)
2559

60+
../../../libgit.a:
61+
cd ../../..; make libgit.a
62+
2663
clean:
2764
$(RM) git-credential-osxkeychain git-credential-osxkeychain.o
2865

‎contrib/credential/osxkeychain/git-credential-osxkeychain.c‎

Lines changed: 92 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
#include <string.h>
33
#include <stdlib.h>
44
#include <Security/Security.h>
5+
#include "git-compat-util.h"
6+
#include "strbuf.h"
7+
#include "wrapper.h"
58

69
#define ENCODING kCFStringEncodingUTF8
710
static CFStringRef protocol; /* Stores constant strings - not memory managed */
@@ -12,7 +15,7 @@ static CFStringRef username;
1215
static CFDataRef password;
1316
static CFDataRef password_expiry_utc;
1417
static CFDataRef oauth_refresh_token;
15-
static int state_seen;
18+
static char *state_seen;
1619

1720
static void clear_credential(void)
1821
{
@@ -48,27 +51,6 @@ static void clear_credential(void)
4851

4952
#define STRING_WITH_LENGTH(s) s, sizeof(s) - 1
5053

51-
__attribute__((format (printf, 1, 2), __noreturn__))
52-
static void die(const char *err, ...)
53-
{
54-
char msg[4096];
55-
va_list params;
56-
va_start(params, err);
57-
vsnprintf(msg, sizeof(msg), err, params);
58-
fprintf(stderr, "%s\n", msg);
59-
va_end(params);
60-
clear_credential();
61-
exit(1);
62-
}
63-
64-
static void *xmalloc(size_t len)
65-
{
66-
void *ret = malloc(len);
67-
if (!ret)
68-
die("Out of memory");
69-
return ret;
70-
}
71-
7254
static CFDictionaryRef create_dictionary(CFAllocatorRef allocator, ...)
7355
{
7456
va_list args;
@@ -112,6 +94,66 @@ static void write_item(const char *what, const char *buf, size_t len)
11294
putchar('\n');
11395
}
11496

97+
static void write_item_strbuf(struct strbuf *sb, const char *what, const char *buf, int n)
98+
{
99+
char s[32];
100+
101+
xsnprintf(s, sizeof(s), "__%s=", what);
102+
strbuf_add(sb, s, strlen(s));
103+
strbuf_add(sb, buf, n);
104+
}
105+
106+
static void write_item_strbuf_cfstring(struct strbuf *sb, const char *what, CFStringRef ref)
107+
{
108+
char *buf;
109+
int len;
110+
111+
if (!ref)
112+
return;
113+
len = CFStringGetMaximumSizeForEncoding(CFStringGetLength(ref), ENCODING) + 1;
114+
buf = xmalloc(len);
115+
if (CFStringGetCString(ref, buf, len, ENCODING))
116+
write_item_strbuf(sb, what, buf, strlen(buf));
117+
free(buf);
118+
}
119+
120+
static void write_item_strbuf_cfnumber(struct strbuf *sb, const char *what, CFNumberRef ref)
121+
{
122+
short n;
123+
char buf[32];
124+
125+
if (!ref)
126+
return;
127+
if (!CFNumberGetValue(ref, kCFNumberShortType, &n))
128+
return;
129+
xsnprintf(buf, sizeof(buf), "%d", n);
130+
write_item_strbuf(sb, what, buf, strlen(buf));
131+
}
132+
133+
static void write_item_strbuf_cfdata(struct strbuf *sb, const char *what, CFDataRef ref)
134+
{
135+
char *buf;
136+
int len;
137+
138+
if (!ref)
139+
return;
140+
buf = (char *)CFDataGetBytePtr(ref);
141+
if (!buf || strlen(buf) == 0)
142+
return;
143+
len = CFDataGetLength(ref);
144+
write_item_strbuf(sb, what, buf, len);
145+
}
146+
147+
static void encode_state_seen(struct strbuf *sb)
148+
{
149+
strbuf_add(sb, "osxkeychain:seen=", strlen("osxkeychain:seen="));
150+
write_item_strbuf_cfstring(sb, "host", host);
151+
write_item_strbuf_cfnumber(sb, "port", port);
152+
write_item_strbuf_cfstring(sb, "path", path);
153+
write_item_strbuf_cfstring(sb, "username", username);
154+
write_item_strbuf_cfdata(sb, "password", password);
155+
}
156+
115157
static void find_username_in_item(CFDictionaryRef item)
116158
{
117159
CFStringRef account_ref;
@@ -124,6 +166,7 @@ static void find_username_in_item(CFDictionaryRef item)
124166
write_item("username", "", 0);
125167
return;
126168
}
169+
username = CFStringCreateCopy(kCFAllocatorDefault, account_ref);
127170

128171
username_buf = (char *)CFStringGetCStringPtr(account_ref, ENCODING);
129172
if (username_buf)
@@ -163,6 +206,7 @@ static OSStatus find_internet_password(void)
163206
}
164207

165208
data = CFDictionaryGetValue(item, kSecValueData);
209+
password = CFDataCreateCopy(kCFAllocatorDefault, data);
166210

167211
write_item("password",
168212
(const char *)CFDataGetBytePtr(data),
@@ -173,7 +217,14 @@ static OSStatus find_internet_password(void)
173217
CFRelease(item);
174218

175219
write_item("capability[]", "state", strlen("state"));
176-
write_item("state[]", "osxkeychain:seen=1", strlen("osxkeychain:seen=1"));
220+
{
221+
struct strbuf sb;
222+
223+
strbuf_init(&sb, 1024);
224+
encode_state_seen(&sb);
225+
write_item("state[]", sb.buf, strlen(sb.buf));
226+
strbuf_release(&sb);
227+
}
177228

178229
out:
179230
CFRelease(attrs);
@@ -288,13 +339,22 @@ static OSStatus add_internet_password(void)
288339
CFDictionaryRef attrs;
289340
OSStatus result;
290341

291-
if (state_seen)
292-
return errSecSuccess;
293-
294342
/* Only store complete credentials */
295343
if (!protocol || !host || !username || !password)
296344
return -1;
297345

346+
if (state_seen) {
347+
struct strbuf sb;
348+
349+
strbuf_init(&sb, 1024);
350+
encode_state_seen(&sb);
351+
if (!strcmp(state_seen, sb.buf)) {
352+
strbuf_release(&sb);
353+
return errSecSuccess;
354+
}
355+
strbuf_release(&sb);
356+
}
357+
298358
data = CFDataCreateMutableCopy(kCFAllocatorDefault, 0, password);
299359
if (password_expiry_utc) {
300360
CFDataAppendBytes(data,
@@ -403,8 +463,9 @@ static void read_credential(void)
403463
(UInt8 *)v,
404464
strlen(v));
405465
else if (!strcmp(buf, "state[]")) {
406-
if (!strcmp(v, "osxkeychain:seen=1"))
407-
state_seen = 1;
466+
int len = strlen("osxkeychain:seen=");
467+
if (!strncmp(v, "osxkeychain:seen=", len))
468+
state_seen = xstrdup(v);
408469
}
409470
/*
410471
* Ignore other lines; we don't know what they mean, but
@@ -443,5 +504,8 @@ int main(int argc, const char **argv)
443504

444505
clear_credential();
445506

507+
if (state_seen)
508+
free(state_seen);
509+
446510
return 0;
447511
}

‎contrib/credential/osxkeychain/meson.build‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
executable('git-credential-osxkeychain',
22
sources: 'git-credential-osxkeychain.c',
33
dependencies: [
4+
libgit,
45
dependency('CoreFoundation'),
56
dependency('Security'),
67
],

0 commit comments

Comments
 (0)