Commit fa5400a4 authored by Omar Morsi's avatar Omar Morsi Committed by Chromium LUCI CQ

Fix Platform Keys API tests flakiness

Before this CL, platform keys API tests were disabled because they were
flaky. The main reason behind that was mixing chrome.test.callbackPass()
and chrome.test.succeed() in more than one test. This introduces
unpredictable behavior as if the callback counter is incremented
anywhere in a test function, the test infrastructure will automatically
invoke chrome.test.succeed(). For more information about this problem,
please refer to
https://chromium.googlesource.com/chromium/src.git/+/master/extensions/docs/testing_api.md#don_t-mix-chrome_test_callbackpass_et-al_and-chrome_test_succeed

This CL fixes mixing chrome.test.callbackPass() and
chrome.test.succeed() by adapting only one of them in each test.

This is a quick fix so as to include platform keys API tests again in
trybots, but a refactor to totally avoid using callbackPass is being
tracked in crbug.com/1154680.

The flakiness is verified to be fixed by running the suite 20
consecutive successful times using the following command.
browser_tests --gtest_filter=*EnterprisePlatform*  --gtest_repeat=20

Bug: 1157137
Change-Id: If7a94d4cc37e5b0719d18fdd3a9cef425de17ca1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2624473Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Commit-Queue: Omar Morsi <omorsi@google.com>
Cr-Commit-Position: refs/heads/master@{#843993}
parent a20ceb28
......@@ -277,8 +277,7 @@ IN_PROC_BROWSER_TEST_P(EnterprisePlatformKeysTest, PRE_Basic) {
RunPreTest();
}
// Flaky: https://crbug.com/1157137.
IN_PROC_BROWSER_TEST_P(EnterprisePlatformKeysTest, DISABLED_Basic) {
IN_PROC_BROWSER_TEST_P(EnterprisePlatformKeysTest, Basic) {
{
base::RunLoop loop;
GetNSSCertDatabaseForProfile(
......
......@@ -444,7 +444,7 @@ function verifyRsaKeySign(
const SIGN_PARAMS = {name: 'RSASSA-PKCS1-v1_5'};
token.subtleCrypto.sign(SIGN_PARAMS, keyPair.privateKey, DATA)
.then(
function(signature) {
callbackPass(function(signature) {
var importParams = {
name: algorithm.name,
// RsaHashedImportParams
......@@ -458,12 +458,12 @@ function verifyRsaKeySign(
cachedSignature = signature;
return window.crypto.subtle.importKey(
'spki', spki, importParams, false, ['verify']);
},
}),
function(error) {
fail(debugMessage + ': Sign failed: ' + error);
})
.then(
function(webCryptoPublicKey) {
callbackPass(function(webCryptoPublicKey) {
assertTrue(!!webCryptoPublicKey);
assertEq(
algorithm.modulusLength,
......@@ -473,15 +473,15 @@ function verifyRsaKeySign(
webCryptoPublicKey.algorithm.publicExponent);
return window.crypto.subtle.verify(
algorithm, webCryptoPublicKey, cachedSignature, DATA);
},
}),
function(error) {
fail(debugMessage + ': Import failed: ' + error);
})
.then(
function(success) {
callbackPass(function(success) {
assertEq(true, success, debugMessage + ': Signature invalid.');
callback(keyPair, spki);
},
}),
function(error) {
fail(debugMessage + ': Verification failed: ' + error);
});
......@@ -494,31 +494,31 @@ function verifyEcKeySign(token, params, keyPair, spki, debugMessage, callback) {
var cachedSignature;
token.subtleCrypto.sign(params.sign, keyPair.privateKey, DATA)
.then(
function(signature) {
callbackPass(function(signature) {
assertTrue(!!signature, debugMessage + ': No signature.');
assertTrue(
signature.length != 0, debugMessage + ': Signature is empty.');
cachedSignature = signature;
return window.crypto.subtle.importKey(
'spki', spki, params.importKey, false, ['verify']);
},
}),
function(error) {
fail(debugMessage + ': Sign failed: ' + error);
})
.then(
function(webCryptoPublicKey) {
callbackPass(function(webCryptoPublicKey) {
assertTrue(!!webCryptoPublicKey);
return window.crypto.subtle.verify(
params.verify, webCryptoPublicKey, cachedSignature, DATA);
},
}),
function(error) {
fail(debugMessage + ': Import failed: ' + error);
})
.then(
function(success) {
callbackPass(function(success) {
assertEq(true, success, debugMessage + ': Signature invalid.');
callback(keyPair, spki);
},
}),
function(error) {
fail(debugMessage + ': Verification failed: ' + error);
});
......@@ -562,7 +562,8 @@ function generateRsaKeyAndVerify(token, algorithm, callback) {
verifyRsaKeySign(
token, algorithm, cachedKeyPair, publicKeySpki,
/*debugMessage=*/ 'First signing attempt', callback);
/*debugMessage=*/ 'First signing attempt',
callbackPass(callback));
}),
function(error) {
fail('Export failed: ' + error);
......@@ -573,16 +574,16 @@ function generateEcKeyAndVerify(token, params, callback) {
var cachedKeyPair;
token.subtleCrypto.generateKey(params.generateKey, false, ['sign'])
.then(
function(keyPair) {
callbackPass(function(keyPair) {
assertTrue(!!keyPair, 'No key pair.');
cachedKeyPair = keyPair;
return token.subtleCrypto.exportKey('spki', keyPair.publicKey);
},
}),
function(error) {
fail('GenerateKey failed: ' + error);
})
.then(
function(publicKeySpki) {
callbackPass(function(publicKeySpki) {
// Ensure that the returned key pair has the expected format.
// Some parameter independent checks:
checkEcKeyPairCommonFormat(cachedKeyPair);
......@@ -594,10 +595,11 @@ function generateEcKeyAndVerify(token, params, callback) {
var publicKey = cachedKeyPair.publicKey;
assertEq([], publicKey.usages);
return verifyEcKeySign(
verifyEcKeySign(
token, params, cachedKeyPair, publicKeySpki,
/*debugMessage=*/ 'First signing attempt', callback);
},
/*debugMessage=*/ 'First signing attempt',
callbackPass(callback));
}),
function(error) {
fail('Export failed: ' + error);
});
......@@ -647,7 +649,6 @@ function testGenerateRsaKeyAndSignAllowedOnce(token) {
assertEq(
'The operation failed for an operation-specific reason',
error.message);
succeed();
}));
}));
}
......@@ -655,15 +656,15 @@ function testGenerateRsaKeyAndSignAllowedOnce(token) {
// Generates an RSA key pair and signs some data with it. Verifies the signature
// using WebCrypto. Verifies also that a second sign operation succeeds.
function testGenerateRsaKeyAndSignAllowedMultipleTimes(token) {
generateRsaKeyAndVerify(token, RSA_ALGORITHM, function(keyPair, spki) {
// Try to sign data with the same key a second time, which
// must succeed.
verifyRsaKeySign(
token, RSA_ALGORITHM, keyPair, spki,
/*debugMessage=*/ 'Second signing attempt', function(keyPair, spki) {
succeed();
});
});
generateRsaKeyAndVerify(
token, RSA_ALGORITHM, callbackPass(function(keyPair, spki) {
// Try to sign data with the same key a second time, which
// must succeed.
verifyRsaKeySign(
token, RSA_ALGORITHM, keyPair, spki,
/*debugMessage=*/ 'Second signing attempt',
callbackPass(function(keyPair, spki) {}));
}));
}
// Web Crypto ECDSA Operation Params.
......@@ -707,7 +708,6 @@ function testGenerateEcKeyAndSignAllowedOnce(token) {
assertEq(
'The operation failed for an operation-specific reason',
error.message);
succeed();
}));
}));
}
......@@ -716,13 +716,13 @@ function testGenerateEcKeyAndSignAllowedOnce(token) {
// Verifies the signature using WebCrypto. Verifies also that a second sign
// operation succeeds.
function testGenerateEcKeyAndSignAllowedMultipleTimes(token) {
generateEcKeyAndVerify(token, ALL_ECDSA_PARAMS, function(keyPair, spki) {
verifyEcKeySign(
token, ALL_ECDSA_PARAMS, keyPair, spki,
/*debugMessage=*/ 'Second signing attempt', function(keyPair, spki) {
succeed();
});
});
generateEcKeyAndVerify(
token, ALL_ECDSA_PARAMS, callbackPass(function(keyPair, spki) {
verifyEcKeySign(
token, ALL_ECDSA_PARAMS, keyPair, spki,
/*debugMessage=*/ 'Second signing attempt',
callbackPass(function(keyPair, spki) {}));
}));
}
// Generates a key and signs some data with other parameters. Verifies the
......@@ -739,9 +739,8 @@ function testGenerateKeyAndSignOtherParameters(token) {
}
};
generateRsaKeyAndVerify(token, algorithm, function(keyPair, spki) {
succeed();
});
generateRsaKeyAndVerify(
token, algorithm, callbackPass(function(keyPair, spki) {}));
}
// Call generate key with invalid algorithm parameter, missing
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment