)]}' { "commit": "6c95434cc9876dfe1cdc375c5ff04e70ebfe3770", "tree": "61f95376e521444e93951d3e2eee5ad00b2347a0", "parents": [ "b3ac6bb39ad3f980dccae24dfacd97b6e3e57391" ], "author": { "name": "David Benjamin", "email": "davidben@google.com", "time": "Fri Jan 03 17:47:07 2020 +0000" }, "committer": { "name": "CQ bot account: commit-bot@chromium.org", "email": "commit-bot@chromium.org", "time": "Fri Jan 03 17:47:29 2020 +0000" }, "message": "Revert \"Replace aes_nohw with a bitsliced implementation.\"\n\nThis reverts commit b3ac6bb39ad3f980dccae24dfacd97b6e3e57391.\n\nReason for revert: 32-bit version seems to be broken. I\u0027ll debug this\nand improve pre-commit CQ coverage before relanding.\n\nOriginal change\u0027s description:\n\u003e Replace aes_nohw with a bitsliced implementation.\n\u003e \n\u003e aes_nohw is currently one of several variable-time table-based\n\u003e implementations in C or assembly (armv4, x86, and x86_64). Replace all\n\u003e of these with a C bitsliced implementation, with 32-bit, 64-bit, and\n\u003e 128-bit (SSE2) variants. This is based on the algorithms described in:\n\u003e \n\u003e https://bearssl.org/constanttime.html#aes\n\u003e https://eprint.iacr.org/2009/129.pdf\n\u003e https://eprint.iacr.org/2009/191.pdf\n\u003e \n\u003e This makes our AES implementation constant-time in all build\n\u003e configurations!\n\u003e \n\u003e There were far too many benchmarks to put in the commit message.\n\u003e Instead, please refer to this fancy spreadsheet:\n\u003e https://docs.google.com/spreadsheets/d/1wDCzfkPl7brfjWJKq55awQjwCPhOYI8O7zSQZuEc2Xg/edit?usp\u003dsharing\n\u003e \n\u003e Parallel modes on x86 and x86_64 do fine due to the SSE2 code. AES-GCM\n\u003e actually gets faster. The 64-bit (4x) bitsliced implementation is less\n\u003e effective at speeding parallel modes but still helps. The 32-bit (2x)\n\u003e bitsliced implementation even less.\n\u003e \n\u003e Non-parallel modes, sadly, take a *dramatic* performance hit. I tried a\n\u003e constant-time table lookup for comparison, but bitslicing was still\n\u003e better. This implementation performs comparably to the table in\n\u003e BearSSL\u0027s documentation, which suggests I didn\u0027t do anything obviously\n\u003e wrong. (Note BearSSL\u0027s table for \u0027ct\u0027 corresponds to a 32-bit bitsliced\n\u003e implementation compiled for 64-bit. Compiling this implementation for\n\u003e 64-bit matches, but compiling it for 32-bit seems to be considerably\n\u003e slower.)\n\u003e \n\u003e Assumptions that may make this palatable:\n\u003e \n\u003e - AES-GCM is by far the most important AES mode, and we perform okay\n\u003e with it. Modern things aren\u0027t built out of CBC.\n\u003e \n\u003e - A nontrivial chunk of Chrome users on Windows don\u0027t have SSSE3 and\n\u003e would be affected by this change. They would get the SSE2 version\n\u003e which performs well for AES-GCM *and* is constant-time.\n\u003e \n\u003e - ARM devices are primarily mobile which cycles hardware much faster.\n\u003e Chrome for Android has required NEON for several years now, so it\n\u003e would not run this code. (Aside from https://crbug.com/341598.)\n\u003e \n\u003e - aarch64 mandates NEON, so it would not run this code.\n\u003e \n\u003e - QUIC packet number encryption does use a one-off block operation, but\n\u003e only once per packet.\n\u003e \n\u003e - Arguably this is undoing a performance gain that we never earned. That\n\u003e said, it was a dramatic performance gain in places.\n\u003e \n\u003e As an alternative, we could just check in the SSE2 version and drop the\n\u003e x86 and x86_64 table-based assembly, but this still leaves the generic\n\u003e code with cache-timing side channels.\n\u003e \n\u003e Change-Id: I0f4b4467a49790509503c529d7c0940318096a00\n\u003e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39206\n\u003e Commit-Queue: Adam Langley \u003cagl@google.com\u003e\n\u003e Reviewed-by: Adam Langley \u003cagl@google.com\u003e\n\nTBR\u003dagl@google.com,davidben@google.com\n\nChange-Id: Iffaf01a98ab40bbfa009c451aa20ba3eb923eab9\nNo-Presubmit: true\nNo-Tree-Checks: true\nNo-Try: true\nReviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39285\nReviewed-by: David Benjamin \u003cdavidben@google.com\u003e\nCommit-Queue: David Benjamin \u003cdavidben@google.com\u003e\n", "tree_diff": [ { "type": "modify", "old_id": "d7175723cf0bcf390c98322eb4d3fbeee1447e98", "old_mode": 33188, "old_path": "crypto/cipher_extra/e_aesgcmsiv.c", "new_id": "64febae41715bdac063078ba526f640d31c0d818", "new_mode": 33188, "new_path": "crypto/cipher_extra/e_aesgcmsiv.c" }, { "type": "modify", "old_id": "a675fbdd54a4dd308f431c8189a915c1ed45497e", "old_mode": 33188, "old_path": "crypto/fipsmodule/CMakeLists.txt", "new_id": "3081a41219144df834de9fcb893710929ae32ad8", "new_mode": 33188, "new_path": "crypto/fipsmodule/CMakeLists.txt" }, { "type": "modify", "old_id": "f60281dfaf549b3467ed55edddd9ea577c890e1e", "old_mode": 33188, "old_path": "crypto/fipsmodule/aes/aes.c", "new_id": "48d60eeb648a274c62d841ae939130902784a716", "new_mode": 33188, "new_path": "crypto/fipsmodule/aes/aes.c" }, { "type": "delete", "old_id": "e3e4e285aa0a917c4618cc2a1033e9e9d21c8749", "old_mode": 33188, "old_path": "crypto/fipsmodule/aes/aes_nohw.c", "new_id": "0000000000000000000000000000000000000000", "new_mode": 0, "new_path": "/dev/null" }, { "type": "modify", "old_id": "fd7ce24d24334bb9f9edd4291f8bccd9030e4f92", "old_mode": 33188, "old_path": "crypto/fipsmodule/aes/aes_test.cc", "new_id": "4c913d3ca687cc52425d3a38542016afb34e0d99", "new_mode": 33188, "new_path": "crypto/fipsmodule/aes/aes_test.cc" }, { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "9b373de206433512734e5e44a053edaae7a25fd7", "new_mode": 33261, "new_path": "crypto/fipsmodule/aes/asm/aes-586.pl" }, { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "fbb199504e553307a1fe18980ce51fee2a49f3d7", "new_mode": 33188, "new_path": "crypto/fipsmodule/aes/asm/aes-armv4.pl" }, { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "5b95785e0bc398a933116c90d938045fc47cc52d", "new_mode": 33261, "new_path": "crypto/fipsmodule/aes/asm/aes-x86_64.pl" }, { "type": "modify", "old_id": "5b806955032726ba30e9e3ee1d87ada370fea3ac", "old_mode": 33188, "old_path": "crypto/fipsmodule/aes/internal.h", "new_id": "99d509a98394a80e4a1e6a3aae509b36139b01cc", "new_mode": 33188, "new_path": "crypto/fipsmodule/aes/internal.h" }, { "type": "modify", "old_id": "206fcfd49ec640d8a4f395c99c0a9e1ecac8ef32", "old_mode": 33188, "old_path": "crypto/fipsmodule/aes/mode_wrappers.c", "new_id": "ae8a91b1a6c535d39030ec1044a1e9faf2af1a04", "new_mode": 33188, "new_path": "crypto/fipsmodule/aes/mode_wrappers.c" }, { "type": "modify", "old_id": "567a0cdfa76d0e733d10be6104d5c3f10cdb7fce", "old_mode": 33188, "old_path": "crypto/fipsmodule/bcm.c", "new_id": "7485f6ccd928a31f2524b7183baa862cc3bfbf70", "new_mode": 33188, "new_path": "crypto/fipsmodule/bcm.c" }, { "type": "modify", "old_id": "8f4907f3a4a12681ab449131f2fb2f65a7bef2ba", "old_mode": 33188, "old_path": "crypto/fipsmodule/cipher/e_aes.c", "new_id": "c6dd973a5aa2900a1bb7ccb2a73952f42d04d500", "new_mode": 33188, "new_path": "crypto/fipsmodule/cipher/e_aes.c" } ] }