We've got a couple of credential helpers in "contrib/credential", all
of which aren't yet wired up via Meson. Do so.
Note that ideally, we'd also wire up t0303 to be executed with each of
the credential helpers to verify their functionality. Unfortunately
though, none of them pass the test suite right now, so this is left for
a future change.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "osxkeychain" helper does not compile due to a warning generated by
the unused `argc` parameter. Fix the warning by checking for the minimum
number of required arguments explicitly in the least restrictive way
possible.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Explicitly set the default goal at the very top of various makefiles.
This is already present in some makefiles, but not all of them.
In particular, this corrects a regression introduced in a38edab7c8
(Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06). That
commit added some config files as build targets for the Documentation
directory, and put the target configuration in a sensible place.
Unfortunately, that sensible place was above any other build target
definitions, meaning the default goal changed to being those
configuration files only, rather than the HTML and man page
documentation.
Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The credential helper to talk to OSX keychain sometimes sent
garbage bytes after the username, which has been corrected.
* jk/osxkeychain-username-is-nul-terminated:
credential/osxkeychain: respect NUL terminator in username
This patch fixes a case where git-credential-osxkeychain might output
uninitialized bytes to stdout.
We need to get the username string from a system API using
CFStringGetCString(). To do that, we get the max size for the string
from CFStringGetMaximumSizeForEncoding(), allocate a buffer based on
that, and then read into it. But then we print the entire buffer to
stdout, including the trailing NUL and any extra bytes which were not
needed. Instead, we should stop at the NUL.
This code comes from 9abe31f5f1 (osxkeychain: replace deprecated
SecKeychain API, 2024-02-17). The bug was probably overlooked back then
because this code is only used as a fallback when we can't get the
string via CFStringGetCStringPtr(). According to Apple's documentation:
Whether or not this function returns a valid pointer or NULL depends
on many factors, all of which depend on how the string was created and
its properties.
So it's not clear how we could make a test for this, and we'll have to
rely on manually testing on a system that triggered the bug in the first
place.
Reported-by: Hong Jiang <ilford@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Tested-by: Hong Jiang <ilford@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git passes a credential that has been used successfully to the helpers
to record. If a credential is already stored,
"git-credential-osxkeychain store" just records the credential returned
by "git-credential-osxkeychain get", and unnecessary (sometimes
problematic) SecItemAdd() and/or SecItemUpdate() are performed.
We can skip such unnecessary operations by marking a credential returned
by "git-credential-osxkeychain get". This marking can be done by
utilizing the "state[]" feature:
- The "get" command sets the field "state[]=osxkeychain:seen=1".
- The "store" command skips its actual operation if the field
"state[]=osxkeychain:seen=1" exists.
Introduce a new state "state[]=osxkeychain:seen=1".
Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Koji Nakamaru <koji.nakamaru@gree.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git passes a credential that has been used successfully to the helpers
to record. If "git-credential-osxkeychain store" commands run in
parallel (with fetch.parallel configuration and/or by running multiple
git commands simultaneously), some of them may exit with the error
"failed to store: -25299". This is because SecItemUpdate() in
add_internet_password() may return errSecDuplicateItem (-25299) in this
situation. Apple's documentation [1] also states as below:
In macOS, some of the functions of this API block while waiting for
input from the user (for example, when the user is asked to unlock a
keychain or give permission to change trust settings). In general, it
is safe to use this API in threads other than your main thread, but
avoid calling the functions from multiple operations, work queues, or
threads concurrently. Instead, serialize function calls or confine
them to a single thread.
The error has not been noticed before, because the former implementation
ignored the error.
Introduce an exclusive lock to serialize execution of operations.
[1] https://developer.apple.com/documentation/security/certificate_key_and_trust_services/working_with_concurrency
Signed-off-by: Koji Nakamaru <koji.nakamaru@gree.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
d208bfdfef (credential: new attribute password_expiry_utc, 2023-02-18)
and a5c76569e7 (credential: new attribute oauth_refresh_token,
2023-04-21) introduced new credential attributes but support was missing
from git-credential-osxkeychain.
Support these attributes by appending the data to the password in the
keychain, separated by line breaks. Line breaks cannot appear in a git
credential password so it is an appropriate separator.
Fixes the remaining test failures with osxkeychain:
18 - helper (osxkeychain) gets password_expiry_utc
19 - helper (osxkeychain) overwrites when password_expiry_utc
changes
21 - helper (osxkeychain) gets oauth_refresh_token
Signed-off-by: Bo Anderson <mail@boanderson.me>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Other credential helpers support deleting credentials that match a
specified password. See 7144dee3ec (credential/libsecret: erase matching
creds only, 2023-07-26) and cb626f8e5c (credential/wincred: erase
matching creds only, 2023-07-26).
Support this in osxkeychain too by extracting, decrypting and comparing
the stored password before deleting.
Fixes the following test failure with osxkeychain:
11 - helper (osxkeychain) does not erase a password distinct from
input
Signed-off-by: Bo Anderson <mail@boanderson.me>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Other credential managers erased all matching credentials, as indicated
by a test case that osxkeychain failed:
15 - helper (osxkeychain) erases all matching credentials
Signed-off-by: Bo Anderson <mail@boanderson.me>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
The macOS Keychain-based credential helper reads the newline-delimited
protocol stream one line at a time by repeatedly calling fgets() into a
fixed-size buffer, and is thus affected by the vulnerability described
in the previous commit.
To mitigate this attack, avoid using a fixed-size buffer, and instead
rely on getline() to allocate a buffer as large as necessary to fit the
entire content of the line, preventing any protocol injection.
We solved a similar problem in a5bb10fd5e (config: avoid fixed-sized
buffer when renaming/deleting a section, 2023-04-06) by switching to
strbuf_getline(). We can't do that here because the contrib helpers do
not link with the rest of Git, and so can't use a strbuf. But we can use
the system getline() directly, which works similarly.
In most parts of Git we don't assume that every platform has getline().
But this helper is run only on OS X, and that platform added support in
10.7 ("Lion") which was released in 2011.
Tested-by: Taylor Blau <me@ttaylorr.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Like in all the other credential helpers, the osxkeychain helper
ignores unknown credential lines.
Add a comment (a la the other helpers) to make it clear and explicit
that this is the desired behaviour.
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update git-credential-osxkeychain.c to remove 'format string is not a string
literal (potentially insecure)' compiler warning by treating the string as
an argument.
Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add missing __attribute__((format)) function attributes to various
"static" functions that take printf arguments.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add protocol imap, imaps, ftp and smtp for credential-osxkeychain.
Signed-off-by: Xidorn Quan <quanxunzhen@gmail.com>
Acked-by: John Szakmeister <john@szakmeister.net>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make the usage string consistent with Git.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The default compiler and cflags were mostly "works for me"
when I built the original version. We need to be much less
careful here than usual, because we know we are building
only on OS X. But it's only polite to at least respect the
CFLAGS and CC definitions that the user may have provided
earlier.
While we're at it, let's update our definitions and rules to
be more like the top-level Makefile; default our CFLAGS to
include -O2, and make sure we use CFLAGS and LDFLAGS when
linking.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With this installed in your $PATH, you can store
git-over-http passwords in your keychain by doing:
git config credential.helper osxkeychain
The code is based in large part on the work of Jay Soffian,
who wrote the helper originally for the initial, unpublished
version of the credential helper protocol.
This version will pass t0303 if you do:
GIT_TEST_CREDENTIAL_HELPER=osxkeychain \
GIT_TEST_CREDENTIAL_HELPER_SETUP="export HOME=$HOME" \
./t0303-credential-external.sh
The "HOME" setup is unfortunately necessary. The test
scripts set HOME to the trash directory, but this causes the
keychain API to complain.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano