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
main
Junio C Hamano 2025-11-24 15:46:41 -08:00
commit 861312b51d
3 changed files with 132 additions and 30 deletions

View File

@ -1,21 +1,55 @@
# The default target of this Makefile is...
all:: git-credential-osxkeychain

include ../../../config.mak.uname
-include ../../../config.mak.autogen
-include ../../../config.mak

ifdef ZLIB_NG
BASIC_CFLAGS += -DHAVE_ZLIB_NG
ifdef ZLIB_NG_PATH
BASIC_CFLAGS += -I$(ZLIB_NG_PATH)/include
EXTLIBS += $(call libpath_template,$(ZLIB_NG_PATH)/$(lib))
endif
EXTLIBS += -lz-ng
else
ifdef ZLIB_PATH
BASIC_CFLAGS += -I$(ZLIB_PATH)/include
EXTLIBS += $(call libpath_template,$(ZLIB_PATH)/$(lib))
endif
EXTLIBS += -lz
endif
ifndef NO_ICONV
ifdef NEEDS_LIBICONV
ifdef ICONVDIR
BASIC_CFLAGS += -I$(ICONVDIR)/include
ICONV_LINK = $(call libpath_template,$(ICONVDIR)/$(lib))
else
ICONV_LINK =
endif
ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
ICONV_LINK += -lintl
endif
EXTLIBS += $(ICONV_LINK) -liconv
endif
endif
ifndef LIBC_CONTAINS_LIBINTL
EXTLIBS += -lintl
endif

prefix ?= /usr/local
gitexecdir ?= $(prefix)/libexec/git-core

CC ?= gcc
CFLAGS ?= -g -O2 -Wall
CFLAGS ?= -g -O2 -Wall -I../../.. $(BASIC_CFLAGS)
LDFLAGS ?= $(BASIC_LDFLAGS) $(EXTLIBS)
INSTALL ?= install
RM ?= rm -f

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

git-credential-osxkeychain: git-credential-osxkeychain.o
git-credential-osxkeychain: git-credential-osxkeychain.o ../../../libgit.a
$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) \
-framework Security -framework CoreFoundation

@ -23,6 +57,9 @@ install: git-credential-osxkeychain
$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)

../../../libgit.a:
cd ../../..; make libgit.a

clean:
$(RM) git-credential-osxkeychain git-credential-osxkeychain.o


View File

@ -2,6 +2,9 @@
#include <string.h>
#include <stdlib.h>
#include <Security/Security.h>
#include "git-compat-util.h"
#include "strbuf.h"
#include "wrapper.h"

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

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

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

__attribute__((format (printf, 1, 2), __noreturn__))
static void die(const char *err, ...)
{
char msg[4096];
va_list params;
va_start(params, err);
vsnprintf(msg, sizeof(msg), err, params);
fprintf(stderr, "%s\n", msg);
va_end(params);
clear_credential();
exit(1);
}

static void *xmalloc(size_t len)
{
void *ret = malloc(len);
if (!ret)
die("Out of memory");
return ret;
}

static CFDictionaryRef create_dictionary(CFAllocatorRef allocator, ...)
{
va_list args;
@ -112,6 +94,66 @@ static void write_item(const char *what, const char *buf, size_t len)
putchar('\n');
}

static void write_item_strbuf(struct strbuf *sb, const char *what, const char *buf, int n)
{
char s[32];

xsnprintf(s, sizeof(s), "__%s=", what);
strbuf_add(sb, s, strlen(s));
strbuf_add(sb, buf, n);
}

static void write_item_strbuf_cfstring(struct strbuf *sb, const char *what, CFStringRef ref)
{
char *buf;
int len;

if (!ref)
return;
len = CFStringGetMaximumSizeForEncoding(CFStringGetLength(ref), ENCODING) + 1;
buf = xmalloc(len);
if (CFStringGetCString(ref, buf, len, ENCODING))
write_item_strbuf(sb, what, buf, strlen(buf));
free(buf);
}

static void write_item_strbuf_cfnumber(struct strbuf *sb, const char *what, CFNumberRef ref)
{
short n;
char buf[32];

if (!ref)
return;
if (!CFNumberGetValue(ref, kCFNumberShortType, &n))
return;
xsnprintf(buf, sizeof(buf), "%d", n);
write_item_strbuf(sb, what, buf, strlen(buf));
}

static void write_item_strbuf_cfdata(struct strbuf *sb, const char *what, CFDataRef ref)
{
char *buf;
int len;

if (!ref)
return;
buf = (char *)CFDataGetBytePtr(ref);
if (!buf || strlen(buf) == 0)
return;
len = CFDataGetLength(ref);
write_item_strbuf(sb, what, buf, len);
}

static void encode_state_seen(struct strbuf *sb)
{
strbuf_add(sb, "osxkeychain:seen=", strlen("osxkeychain:seen="));
write_item_strbuf_cfstring(sb, "host", host);
write_item_strbuf_cfnumber(sb, "port", port);
write_item_strbuf_cfstring(sb, "path", path);
write_item_strbuf_cfstring(sb, "username", username);
write_item_strbuf_cfdata(sb, "password", password);
}

static void find_username_in_item(CFDictionaryRef item)
{
CFStringRef account_ref;
@ -124,6 +166,7 @@ static void find_username_in_item(CFDictionaryRef item)
write_item("username", "", 0);
return;
}
username = CFStringCreateCopy(kCFAllocatorDefault, account_ref);

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

data = CFDictionaryGetValue(item, kSecValueData);
password = CFDataCreateCopy(kCFAllocatorDefault, data);

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

write_item("capability[]", "state", strlen("state"));
write_item("state[]", "osxkeychain:seen=1", strlen("osxkeychain:seen=1"));
{
struct strbuf sb;

strbuf_init(&sb, 1024);
encode_state_seen(&sb);
write_item("state[]", sb.buf, strlen(sb.buf));
strbuf_release(&sb);
}

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

if (state_seen)
return errSecSuccess;

/* Only store complete credentials */
if (!protocol || !host || !username || !password)
return -1;

if (state_seen) {
struct strbuf sb;

strbuf_init(&sb, 1024);
encode_state_seen(&sb);
if (!strcmp(state_seen, sb.buf)) {
strbuf_release(&sb);
return errSecSuccess;
}
strbuf_release(&sb);
}

data = CFDataCreateMutableCopy(kCFAllocatorDefault, 0, password);
if (password_expiry_utc) {
CFDataAppendBytes(data,
@ -403,8 +463,9 @@ static void read_credential(void)
(UInt8 *)v,
strlen(v));
else if (!strcmp(buf, "state[]")) {
if (!strcmp(v, "osxkeychain:seen=1"))
state_seen = 1;
int len = strlen("osxkeychain:seen=");
if (!strncmp(v, "osxkeychain:seen=", len))
state_seen = xstrdup(v);
}
/*
* Ignore other lines; we don't know what they mean, but
@ -443,5 +504,8 @@ int main(int argc, const char **argv)

clear_credential();

if (state_seen)
free(state_seen);

return 0;
}

View File

@ -1,6 +1,7 @@
executable('git-credential-osxkeychain',
sources: 'git-credential-osxkeychain.c',
dependencies: [
libgit,
dependency('CoreFoundation'),
dependency('Security'),
],