The current X25519 assembly has a 352-byte stack frame and saves the
regsiters at the bottom. This means that the CFI information cannot be
represented in the “compact” form that MacOS seems to want to use (see
linked bug).
The stack frame looked like:
360 CFA
352 return address
⋮
56 (296 bytes of scratch space)
48 saved RBP
40 saved RBX
32 saved R15
24 saved R14
16 saved R13
8 saved R12
0 (hole left from 3f38d80b dropping the superfluous saving of R11)
Now it looks like:
352 CFA
344 return address
336 saved RBP
328 saved RBX
320 saved R15
312 saved R14
304 saved R13
296 saved R12
⋮
0 (296 bytes of scratch space)
The bulk of the changes involve subtracting 56 from all the offsets to
RSP when working in the scratch space. This was done in Vim with:
'<,'>s/\([1-9][0-9]*\)(%rsp)/\=submatch(1)-56."(%rsp)"/
BUG=176
Change-Id: I022830e8f896fe2d877015fa3ecfa1d073207679
Reviewed-on: https://boringssl-review.googlesource.com/13580
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The Mac ld gets unhappy about "weird" unwind directives:
In chacha20_poly1305_x86_64.pl, $keyp is being pushed on the stack
(according to the comment) because it gets clobbered in the computation
somewhere. $keyp is %r9 which is not callee-saved (it's an argument
register), so we don't need to tag it with .cfi_offset.
In x25519-asm-x86_64.S, x25519_x86_64_mul saves %rdi on the stack.
However it too is not callee-saved (it's an argument register) and
should not have a .cfi_offset. %rdi also does not appear to be written
to anywhere in the function, so there's no need to save it at all.
(This does not resolve the "r15 is saved too far from return address"
errors. Just the non-standard register ones.)
BUG=176
Change-Id: I53f3f7db3d1745384fb47cb52cd6536aabb5065e
Reviewed-on: https://boringssl-review.googlesource.com/13560
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This change serves to check that all our consumers can process assembly
with CFI directives in it.
For the first change I picked a file that's not perlasm to keep things
slightly simplier, but that might have been a mistake:
DJB's tooling always aligns the stack to 32 bytes and it's not possible
to express this in DWARF format (without using a register to store the
old stack pointer).
Since none of the functions here appear to care about that alignment, I
removed it from each of them. I also trimmed the set of saved registers
where possible and used the redzone for functions that didn't need much
stack.
Overall, this appears to have slightly improved the performance (by
about 0.7%):
Before:
Did 46000 Curve25519 base-point multiplication operations in 3023288us (15215.2 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3017315us (15245.3 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3015346us (15255.3 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3018609us (15238.8 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3019004us (15236.8 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3013135us (15266.5 ops/sec)
After:
Did 46000 Curve25519 base-point multiplication operations in 3007659us (15294.3 ops/sec)
Did 47000 Curve25519 arbitrary point multiplication operations in 3054202us (15388.6 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3008714us (15288.9 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3004740us (15309.1 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3009140us (15286.8 ops/sec)
Did 47000 Curve25519 arbitrary point multiplication operations in 3057518us (15371.9 ops/sec)
Change-Id: I31df11c45b2ea0bf44dde861d52c27f848331691
Reviewed-on: https://boringssl-review.googlesource.com/13200
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html
Add OPENSSL_memcpy, etc., wrappers which avoid this problem.
BUG=23
Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This function allows callers to unpack an Ed25519 “seed” value, which is
a 32 byte value that contains sufficient information to build a public
and private key from.
Change-Id: Ie5d8212a73e5710306314b4f8a93b707665870fd
Reviewed-on: https://boringssl-review.googlesource.com/12040
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I87cbc12aeb399646c6426b7a099dbf13aafc2532
Reviewed-on: https://boringssl-review.googlesource.com/11983
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Unlike the Scoped* types, bssl::UniquePtr is available to C++ users, and
offered for a large variety of types. The 'extern "C++"' trick is used
to make the C++ bits digestible to C callers that wrap header files in
'extern "C"'.
Change-Id: Ifbca4c2997d6628e33028c7d7620c72aff0f862e
Reviewed-on: https://boringssl-review.googlesource.com/10521
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This reverts commits:
8d79ed674019fdcb52348d79ed6740
Because WebRTC (at least) includes our headers in an extern "C" block,
which precludes having any C++ in them.
Change-Id: Ia849f43795a40034cbd45b22ea680b51aab28b2d
This change scatters the contents of the two scoped_types.h files into
the headers for each of the areas of the code. The types are now in the
|bssl| namespace.
Change-Id: I802b8de68fba4786b6a0ac1bacd11d81d5842423
Reviewed-on: https://boringssl-review.googlesource.com/8731
Reviewed-by: Adam Langley <agl@google.com>
We currently have the situation where the |tool| and |bssl_shim| code
includes scoped_types.h from crypto/test and ssl/test. That's weird and
shouldn't happen. Also, our C++ consumers might quite like to have
access to the scoped types.
Thus this change moves some of the template code to base.h and puts it
all in a |bssl| namespace to prepare for scattering these types into
their respective headers. In order that all the existing test code be
able to access these types, it's all moved into the same namespace.
Change-Id: I3207e29474dc5fcc344ace43119df26dae04eabb
Reviewed-on: https://boringssl-review.googlesource.com/8730
Reviewed-by: David Benjamin <davidben@google.com>
In order to ensure that we don't randomly interoperate with
implementations that don't mask scalars correctly, always generate
scalars with the wrong fixed bits.
Change-Id: I82536a856f034cfe4464fc545a99c21b3cff1691
Reviewed-on: https://boringssl-review.googlesource.com/8391
Reviewed-by: David Benjamin <davidben@google.com>
x25519-x86_64.c, like the rest of crypto/curve25519, is descended from
SUPERCOP. Add the usual copyright header along with the SUPERCOP attribution.
BUG=64
Change-Id: I43f3de0731f33ab2aa48492c4b742e9f23c87fe1
Reviewed-on: https://boringssl-review.googlesource.com/8195
Reviewed-by: Adam Langley <agl@google.com>
Although exactly one iteration of cmov_cached will always initialize selected,
it ends up messing with uninitialized memory. Initialize |selected| before the
loop.
BUG=593540
Change-Id: I5921843f68c6dd1dc7f752538825bc43ba75df4a
Reviewed-on: https://boringssl-review.googlesource.com/7415
Reviewed-by: Arnar Birgisson <arnarb@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
C is still kind of unsure about the whole two's complement thing and leaves
left-shifting of negative numbers undefined. Sadly, some sanitizers believe in
teaching the controversy and complain when code relies on the theory of two's
complement.
Shushing these sanitizers in this case is easier than fighting with build
configuration, so replace the shifts with masks. (This is equivalent as the
left-shift was of a value right-shifted by the same amount. Instead, we store
the unshifted value in carry0, etc., and mask off the bottom bits.) A few other
places get casts to unsigned types which, by some miracle, C compilers are
forbidden from miscompiling.
This is imported from upstream's b95779846dc876cf959ccf96c49d4c0a48ea3082 and
5b7af0dd6c9315ca76fba16813b66f5792c7fe6e.
Change-Id: I6bf8156ba692165940c0c4ea1edd5b3e88ca263e
Reviewed-on: https://boringssl-review.googlesource.com/7320
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's dc22d6b37e8058a4334e6f98932c2623cd3d8d0d. (Though I'm not
sure why they didn't need to fix cmov.)
Change-Id: I2a194a8aea1734d4c1e7f6a0536a636379381627
Reviewed-on: https://boringssl-review.googlesource.com/7280
Reviewed-by: Adam Langley <agl@google.com>
The function |ge_frombytes_negate_vartime|, as the name suggests,
negates its output. This change converts it to |ge_frombytes_vartime|
and, instead, does the negation explicitly when verifying signatures.
The latter function is more generally useful.
Change-Id: I465f8bdf5edb101a80ab1835909ae0ff41d3e295
Reviewed-on: https://boringssl-review.googlesource.com/7142
Reviewed-by: Arnar Birgisson <arnarb@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Add guards for the architecture and OPENSSL_NO_ASM to
the assembly-language files in crypto/curve25519/asm.
The Dart compilation of BoringSSL includes all files,
because the architecture is not known when gyp is run.
Change-Id: I66f5ae525266b63b0fe3a929012b771d545779b5
Reviewed-on: https://boringssl-review.googlesource.com/7030
Reviewed-by: Adam Langley <agl@google.com>
These symbols can show up in lists of large symbols but, so I
understand, these lists might not include the filename path. Thus |base|
as a symbol name is rather unhelpful.
This change renames the two precomputated tables to have slightly more
greppable names.
Change-Id: I77059250cfce4fa9eceb64e260b45db552b63255
Reviewed-on: https://boringssl-review.googlesource.com/6813
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
Some build systems don't like two targets with the same base name and
the curve25519 code had x25519-x86_64.[Sc].
Change-Id: If8382eb84996d7e75b34b28def57829d93019cff
Reviewed-on: https://boringssl-review.googlesource.com/6878
Reviewed-by: Adam Langley <agl@google.com>
This assembly is in gas syntax so is not built on Windows nor when
OPENSSL_SMALL is defined.
Change-Id: I1050cf1b16350fd4b758e4c463261b30a1b65390
Reviewed-on: https://boringssl-review.googlesource.com/6782
Reviewed-by: Adam Langley <agl@google.com>
OPENSSL_SMALL will still cause the smaller base-point table to be used
and so won't be as fast at signing as the full version, but Ed25519 will
now work in those builds.
Without OPENSSL_SMALL:
Did 20000 Ed25519 key generation operations in 1008347us (19834.4 ops/sec)
Did 20000 Ed25519 signing operations in 1025594us (19500.9 ops/sec)
Did 6138 Ed25519 verify operations in 1001712us (6127.5 ops/sec)
Did 21000 Curve25519 base-point multiplication operations in 1019237us (20603.6 ops/sec)
Did 7095 Curve25519 arbitrary point multiplication operations in 1065986us (6655.8 ops/sec)
With (on the same machine):
Did 8415 Ed25519 key generation operations in 1020958us (8242.3 ops/sec)
Did 8952 Ed25519 signing operations in 1077635us (8307.1 ops/sec)
Did 6358 Ed25519 verify operations in 1047533us (6069.5 ops/sec)
Did 6620 Curve25519 base-point multiplication operations in 1008922us (6561.5 ops/sec)
Did 7183 Curve25519 arbitrary point multiplication operations in 1096285us (6552.1 ops/sec)
Change-Id: Ib443c0e2bdfd11e044087e66efd55b651a5667e7
Reviewed-on: https://boringssl-review.googlesource.com/6772
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This ensures the run_tests target updates those binaries.
Change-Id: I32b68026da4852424b5621e014e71037c8a5754c
Reviewed-on: https://boringssl-review.googlesource.com/6513
Reviewed-by: Adam Langley <agl@google.com>
Nexus 7 goes from 1002.8 ops/sec to 4704.8 at a cost of 10KB of code.
(It'll actually save code if built with -mfpu=neon because then the
generic version can be discarded by the compiler.)
Change-Id: Ia6d02efb2c2d1bb02a07eb56ec4ca3b0dba99382
Reviewed-on: https://boringssl-review.googlesource.com/6524
Reviewed-by: Adam Langley <agl@google.com>
MSVC doesn't like unary minus on unsigned types. Also, the speed test
always failed because the inputs were all zeros and thus had small
order.
Change-Id: Ic2d3c2c9bd57dc66295d93891396871cebac1e0b
