This code is currently only used in the p256.cc.inc (not nistz), but it
uses formulas similar to the nistz implementation. In particular, point
doubling is implemented using coordinate halving instead of multiplying
denominators by powers of two.
This change also removes the affine-addition optimization, counteracting
some of the efficiency gains but still resulting in an overall speedup
for client-side usage. This removal is intended to be temporary. The
verified affine-addition function uses a slightly different (nistz-like)
calling convention. Later work will unify the p256 and p256-nistz
files behind the new calling convention and restore the optimization
for both small and large tables.
Benchmarks on Zen 4:
../main/build-small/bssl.json
Did 14400 ECDH P-256 operations in 1000289us (14395.8 ops/sec)
Did 37000 ECDSA P-256 signing operations in 1009883us (36637.9 ops/sec)
Did 16000 ECDSA P-256 verify operations in 1003734us (15940.5 ops/sec)
build-small/bssl.json
Did 15886 ECDH P-256 operations in 1000894us (15871.8 ops/sec) [+10.3%]
Did 36000 ECDSA P-256 signing operations in 1017983us (35364.0 ops/sec) [-3.5%]
Did 18000 ECDSA P-256 verify operations in 1045215us (17221.3 ops/sec) [+8.0%]
P-256 ECDH is still ~40% slower in the small build than full build.
Estimating with double = 0.6 add, table size alone would yield ~30%.
Change-Id: Ide90296f9e14bf543df4e397f088602942c0b658
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79787
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Andres Erbsen <andreser@google.com>
This change adds the scaffolding to integrate code proven in Rocq using
Bedrock and Fiat Cryptography to libcrypto, in particular for optimized
elliptic-curve and finite-field arithmetic. A new implementation of p256
coordinate subtracton is added as an initial demonstration.
Change-Id: I73b3bd8258a33681a2e5b6532e389151e157985b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79208
Auto-Submit: Andres Erbsen <andreser@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Looks like it's meant to have a hyphen, to match the SPDX identifier.
https: //source.chromium.org/search?q=License%5C:%5C%20Apache%20file:README.chromium&sq=&ss=chromium
https: //chromium.googlesource.com/chromium/src/+/HEAD/docs/adding_to_third_party.md#add-a-license-file-and-run-related-checks
Change-Id: I553cb40dc0ef55a7a3634ac2488f389e1ed47dfc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77288
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
We use the standard Apache 2.0 file header, described in "APPENDIX: How
to apply the Apache License to your work."
This was primarily automated by running:
git ls-tree -r --name-only HEAD | xargs go run ./util/relicense.go
See go/boringssl-relicensing-triage for the results of triaging the
output of the tool.
As part of this, switch from taking fiat-crypto under MIT license to
Apache 2.0. (It is licensed under MIT OR Apache-2.0 OR BSD-1-Clause.)
The copyright_summary tool can also be used to confirm we didn't
accidentally drop any copyright lines:
# Run before the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -out /tmp/old.json
# Run after the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -compare /tmp/old.json
Bug: 364634028
Change-Id: I17c50e761e9d077a1f92e25969e50ed35e320c59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75852
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is deliberately not exported in the public interface. Obviously we
could add it in the future if we wished but, for now, our suggestion is
to use ML-DSA-65. However, if needed in the future, it will already be
ready and FIPS validated inside the module.
Change-Id: I38a59d4e58b76cf74004432e87143380691a722b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73347
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This change switches nearly all of BoringSSL to use C++. The public
functions still use the C ABI, and so code written in C can still use
BoringSSL. Also the use of the C++ standard library is minimal and no
run-time requirement for it is intended.
Change-Id: I902d2f51a3c8d6bd0dc4aabe1b192a15d7b788e8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72747
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
With -march=haswell -DOPENSSL_SMALL=1 on cascadelake:
Did 9999 ECDH P-256 operations in 1062469us (9411.1 ops/sec) [+63.5%]
Did 25000 ECDSA P-256 signing operations in 1028302us (24311.9 ops/sec) [+48.9%]
Did 11004 ECDSA P-256 verify operations in 1072646us (10258.7 ops/sec) [+58.8%]
Same configuration measured no performance difference on haswell.
The added assembly code occupies 1352 bytes.
Change-Id: I42635b7a9bf24d942817976a5d4ce269f642251c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63185
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Some tests in the Chromium verifier use gmock. Since upstream googletest
considers them a single project now, just import both of them.
Update-Note: As part of this, I've made gtest_main.cc now initializes
gmock alongside gtest. This means BoringSSL's test targets now depend on
both. Downstream builds where googletest and googlemock are separate
build targets may need to add a dependency. (Though every downstream
test I looked at treats them as one target, so this is most likely a
no-op.)
Bug: chromium:1322914
Change-Id: Ief38c675bc13a4639dee061d058580967ab99d41
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62945
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Although it works without these (we just refer the unwinder to the red
zone), older versions of libunwind seem to have a bug that cause it to
flakily fail to restore rbx without this. I've attempted to bisect the
problem, but the issue is very flaky and I've failed to find the culprit
four times now, so just give up and work around it. Explicit restores
match what we do in other files.
Hopefully this will clear some issues tha fiat-crypto's CI are running
into.
Change-Id: I6a19679a37cad8e93e6dee554b6a9b3b9b4bbe4a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62865
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
RBP pointed 8 bytes off of where it should be. I've left the RSP offsets
alone, though it does mean they're shifted by 8 from what they
previously were. Per Andres, the new version of CryptOpt will generate
an RBP-compatible prolog, but for now I've just fixed it up by hand.
(This part was already hand-written.)
Change-Id: I23720e76affff6fae46b8f85b0a509380ccc8bc0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62805
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This CL adds the Shipped field (and may update the
License File field) in Chromium READMEs. Changes were
automatically created, so if you disagree with any of
them (e.g. a package is used only for testing purposes
and is not shipped), comment the suggested change and
why.
See the LSC doc at go/lsc-chrome-metadata.
Bug: b:285450740
Change-Id: I63755d8a42ea69ff6d3a4e0c21ddacd2b7ce9053
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61545
Auto-Submit: Anne Redulla <aredulla@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This allows operating systems to insist on IBT
enforcement as an exploit mitigation mechanism without
needing to make an exception for anything using a
bundled boringssl, such as chrome, mono, and qtwebengine.
Change-Id: Iac28dd3d2af177b89ffde10ae97bce23739feb94
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60625
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Trying to migrate Chromium to the "link all the asm files together"
strategy broke the aarch64 Android build because some of the ifdef'd out
assembly files were missing the .note.gnu.property section for BTI. If
we add support for IBT, that'll be another one.
To fix this, introduce <openssl/asm_base.h>, which must be included at
the start of every assembly file (before the target ifdefs). This does a
couple things:
- It emits BTI and noexecstack markers into every assembly file, even
those that ifdef themselves out.
- It resolves the MSan -> OPENSSL_NO_ASM logic, so we only need to do it
once.
- It defines the same OPENSSL_X86_64, etc., defines we set elsewhere, so
we can ensure they're consistent.
This required carving files up a bit. <openssl/base.h> has a lot of
things, such that trying to guard everything in it on __ASSEMBLER__
would be tedious. Instead, I moved the target defines to a new
<openssl/target.h>. Then <openssl/asm_base.h> is the new header that
pulls in all those things.
Bug: 542
Change-Id: I1682b4d929adea72908655fa1bb15765a6b3473b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60765
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
__builtin_ia32_addcarryx_u64 is, strictly speaking, an ADX intrinsic.
GCC and newer Clang seem to actually implement it without ADX, but
Clang 7 and older will actually try to generate ADX code with it. But
since the caller is not marked target("adx"), this fails to build.
Manually add ADX and BMI2 target attributes to all these functions. The
compiler should be free to use those instructions as these functions all
call into an ADX+BMI2 assembly function anyway. (Though it doesn't do
much with this.)
Note we cannot just annotate fiat_addcarryx_u64. Clang and GCC won't
inline across incompatible targets, so if we tag fiat_addcarryx_u64, we
need to tag the callers up the chain until we're willing to stop
inlining.
Change-Id: I855bb88fea666d92997984836e664292d90df5be
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60612
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This fixes the generated Bazel build. Bazel is strict about having all
dependencies declared, which includes files that are #included into
other files. (It also is not particularly pleased about textual
headers and wants them declared in a separate place.)
The new fiat curve25519 assembly is currently split into a BoringSSL
half, and a more generic fiat half. For now, just move the BoringSSL
customizations directly into the fiat half. This isn't ideal, as we'd,
long term, like something where the fiat code can be made standalone.
But, to fix the build, just patch in the changes now and we can ponder
how to do this better later. (Build tools and conventions for assembly
are much less clear than C, sadly.)
Also add the .note.GNU-stack bit at the end.
Change-Id: I04aa733eabf8562dba42dee63a8fd25c86a59db9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60566
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
.type seems to be needed for the unwind tester to report symbols
correctly. I assume it does other useful things. .hidden
(.private_extern on macOS) keeps it from being exported out of shared
libraries. I'm not positive what .size does, but if ELF wants to know
the size of the function, we should probably tell it that.
Change-Id: Iaa2e4f8a72e81092ec1d81ee2177504c7dc89c76
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60465
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Andres Erbsen <andreser@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Did 75000 Ed25519 key generation operations in 1007110us (74470.5 ops/sec) [+26.9%]
Did 72000 Ed25519 signing operations in 1011133us (71207.2 ops/sec) [+25.5%]
Did 78000 Curve25519 base-point multiplication operations in 1006737us (77478.0 ops/sec) [+27.5%]
Change-Id: I32ca2056f42f9b92af315d8381e1b72be69dd331
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60386
Commit-Queue: Andres Erbsen <andreser@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
It's been a couple years since we did that.
I had to upstream a fix to CreateArgvFromArgs, but that landed quickly,
so we're actually carrying no googletest patches with this.
Change-Id: Ifaf3476f2079d444512a235756cfe54f492b9c92
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59745
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
The files no longer need to be patched because fiat-crypto now has its
own copy of our value barrier. It does, however, require syncing our
NO_ASM define with fiat's.
fiat-crypto is now licensed under any of MIT, BSD 1-clause, or Apache 2.
I've stuck with the MIT one as that's what we were previously importing.
No measurable perf difference before/after this CL, with GCC or Clang on
x86_64.
Change-Id: I2939fd517de37aabdea3ead49150135200a1b112
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52045
Reviewed-by: Adam Langley <agl@google.com>
Fuchsia is looking to remove the ZX_WAIT_ASYNC_ONCE constant, and our
copy of GoogleTest still has it. As usually with GoogleTest updates, our
old local patch is no longer necessary, but now we need a new one.
Change-Id: I8d226f01cf0951fd278605688684bf1ce3e17898
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44884
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Originally, when we imported fiat-crypto field operations, the pipeline
was in early stages and the generated code had to be manually integrated
with the rest of the curve implementation, so we moved all our
supporting code to third_party/fiat for simplicity. Over time more
supporting code, like the table generators, landed there to be next to
its callers.
fiat-crypto now generates standalone files which we #include into the
supporting code. This moves the supporting code back to the usual
location. It also updates the README.md file to reflect the new
pipeline. (Most of it was a documentation of the old pipeline, which was
much more manual.)
Change-Id: I64db7235feb6566f0d3cd4db3a7146050edaf25a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40904
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This pulls in the latest upstream files and reapplies our value_barrier patch.
In particular, https://github.com/mit-plv/fiat-crypto/pull/723 made 64-bit
P-256 faster!
32-bit x86, gcc
Before:
Did 3150 ECDH P-256 operations in 4027477us (782.1 ops/sec)
Did 9912 ECDSA P-256 signing operations in 4067212us (2437.1 ops/sec)
Did 3772 ECDSA P-256 verify operations in 4059197us (929.2 ops/sec)
Did 74800 Ed25519 key generation operations in 4020883us (18602.9 ops/sec)
Did 74000 Ed25519 signing operations in 4001827us (18491.6 ops/sec)
Did 21371 Ed25519 verify operations in 4024606us (5310.1 ops/sec)
Did 78000 Curve25519 base-point multiplication operations in 4051574us (19251.8 ops/sec)
Did 25133 Curve25519 arbitrary point multiplication operations in 4063280us (6185.4 ops/sec)
After:
Did 3250 ECDH P-256 operations in 4025179us (807.4 ops/sec) [+3.2%]
Did 10277 ECDSA P-256 signing operations in 4084926us (2515.8 ops/sec) [+3.2%]
Did 3895 ECDSA P-256 verify operations in 4048734us (962.0 ops/sec) [+3.5%]
Did 74480 Ed25519 key generation operations in 4002460us (18608.6 ops/sec) [+0.0%]
Did 74000 Ed25519 signing operations in 4004425us (18479.6 ops/sec) [-0.1%]
Did 21756 Ed25519 verify operations in 4038856us (5386.7 ops/sec) [+1.4%]
Did 78000 Curve25519 base-point multiplication operations in 4031991us (19345.3 ops/sec) [+0.5%]
Did 25133 Curve25519 arbitrary point multiplication operations in 4064925us (6182.9 ops/sec) [-0.0%]
x86_64, clang, OPENSSL_SMALL
Before:
Did 20090 ECDH P-256 operations in 4019408us (4998.2 ops/sec)
Did 56000 ECDSA P-256 signing operations in 4004370us (13984.7 ops/sec)
Did 23562 ECDSA P-256 verify operations in 4062283us (5800.2 ops/sec)
Did 127000 Ed25519 key generation operations in 4005053us (31709.9 ops/sec)
Did 128000 Ed25519 signing operations in 4021902us (31825.7 ops/sec)
Did 71000 Ed25519 verify operations in 4036015us (17591.6 ops/sec)
Did 132000 Curve25519 base-point multiplication operations in 4002101us (32982.7 ops/sec)
Did 93000 Curve25519 arbitrary point multiplication operations in 4023827us (23112.3 ops/sec)
After:
Did 22263 ECDH P-256 operations in 4005099us (5558.7 ops/sec) [+11.2%]
Did 61000 ECDSA P-256 signing operations in 4024810us (15156.0 ops/sec) [+8.4%]
Did 27426 ECDSA P-256 verify operations in 4038547us (6791.1 ops/sec) [+17.1%]
Did 128000 Ed25519 key generation operations in 4015033us (31880.2 ops/sec) [+0.5%]
Did 127000 Ed25519 signing operations in 4003894us (31719.1 ops/sec) [-0.3%]
Did 70000 Ed25519 verify operations in 4017446us (17424.0 ops/sec) [-1.0%]
Did 132000 Curve25519 base-point multiplication operations in 4006282us (32948.3 ops/sec) [-0.1%]
Did 93000 Curve25519 arbitrary point multiplication operations in 4025190us (23104.5 ops/sec) [-0.0%]
Change-Id: I2f705772899c701480ca0e0885e6b75dd1bb1f5d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40746
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This fixes two issues. First, we have been lax about whether the
low-level inversion functions fail on zero input or output zero. Fix the
documentation and call the latter inv0 or inverse0 to match the
terminology used in draft-irtf-cfrg-hash-to-curve. (Although we may not
actually need inv0 given the optimization in D.2.)
This has no actual effect because the functions were only used in
contexts where the inputs were already guaranteed to be non-zero. Still,
we should be consistent here.
Second, ec_scalar_inv_montgomery and ec_scalar_inv_montgomery_vartime
claim to perform the same operation, but they do not. First, one
computed inv0 and the other computed inv (except only in some
implementations, so fix it to be consistent). Second, the former
computes inverses in the Montgomery domain, while the latter converts to
the Montgomery domain and then inverts. Rename it to
ec_scalar_to_montgomery_inv_vartime, which is... questionably
understandable but at least looks different.
Change-Id: I9b4829ce5013bdb9528078a093f41b1b158df265
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40526
Reviewed-by: Adam Langley <agl@google.com>
In doing so, add an IgnoreAllInstructions option for FileTest. FileTest
tracks unused fields so test drivers don't accidentally miss a portion
of the test. Wycheproof tests, however, have many different key formats
in instructions. These are tedious to list out, so add an option to
ignore them, on the assumption that checking attributes is more useful
than instructions.
Change-Id: I01cc9f3a95577d576c8c2dd68f5092fceb3215b1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39191
Reviewed-by: Adam Langley <agl@google.com>
This CL updates the JSON conversion to preserve the flags. A
WycheproofResult now captures both "result" and "flags". An "acceptable"
test case's validity is determined by its flags. By default, we consider
an "acceptable" case as invalid, but a test driver may mark some of them
as valid by listing the flags as a parameter.
Previously, some Wycheproof tests (I think it was x25519_tests.txt?) did
not contain enough information to resolve this unambiguously. This has
since been fixed.
This also makes the converted files smaller because we no longer expand the
flags into comments.
Change-Id: I2ca02d7f1b95f250409e8b23c4ad7bb595d77fdf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39188
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Andres Erbsen