Measure entire X25519 work for easy comparison with ECDH#821
Merged
torben-hansen merged 2 commits intoaws:mainfrom Feb 21, 2023
Merged
Measure entire X25519 work for easy comparison with ECDH#821torben-hansen merged 2 commits intoaws:mainfrom
torben-hansen merged 2 commits intoaws:mainfrom
Conversation
andrewhop
reviewed
Feb 19, 2023
dkostic
previously approved these changes
Feb 20, 2023
andrewhop
approved these changes
Feb 21, 2023
dkostic
approved these changes
Feb 21, 2023
skmcgrail
added a commit
that referenced
this pull request
Feb 23, 2023
* Emphasize that bindings are autogenerated (#778) * Add Rust Docker Images README.md for AL2/Linux (#780) Co-authored-by: Justin W Smith <103147162+justsmth@users.noreply.github.com> * Add CI that uses M1 Mac ec2 Instance (#773) The framework of this was taken from our original benchmarking framework. The CDK stack creates a Codebuild, Cloudwatch, and M1 ec2 instance resources, while we use the aws cli to spin up an M1 ec2. When a Codebuild job is kicked off, we use SSM (Systems Manager) to send a command document to our M1 ec2. The results of this can be observed in a Cloudwatch URL printed on the Console. I envision this CI stack as something temporary we can use to address our M1 CI gap. The end goal would be for us to migrate to GIthub Action's M1 Mac workflows when they make that available. This seems to be very near on the horizon (set for Q3 2023). * Small fix for M1 ec2 CI script (#782) * API diff during Rust crate generation (#781) * API diff during Rust crate generation * Verify version change * aws-lc commit hash as crate metadata * Get commit hash from cloned repo * Logic for FIPS crate generation is on the main branch * Move x86 valgrind to AL2022, fix known valgrind supression issues (#779) * Turn on static FIPS build for Android (#752) * Improve X.509 GeneralName parsing. (#789) Fixes a type confusion related to CVE-2023-0286. * Bump release version to v1.4.0 (#792) * Update Rust crate bindgen (#794) * Use CAVP test data for SSHKDF KATs (#777) * Use CAVP test data for SSHKDF KATs * Update pre-generated crypto test data for SSHKDF * update ssm document to handle s3 files (#787) * Fix/build on openbsd (#764) * fix: googletest compile error on OpenBSD `-Wvla` triggers an error when compiling gtest-port.cc. For now, remove the offending code. ``` third_party/googletest/src/gtest-port.cc:205:26: error: variable length arrays are a C99 feature [-Werror,-Wvla-extension] struct kinfo_proc info[mib[5]]; ^~~~~~ ``` * fix: OpenBSD: unknown type name 'pthread_rwlock_t' --------- Co-authored-by: Andrew Hopkins <andhop@amazon.com> Co-authored-by: torben-hansen <50673096+torben-hansen@users.noreply.github.com> Co-authored-by: Samuel Chiang <sachiang@amazon.com> * Drop the preference for 256-bit ciphers with CECPQ2. I did this because I was tired of explaining Grover's algorithm and circuit depth, but it never large amounts of sense and it conflates any measurements of post-quantum impact. If you want to configure a server with a preference for 256-bit ciphers, that's still completely possible. Change-Id: I3dc951ec724a713bb4da75c204d1105c62de8d74 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55929 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com> (cherry picked from commit ec6425ca2a85389606105c71be4d4ceb01621e7d) * Remove hmac.h include from ssl.h. This workaround was added in https://boringssl-review.googlesource.com/21664, but the correct <openssl/hmac.h> include was added to NGINX over 5 years ago in https://hg.nginx.org/nginx/rev/8076ba459f05, so this is no longer needed. Change-Id: I30571871b336e1f68d385202bcc8836a621e0204 Signed-off-by: Piotr Sikora <piotr@aviatrix.com> Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56085 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 05b360d797353dd19842aa5a38dbccbf1c867f21) * Add over_message issuance and redemption to Trust Tokens. This adds function to allow for issuing and redeeming tokens derived from a particular message rather than a completely random nonce. Change-Id: Ia29ae06ca419405ffff79ab6defadbed4f184b29 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55565 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Steven Valdez <svaldez@google.com> (cherry picked from commit fa4555a8b665b222e5d73d4cda3d4bbfa8518457) * Add sk_FOO_delete_if. Change-Id: I9abfef8d3d4d3ce0dabe29a5dc391fd4e0200d7f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56030 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit 49e07914b396fabd7f372d872de23e583eb82cdc) * Empty stacks are vacuously sorted In the X.509 policy rewrite, I'll be using sorted stacks to keep the overall algorithm subquadratic. Fix up sk_FOO_is_sorted in these edge cases so the asserts work more smoothly. Change-Id: I369f53543f0c2219df6f62a81aead630a9dbcd8d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56031 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit ff23b7cb2c5bec11044d92dca8fa7d3ca0ec5fbc) * Further const-correct config-based extension creation. Constructing extensions from a config file should not modify the config file or input certificates, so everything here should be const. While I'm here, fix various missing sk_FOO_push malloc failure checks. Change-Id: Ic29b21248a9d9243050c771fd0ce1e1d77f7ce7f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56027 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 44b3a283174cbfdf3c67ecd9cb515a4a8e029b60) * Remove the last of the filename comments. We've removed them in other files. Change-Id: I14ea99c85fd3a21094beeac88cb669e7aa9e2763 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56028 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 974083f2afe32b54645cecee013092f531836027) * Deprecate, test, and document X.509 config APIs. These APIs should not be used, but far too many callers use them. In the meantime, at least test this behavior (so it can be rewritten) and write down why it should not be used. In doing so, I noticed that we actually broke some cases in the ASN1_generate_v3 logic. I think it broke in https://boringssl-review.googlesource.com/c/boringssl/+/48825. But since no one's noticed, I've just kept it broken. Bug: 430 Change-Id: I80ab1985964fc506c8aead579c769f15291b1384 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56029 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit 7c81c5faec7730fc232f278c1ac9977338cdecc0) * Const-correct sk_FOO_deep_copy's copy callback. This aligns with upstream OpenSSL, so it's hopefully more compatible. Code search says no one outside of the project uses this function, so it's unlikely to break anyone. Whether it makes things better is a bit of a wash: OBJ_dup and OPENSSL_strdup loose a pointless wrapper. X509_NAME_dup gains one, but hopefully that can be resolved once we solve the X509_NAME const-correctness problem. CRYPTO_BUFFER_up_ref gains one... really FOO_up_ref should have type const T * -> T *, but OpenSSL decided it returns int, so we've got to cast. Change-Id: Ifa6eaf26777ac7239db6021fc1eafcaed98e42c4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56032 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit df8a55bf622a495b2dd07f8ecf697d5642ee6be2) * Remove no-op entries from asn1_str2tag. They don't work because ASN1_mbstring_ncopy doesn't recognize them. Change-Id: Id036252f4c6790714a73c5d0666149e13050fd4a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56105 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 0d9208e8896a8989af859d490d54c3be3f5174ca) * Test that policy checking always succeeds with just a trust anchor Change-Id: I88354719ccefbe8750bf02e069afbe8ab68b48fb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56033 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit 1f2529d99d2dd7d569352ff4ef9b3ae295a47b4e) * Add optimised Aarch64 GCM. Cost: 6.3KiB, based on the size of the .o file. (The bssl tool size doesn't really change, probably due to padding somewhere.) This code originally came from ARM but David has merged the AES-128 and AES-256 specific code into a function that works across AES sizes. Speeds from an M1 Pro: Did 16546000 AES-128-GCM (16 bytes) seal operations in 1000018us (16545702.2 ops/sec): 264.7 MB/s Did 10450500 AES-128-GCM (256 bytes) seal operations in 1000011us (10450385.0 ops/sec): 2675.3 MB/s Did 2822500 AES-128-GCM (1350 bytes) seal operations in 1000042us (2822381.5 ops/sec): 3810.2 MB/s Did 547000 AES-128-GCM (8192 bytes) seal operations in 1000826us (546548.6 ops/sec): 4477.3 MB/s Did 279000 AES-128-GCM (16384 bytes) seal operations in 1000411us (278885.4 ops/sec): 4569.3 MB/s Did 16991250 AES-256-GCM (16 bytes) seal operations in 1000001us (16991233.0 ops/sec): 271.9 MB/s Did 9257000 AES-256-GCM (256 bytes) seal operations in 1000072us (9256333.5 ops/sec): 2369.6 MB/s Did 2398000 AES-256-GCM (1350 bytes) seal operations in 1000002us (2397995.2 ops/sec): 3237.3 MB/s Did 465000 AES-256-GCM (8192 bytes) seal operations in 1001108us (464485.4 ops/sec): 3805.1 MB/s Did 240000 AES-256-GCM (16384 bytes) seal operations in 1002704us (239352.8 ops/sec): 3921.6 MB/s Did 16670000 AES-128-GCM (16 bytes) seal operations in 1000054us (16669099.9 ops/sec): 266.7 MB/s Did 11450750 AES-128-GCM (256 bytes) seal operations in 1000014us (11450589.7 ops/sec): 2931.4 MB/s Did 3830000 AES-128-GCM (1350 bytes) seal operations in 1000097us (3829628.5 ops/sec): 5170.0 MB/s Did 790000 AES-128-GCM (8192 bytes) seal operations in 1000379us (789700.7 ops/sec): 6469.2 MB/s Did 400000 AES-128-GCM (16384 bytes) seal operations in 1000980us (399608.4 ops/sec): 6547.2 MB/s Did 16877000 AES-256-GCM (16 bytes) seal operations in 1000052us (16876122.4 ops/sec): 270.0 MB/s Did 10438000 AES-256-GCM (256 bytes) seal operations in 1000067us (10437300.7 ops/sec): 2671.9 MB/s Did 3419000 AES-256-GCM (1350 bytes) seal operations in 1000158us (3418459.9 ops/sec): 4614.9 MB/s Did 698000 AES-256-GCM (8192 bytes) seal operations in 1000557us (697611.4 ops/sec): 5714.8 MB/s Did 355000 AES-256-GCM (16384 bytes) seal operations in 1001900us (354326.8 ops/sec): 5805.3 MB/s Change-Id: Id88f6e14482f09591fe95145bf4089de1ab68380 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55926 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: David Benjamin <davidben@google.com> (cherry picked from commit c6e37807639a5ed33fc2bbd8695b104d915a589e) * Update build files in generated-src * Stop clang from un-constant-timing copy_from_prebuf. Newer versions of clang figure out that copy_from_prebuf (used in builds that aren't x86_64 with assembly optimizations) has a bunch of no-op iterations and insert a branch. Add a value barrier to stop it. This was caught by our valgrind-based constant-time validation. As part of this, I noticed that OPENSSL_NO_ASM builds turn off value barriers. This is because the value barriers use an empty inline asm block. While this is technically correct, it's probably unnecessary. The clang|gcc check means we know GCC-style inline assembly is supported. Disabling inline asm is used by sanitizers to shut off unintrumentable code, but there's no uninstrumentable code in the empty string. It's also used by consumers who haven't figured out how to integrate an assembler into their build system, but that also doesn't apply. So just remove the condition on the value barriers so OPENSSL_NO_ASM also get mitigations. Update-Note: It is possible the above is wrong and some OPENSSL_NO_ASM relied on value barriers being disabled. If so, this will break that build and we'll need to reconsider. Change-Id: I6e3ea3ee705bef3afcf42d3532b17aaabbbcc60b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56827 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 53b876a4d15da0145495ec3a17521a2690fe5978) * Avoid branches in GCC in bn/generic.c. bn/generic.c is used for functions like bn_add_words, when there is no assembly implementation available. They're meant to be constant-time, but are particularly dependent on the compiler in this. I ran our valgrind-based tooling and found a couple issues in GCC: First, the various mul_add and sqr_add macros end up branching in GCC. Replacing the conditionals with expressions like c += (a < b) seems to mitigate this. Second, bn_sub_words produces branches in GCC. Replacing the expressions with bit expressions involving the boolean comparisons seems to work for now. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79173 discusses problems with GCC here, which seem to be as yet unresolved. Clang already reliably avoided branches in all of these. (Though it still spills the carry flag far more than would be ideal.) I also checked in godbolt that the new versions didn't generate branches in MSVC, but we don't have tooling to validate this as rigorously. Change-Id: I739758a396fb5ee27fb88bee71bd13ae9cb92bd0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56967 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit d4396e387c820198f509a6927facea84592cddd8) * Different symbols prefix for FIPS (#798) * Adding Kyber768 and Kyber1024 (round 3) KEMs (#784) Co-authored-by: dkostic <dkostic@amazon.com> * migrate over to aws organization (#809) * Allow override of git clone URL and branch to allow generation of crate using local changes (#801) * Allow override of git clone URL and branch to allow generation of crate using local changes * Feedback for prompt checkout url and branch * Prompt when non-default git url or branch * Fix execute permission bits after recent merge * Fix crash if '@section' is used with no CONF. Our NCONF_get_section doesn't silently handle NULLs, so add a check to the caller. Change-Id: I133d10c7a5dec22469a80b78cb45f479f128ada2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56106 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 871704b873e356b9eee354b70bf0f11cf234a1f7) * Handle errors more normally in asn1_gen.c. Functions typically just push their own error codes. Change-Id: Iac83bfdf56ce436bd3a9b8af5c43ca67fb4b9b15 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56107 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit db38fc55939095dd8082e0a89b0c9d187ac164f6) * Rename CTX_TEST to X509V3_CTX_TEST. Squat fewer unprefixed macros. Update-Note: CTX_TEST appears to be unused. If affected, switch to using X509V3_set_ctx_test instead. Change-Id: I43b86c0b6f147bbca85b8bc6b43602fc4f6697c1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56108 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 65ad925f51d879f8babd22ecb2ca4dc779b2cc24) * Reduce caller requirements on X509V3_CTX. This relaxes two caller requirements: First, although one needs to initialize X509V3_CTX in two parts, some callers forget to this. This works some of the time on accident, because most codepaths read ctx->db. But if one were to read it, it'd be uninitialized. Since all the entrypoints take a CONF anyway, and always match them, just implicitly initialize the CONF half of the X509V3_CTX with the provided one. Second, allow X509V3_CTX to be NULL. Some codepaths in the library check for NULL (or don't use it) and some do not. Enough codepaths don't check that it really cannot be considered to work, but enough do that a caller could mistakenly pass in NULL and have it mostly work. I've seen one caller mistakenly do this. Since we have to copy the X509V3_CTX for the first relaxation anyway, allow it to be NULL and fill in an empty one when omitted. Update-Note: If using different CONFs in the X509V3_CTX and the function parameter, the function parameter is now always used. No callers do this, and it's somewhat arbitrary which is used. (The generic code always uses the one in ctx. The @section syntax uses the parameter. Then the per-extension callbacks use the ctx.) Change-Id: I9fc15a581ea375ea06c4b082dcf0d6360be8144f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56109 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit 114fa727b7281bf532d037036356359619c730be) * Indent DB lines in x86_64 NASM output. This is purely cosmetic, but makes it consistent with the DW, etc., lines in the block above. The SEH unwind code support will emit a mix of DW and DB directives and this makes them look more consistent. Bug: 259 Change-Id: Ia16166ab2495aa813d6076d55af5b62511933c28 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56125 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit aa18fe24cdbe4d0b353cd3d82a6a5017af88ccb1) * Rewrite X.509 policy tree logic. This reimplements policy handling using a similar DAG structure as in https://chromium-review.googlesource.com/c/chromium/src/+/4111415. The main difference is that, being C, we don't have std::set or std::map easily available. But the algorithm can be implemented purely with sorted lists, while remaining subquadratic. This implementation relies on two assumptions: 1. We do not return the policy tree. This was removed in https://boringssl-review.googlesource.com/c/boringssl/+/53327 2. We do not return the final set of certificate policies. I.e., certificate policy checking is only used for evaluating policy constraints and X509_V_FLAG_EXPLICIT_POLICY. The second assumption is not very important. It mostly simplifies has_explicit_policy slightly. In addition, this new implementation removes the per-certificate policy cache. Instead, we just process the policy extensions anew on certificate verification. This avoids a mess of threading complexity, including a race condition in the old logic. See https://boringssl-review.googlesource.com/c/boringssl/+/55762 for a description of the race condition. Change-Id: Ifba9037588ecff5eb6ed3c34c8bd7611f60013a6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56036 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> (cherry picked from commit 029d0e77fb64625469cc02c8df26767c72081dfd) * Add initial support for SEH directives in x86_64 perlasm This implements similar directives as MASM, so we do not need to build all the structures by hand. It does not provide any help to abstract between Win64 and SysV differences, however. This is pulled together from some old draft CLs I had, one of which actually synthesized CFI directives from SEH, so it should be possible. I've intentionally omitted that however, as it also brings in questions about how to handle the calling convention differences (the existing machinery won't *quite* work). I've uploaded just this for now, so review can focus on the basic mechanism. I've also preserved perlasm's weird mixed tabs and spaces indentation convention for now, though it is a bit tedious. Bug: 259 Change-Id: Ib3f46a27751a5319b758d12c462c660cf9f3e632 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56126 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> (cherry picked from commit c556ee91ff7fc106baadb41ddaaae951ac9b3ba2) * Convert ghash-x86_64.pl to new directives. The ABI test already uses CHECK_ABI_SEH, so this is covered under tests. As part of the conversion, we no longer rely on the assembler generating a specific encoding and can just write the code normally. Bug: 259 Change-Id: I47cbf81073237f2b95971a782848b85d230b6bf6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56127 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> (cherry picked from commit ae1546b6f3bf1ad7eb24b491c914eb202b5547d3) * Don't automatically sync the two CONF parameters in X509V3_EXT_nconf. https://boringssl-review.googlesource.com/c/boringssl/+/56109 tried to simplify the X509V3_CTX story by automatically handling the second half of initialization, but it turns out not all callers specify both values. Instead, align with OpenSSL 3.0's behavior. Now X509V3_set_ctx implicitly zeros the other fields, so it is the only mandatory init function. This does mean callers which call X509V3_set_nconf before X509V3_set_ctx will break, but that's true in OpenSSL 3.0 too. I've retained the allowance for ctx being NULL, because whether functions tolerate that or not is still a bit inconsistent. Also added some TODOs about how strange this behavior is, but it's probably not worth spending much more time on this code. Change-Id: Ia04cf11eb5158374ca186795b7e579575e80666f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56265 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> (cherry picked from commit 1c9d18307bdf75bee7f3ec7f7d2e421a734831f1) * Maintain a frame pointer in aesni-gcm-x86_64.pl and add SEH unwind codes Some profiling systems cannot unwind with CFI and benefit from having a frame pointer. Since this code doesn't have enough register pressure to actually need to use rbp as a general register, this change tweaks things so that a frame pointer is preserved. As this would invalidate the SEH handler, just replace it with proper unwind codes, which are more profiler-friendly and supportable by our unwind tests. Some notes on this: - We don't currently support the automatic calling convention conversion with unwind codes, but this file already puts all arguments in registers, so I just renamed the arguments and put the last two arguments in RDI and RSI. Those I stashed into the parameter stack area because it's free storage. - It is tedious to write the same directives in both CFI and SEH. We really could do with an abstraction. Although since most of our functions need a Windows variation anyway. - I restored the original file's use of PUSH to save the registers. This matches what Clang likes to output anyway, and push is probably smaller than the corresponding move with offset. (And it reduces how much thinking about offsets I need to do.) - Although it's an extra instruction, I restored the original file's separate fixed stack allocation and alloca for the sake of clarity. - The epilog is constrained by Windows being extremely picky about epilogs. (Windows doesn't annotate epilogs and instead simulates forward.) I think other options are possible, but using LEA with an offset to realign the stack for the POPs both matches the examples in Windows and what Clang seems to like to output. The original file used MOV with offset, but it seems to be related to the funny SEH handler. - The offsets in SEH directives may be surprising to someone used to CFI directives or a SysV RBP frame pointer. All three use slightly different baselines: CFI's canonical frame address (CFA) is RSP just before a CALL (so before the saved RIP in stack order). It is 16-byte aligned by ABI. A SysV RBP frame pointer is 16 bytes after that, after a saved RIP and saved RBP. It is also 16-byte aligned. Windows' baseline is the top of the fixed stack allocation, so potentially some bytes after that (all pushreg and allocstack directives). This too is required to be 16-byte aligned. Windows, however, doesn't require the frame register actually contain the fixed stack allocation. You can specify an offset from the value in the register to the actual top. But all the offsets in savereg, etc., directives use this baseline. Performance difference is within measurement noise. This does not create a stack frame for internal functions so frame-pointer unwinding may miss a function or two, but the broad attribution will be correct. Change originally by Clemens Fruhwirth. Then reworked from Adam Langley's https://boringssl-review.googlesource.com/c/boringssl/+/55945 by me to work on Windows and fix up some issues with the RBP setup. Bug: b/33072965, 259 Change-Id: I52302635a8ad3d9272404feac125e2a4a4a5d14c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56128 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit 0d5b6086143d19f86cc5d01b8944a1c13f99be24) * Fix inhibitPolicyMapping in the new policy tree code. There was a typo and inhibitPolicyMapping updated the wrong value. With this fixed, we pass the PKITS tests (as imported into Chromium). NOTE: Imported the fix only, skipped the new tests because they depend on some previously skipped commits. Added action items to the backlog. Change-Id: I3b80eb56561ae5ae88023fa639d697a9f1757b21 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56205 Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit ba68ca070ca939bc2d6b8f07bd64909bd90b25a5) * Add bash script to install common depednecies, update version of Go everywhere to 1.20.1 (#811) * Add FIPS CI for all linux dimensions (#783) * Allow keys without e in RSA_set0_key (#814) * Allow keys without e in RSA_set0_key In a [previous pull request][1], we added support for JCA-style "stripped" RSA private keys defined only in terms of modulus `d` and private exponent `e`. When that change was made, ACCP only gave these keys to AWS-LC in the form of a DER-encoded ASN.1 structure. Consequently, we only adjusted the key-decoding code path to account for these stripped keys. This changed, however, when ACCP implemented native KeyFactory support. That change [uses][3] AWS-LC's `RSA_set0_key` function to create RSA private keys. Because the key factory branch predated ACCP's migration from OpenSSL to AWS-LC, it [relied][4] on OpenSSL's behavior of treating 0-valued public exponents as absent in order to specify JCA-style stripped keys. Because these stripped keys don't supply the public exponent `e`, the caller must turn off blinding. Some historical commits ([here][5], [here][6]) reference this need for `e` to perform blinding; apparently the `RSA_set0_key` behavior assumed that blinding would always be performed when that function was used to construct the key. Previously, the `RSA_set*` methods didn't have any coverage, so we add a few test cases to cover the source code affected by this change. [1]: #436 [2]: corretto/amazon-corretto-crypto-provider#132 [3]: https://github.com/corretto/amazon-corretto-crypto-provider/blob/8d32fe3fb685875c9e7418fdb30f2d70a8c4d80b/csrc/java_evp_keys.cpp#L588 [4]: https://github.com/corretto/amazon-corretto-crypto-provider/blob/8d32fe3fb685875c9e7418fdb30f2d70a8c4d80b/csrc/java_evp_keys.cpp#L578-L580 [5]: 86361a3 [6]: 598e55a * Wrap reference exponents in BN_dup * Update RSA_set0_key doc comment * Measure entire X25519 work for easy comparison with ECDH (#821) For e.g. TLS, the entire flow is really "public key" generation and computation of the shared secret. For X25519, the AWS-LC functions doing that are: `X25519_public_from_private()` and `X25519()`. Where the latter also includes the randomness generation work. But to make comparison with e.g. `bssl speed -filter ECDH` easy, we should output the performance of the entire flow as well. * Migrate io/ioutil uses to new APIs. (#816) ioutil has been deprecated since Go 1.16. The functions were moved to some combination of io and os. See https://pkg.go.dev/io/ioutil. (File-related functions went to os. Generic things went to io. Names were kept the same except TempDir and TempFile are os.MkdirTemp and os.CreateTemp, respectively.) Change-Id: I031306f69e70424841df08f64fa9d90f31780928 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55186 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> Co-authored-by: David Benjamin <davidben@google.com> * Move all fuzzing to al2022 with clang 14 (#815) * Use PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR in CMakeLists.txt (#826) * Use PROJECT_SOURCE_DIR rather than CMAKE_SOURCE_DIR * Use PROJECT_BINARY_DIR rather than CMAKE_BINARY_DIR * Unexport BN_MONT_CTX_set_locked. The only callers of this function were reaching into RSA internals. We cannot fix all the issues with RSA state management when callers do this. Those have since been fixed, so unexport this function. Update-Note: This removes a function that can only be used by accessing one of BoringSSL's private locks. Change-Id: I0f067b5650ead38d2dbb7302bad4ddd0b2512458 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56286 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> (cherry picked from commit eefe6cf27cad9f8303743208f3886cc3f128de53) * Add a WORKSPACE.toplevel file The WORKSPACE file in master-with-bazel is currently synthesized by the script. If we end up with something like https://boringssl-review.googlesource.com/c/boringssl/+/55965, we'll need to be able to change it more easily. Make it the same way as BUILD. Change-Id: I12a91e9b346f4bd49bfb43ac7e4242e6ce37054c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56306 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> (cherry picked from commit dbbd79e89526f68589064e488b900cb1aedd7422) * Don't send two post-quantum initial key shares. More than one post-quantum group is now defined so it would be possible for two PQ groups to be 1st and 2nd preferences. In that case, we probably don't want to send two PQ initial key shares. (Only one PQ group is _implemented_ currently, so we can't write a test for this.) Change-Id: I51ff118f224153e09a0c3ee8b142aebb6b340dcb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56226 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> (cherry picked from commit 1e97ce3beab9e6d34bc645f6901b2b30b50530c0) * Work around nasm bug with empty assembly files If you pass an empty assembly file into nasm, it crashes. Add a dummy instruction which the static linker will hopefully dropped. (This is a no-op unless you try to link all the assembly files together for a simpler build.) Bug: 542 Change-Id: Idd2b96c129a3a39d5f21e3905762cc34c720f6b2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56326 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> (cherry picked from commit a43c76dbe30d619188dc685b7d432a92e7c2b66b) * Use the same Deleter across all bssl::UniquePtr<T>. Template operator() instead of the type. This fixes converting subclasses with bssl::UniquePtr. std::unique_ptr<T, D> can be converted to std::unique_ptr<U, E> requires either D == E or for D to be implicitly convertable to E, along with other conditions. (Notably T* must be convertible to U*.) In the real std::unique_ptr, we rely on std::default_delete<T> being convertable to std::default_delete<U> if T* is convertible to U*. But rather than write all the SFINAE complexity, I think it suffices to move the template down a later. This simplifies SSLKeyShare::Create a little. Change-Id: I431610f3a69a72dd9def190d3554c89c2d3a4c32 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56385 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> (cherry picked from commit 971b330faf96328618d3653667af65c9f753e0df) * Update build files in generated-src * Fix BORINGSSL_PREFIX_HEADERS_PATH not included in cpreprocess for FIPS (#829) * Merge PKCS#8 V2 (RFC 5958) Support into main (#828) --------- Co-authored-by: Justin W Smith <103147162+justsmth@users.noreply.github.com> Co-authored-by: Samuel Chiang <sachiang@amazon.com> Co-authored-by: Andrew Hopkins <andhop@amazon.com> Co-authored-by: torben-hansen <50673096+torben-hansen@users.noreply.github.com> Co-authored-by: Chris Herborth <chrish@pobox.com> Co-authored-by: Joel Knight <knight.joel@gmail.com> Co-authored-by: Adam Langley <agl@imperialviolet.org> Co-authored-by: Piotr Sikora <piotr@aviatrix.com> Co-authored-by: Steven Valdez <svaldez@google.com> Co-authored-by: David Benjamin <davidben@google.com> Co-authored-by: dkostic <25055813+dkostic@users.noreply.github.com> Co-authored-by: dkostic <dkostic@amazon.com> Co-authored-by: Will Childs-Klein <childw@amazon.com> Co-authored-by: Adam Langley <agl@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues:
Related: CryptoAlg-1587
Description of changes:
For e.g. TLS, the entire flow is really "public key" generation and computation of the shared secret. For X25519, the AWS-LC functions doing that are:
X25519_public_from_private()andX25519(). Where the latter also includes the randomness generation work.ATM, we only repot these individually. But to make comparison with e.g.
bssl speed -filter ECDHeasy, we should output the performance of the entire flow as well.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and
the ISC license.