Add console logging to encourage the use of authenticated encryption in WebCrypto
Created attachment 435096 [details] Patch
rdar://80655397
Comment on attachment 435096 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435096&action=review r=me > Source/WebCore/crypto/SubtleCrypto.cpp:538 > + scriptExecutionContext()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "AES-CBC and AES-CTR do not provide authentication by default, and implementing it manually can result in minor, but serious mistakes. We recommended using authenticated encryption like GCM to protect against chosen-ciphertext attacks."); This might be better as a separate function: void SubtleCrypto::addAuthenticatedEncryptionWarningIfNecessary() { if (key.algorithmIdentifier() == CryptoAlgorithmIdentifier::AES_CBC || key.algorithmIdentifier() == CryptoAlgorithmIdentifier::AES_CTR) scriptExecutionContext()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "AES-CBC and AES-CTR do not provide authentication by default, and implementing it manually can result in minor, but serious mistakes. We recommended using authenticated encryption like GCM to protect against chosen-ciphertext attacks."); } Then the two implementations would always be consistent in language and format, and if we want to warn about more things we could just add them in one place. > Source/WebCore/crypto/SubtleCrypto.cpp:579 > + scriptExecutionContext()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "AES-CBC and AES-CTR do not provide authentication by default, and implementing it manually can result in minor, but serious mistakes. We recommended using authenticated encryption like GCM to protect against chosen-ciphertext attacks."); ... and this could just be replaced with: addAuthenticatedEncryptionWarningIfNecessary();
(In reply to Brent Fulgham from comment #3) > Comment on attachment 435096 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=435096&action=review > > r=me > > > Source/WebCore/crypto/SubtleCrypto.cpp:538 > > + scriptExecutionContext()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "AES-CBC and AES-CTR do not provide authentication by default, and implementing it manually can result in minor, but serious mistakes. We recommended using authenticated encryption like GCM to protect against chosen-ciphertext attacks."); > > This might be better as a separate function: > > void SubtleCrypto::addAuthenticatedEncryptionWarningIfNecessary() > { > if (key.algorithmIdentifier() == CryptoAlgorithmIdentifier::AES_CBC || > key.algorithmIdentifier() == CryptoAlgorithmIdentifier::AES_CTR) > scriptExecutionContext()->addConsoleMessage(MessageSource::Security, > MessageLevel::Warning, "AES-CBC and AES-CTR do not provide authentication by > default, and implementing it manually can result in minor, but serious > mistakes. We recommended using authenticated encryption like GCM to protect > against chosen-ciphertext attacks."); > } > > Then the two implementations would always be consistent in language and > format, and if we want to warn about more things we could just add them in > one place. > > > Source/WebCore/crypto/SubtleCrypto.cpp:579 > > + scriptExecutionContext()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "AES-CBC and AES-CTR do not provide authentication by default, and implementing it manually can result in minor, but serious mistakes. We recommended using authenticated encryption like GCM to protect against chosen-ciphertext attacks."); > > ... and this could just be replaced with: > > addAuthenticatedEncryptionWarningIfNecessary(); Good idea, will change.
Created attachment 435097 [details] Patch
Will land after EWS is green.
Created attachment 435188 [details] Patch
Updated test expectations for imported tests to avoid logging console output to stdout. Also some tests called encrypt/decrypt up to 1000 times to test for crashing. To avoid 1000 lines of console logging in test expectations I also marked these with the "DumpJSConsoleLogInStdErr" expectation. Awaiting green bots.
Created attachment 435195 [details] Patch
Committed r280790 (240369@main): <https://commits.webkit.org/240369@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435195 [details].