osxkeychain: lock for exclusive execution

Koji Nakamaru (2):
osxkeychain: exclusive lock to serialize execution of operations
osxkeychain: state to skip unnecessary store operations

 .../osxkeychain/git-credential-osxkeychain.c       | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

base-commit: 83f1add914c6b4682de1e944ec0d1ac043d53d78
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1729%2FKojiNakamaru%2Ffeature%2Fosxkeychian_exlock-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1729/KojiNakamaru/feature/osxkeychian_exlock-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1729

Range-diff vs v2:

1: 309c17c78f3 ! 1: 3341346c5e6 osxkeychain: lock for exclusive execution
@@ Metadata
Author: Koji Nakamaru <[email protected]>

## Commit message ##
- osxkeychain: lock for exclusive execution
+ osxkeychain: exclusive lock to serialize execution of operations

- Resolves "failed to store: -25299" when "fetch.parallel 0" is configured
- and there are many submodules.
+ 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:

- The error code -25299 (errSecDuplicateItem) may be returned by
- SecItemUpdate() in add_internet_password() if multiple instances of
- git-credential-osxkeychain run in parallel. This patch introduces an
- exclusive lock to serialize execution for avoiding this and other
- potential issues.
+ 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

2: 1f57718abff ! 2: 146b0ae9146 osxkeychain: state[] seen=1 to skip unnecessary store operations
@@ Metadata
Author: Koji Nakamaru <[email protected]>

## Commit message ##
- osxkeychain: state[] seen=1 to skip unnecessary store operations
+ osxkeychain: state to skip unnecessary store operations

- Records whether credentials come from get operations and skips
- unnecessary store operations by utilizing the state[] feature, as
- suggested by brian m. carlson.
+ 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 <[email protected]>

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

gitgitgadget