Fixed
Status Update
Comments
ap...@google.com <ap...@google.com> #2
Project: platform/frameworks/support
Branch: androidx-main
commit b5960239a6065d709ee0df8dd35f529d55a04600
Author: Daniel Angell <danielangell@google.com>
Date: Thu Feb 09 19:17:44 2023
Fix race condition in MasterKeys.getOrCreate
This was reported to me back in December, 2022 but has been
difficult to reproduce. I've only seen it occur in Tink
integration tests hosted on an internal google repo. Attempts to
reproduce in my Android Studio environment have failed, but the
included patch fixed the race condition in the one environment
it could be demonstrated in.
Relnote: Fixed a race condition in MasterKeys.getOrCreate
Test: See above
Bug: 268572037
Change-Id: I3391e7ba62acfa942432eba402e74d64e2e5e98a
M security/security-crypto/src/main/java/androidx/security/crypto/MasterKeys.java
https://android-review.googlesource.com/2429264
Branch: androidx-main
commit b5960239a6065d709ee0df8dd35f529d55a04600
Author: Daniel Angell <danielangell@google.com>
Date: Thu Feb 09 19:17:44 2023
Fix race condition in MasterKeys.getOrCreate
This was reported to me back in December, 2022 but has been
difficult to reproduce. I've only seen it occur in Tink
integration tests hosted on an internal google repo. Attempts to
reproduce in my Android Studio environment have failed, but the
included patch fixed the race condition in the one environment
it could be demonstrated in.
Relnote: Fixed a race condition in MasterKeys.getOrCreate
Test: See above
Bug: 268572037
Change-Id: I3391e7ba62acfa942432eba402e74d64e2e5e98a
M security/security-crypto/src/main/java/androidx/security/crypto/MasterKeys.java
da...@google.com <da...@google.com>
na...@google.com <na...@google.com> #3
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.security:security-crypto:1.1.0-alpha05
Description
```
public void basicTest() throws Exception {
if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.M) {
throw new RuntimeException("API v23 or higher is required to run this test");
}
KeyStore keyStore = KeyStore.getInstance("AndroidKeyStore");
keyStore.load(/* param= */ null);
final ArrayList<Exception> exceptions = new ArrayList<>();
Thread thread =
new Thread() {
@Override
public void run() {
try {
MasterKey masterKey =
new MasterKey.Builder(ApplicationProvider.getApplicationContext())
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build();
} catch (Exception e) {
synchronized (exceptions) {
exceptions.add(e);
}
}
}
};
// This starts a new thread that generates a new master key. I think that thread blocks until
// the new master key is finished.
thread.start();
// We now try to create another master key in this thread.
MasterKey masterKey =
new MasterKey.Builder(ApplicationProvider.getApplicationContext())
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build();
// This should be true, but it is sometimes false.
assertThat(keyStore.containsAlias("_androidx_security_master_key_")).isTrue();
// Wait for other thread to finish
thread.join();
if (exceptions.size() > 0) {
throw exceptions.get(0);
}
}
```
This is because of a non-atomic check and set in `MasterKeys.getOrCreate`.
Reported by juerg@google.com