| Age | Commit message (Collapse) | Author |
|---|
|
git-credential-osxkeychain skips storing a credential if its "get"
action sets "state[]=osxkeychain:seen=1". This behavior was introduced
in e1ab45b2 (osxkeychain: state to skip unnecessary store operations,
2024-05-15), which appeared in v2.46.
However, this state[] persists even if a credential returned by
"git-credential-osxkeychain get" is invalid and a subsequent helper's
"get" operation returns a valid credential. Another subsequent helper
(such as [1]) may expect git-credential-osxkeychain to store the valid
credential, but the "store" operation is incorrectly skipped because it
only checks "state[]=osxkeychain:seen=1".
To solve this issue, "state[]=osxkeychain:seen" needs to contain enough
information to identify whether the current "store" input matches the
output from the previous "get" operation (and not a credential from
another helper).
Set "state[]=osxkeychain:seen" to a value encoding the credential output
by "get", and compare it with a value encoding the credential input by
"store".
[1]: https://github.com/hickford/git-credential-oauth
Reported-by: Petter Sælen <petter@saelen.eu>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-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>
|
|
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>
|
|
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>
|
|
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>
|
