Age | Commit message (Collapse) | Author | Files | Lines |
|
Also remove unnecessary EC_GROUP_free calls. EC_GROUP_free is only
necessary in codepaths where arbitrary groups are possible.
Bug: 20
Change-Id: I3dfb7f07b890ab002ba8a302724d8bc671590cfe
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60932
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
|
|
EC_RAW_POINT is a confusing name. It's mostly about whether this is
stack-allocated EC_POINT without the EC_GROUP pointer. Now that we have
EC_AFFINE, EC_JACOBIAN captures what it's doing a bit better.
Fixed: 326
Change-Id: I5b71a387e899a94c79be8cd5e0b54b8432f7d5da
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59565
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
|
|
Caught by running malloc failure tests on unit tests.
Bug: 563
Change-Id: Ic0167ef346a282dc8b5a26a1cedafced7fef9ed0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56927
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
|
|
PMBTokens ends up converting the same point to affine coordinates
repeatedly. Additionally, it converts many affine coordinates at once,
which we can batch. Introduce an EC_AFFINE type to store affine points
and move the inversion to the Jacobian -> affine conversion.
This does mean we lose the (negligible) Montgomery reduction
optimization in EC_GFp_mont. point_get_affine_coordinates no longer
breaks the EC_FELEM abstraction around Montgomery form.
Unfortunately, this complicates hardening of the callers not checking
return values because EC_AFFINE cannot represent the point at infinity
and, due to OpenSSL's API limitations, groups may not have generators
available and the generator is not affine at the type level. (EC_AFFINE
cannot represent the point at infinity.) Thus this CL:
- Tidies up some duplicate code in setting up the generator and ensures
it always has Z = 1.
- ec_point_set_affine_coordinates hardens against unused results if the
generator is configured. But this is ultimately an internal function.
- Retains the hardening on the public APIs by adding calls to
ec_set_to_safe_point in two places.
This CL does not apply the optimization to Trust Tokens, only introduces
the EC_AFFINE abstraction. It additionally continues to store EC_POINTs
(used in ECDH and ECDSA) in Jacobian form. See
https://crbug.com/boringssl/326#c4 for a discussion on why this is
tricky. Those protocols are hopefully simple enough that they don't need
complexity around inversions.
Having an EC_AFFINE type will also be useful for computing custom tables
for Trust Token public keys, which gives a nice speedup.
Bug: 326
Change-Id: I11b010a33f36a15bac9939351df5205bd35cc665
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41084
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@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>
|
|
While it appears that we internally support constant-time
dual multiplication, it is not actually constant-time. Integrating the
two operations means we hit the doubling branch. Instead, replace the
constant-time functions with single multiplication functions, one for
arbitrary points and one for the base point. This simplifies timing
analysis of the EC_METHODs.
This CL only changes the wrapper functions. A subsequent CL will change
the EC_METHOD hooks. We conservatively assume EC_POINT_mul callers
expect secret scalars and split it into two multiplications and an
addition if needed.
Update-Note: EC_POINT_mul may get slower if called with both g_scalar
and p_scalar non-NULL. If the scalars were secret, this plugs a timing
leak (note neither ECDH nor ECDSA signing use such an operation). If
acting on public scalars, notably ECDSA verify, this slowdown is not
inherently necessary. If necessary, we can expose a public version of
ec_point_mul_scalar_public, but callers should be using BoringSSL's
ECDSA verify API instead.
Change-Id: I9c20b660ce4b58dc633588cfd5b2e97a40203ec3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/36224
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
|
|
With the allocations and BN_CTX gone, ECDH and point2oct are much, much
shorter.
Bug: 242
Change-Id: I3421822e94100f7eb2f5f2373df7fb3b3311365e
Reviewed-on: https://boringssl-review.googlesource.com/c/33071
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
|
|
Now the only allocations in ECDSA are the ECDSA_SIG input and output.
Change-Id: If1fcde6dc2ee2c53f5adc16a7f692e22e9c238de
Reviewed-on: https://boringssl-review.googlesource.com/c/33069
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
|
|
Change-Id: Ie7ff03a2c5b2ae8f56816b02182df40ce7ca0065
Reviewed-on: https://boringssl-review.googlesource.com/c/33066
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
|
|
The fipsmodule is still separate as that's a lot of build mess. (Though
that too may be worth pulling in eventually. CMake usually has different
opinions on generated files if they're in the same directory. We might
be able to avoid the set_source_properties(GENERATED) thing.)
Change-Id: Ie1f9345009044d4f0e7541ca779e01bdc5ad62f6
Reviewed-on: https://boringssl-review.googlesource.com/31586
Reviewed-by: Adam Langley <agl@google.com>
|
|
Also remove some transition step for a recent format change. Together, this
removes the curve hacks in the converter, which can now be purely syntactic.
The RSA ones are still a bit all over the place in terms of sharded vs
combined, so leaving that alone for now.
Change-Id: I721d6b0de388a53a39543725e366dc5b52e83561
Reviewed-on: https://boringssl-review.googlesource.com/30845
Reviewed-by: Adam Langley <agl@google.com>
|
|
This change adds a function so that an ECDH and the hashing of the
resulting 'x' coordinate can occur inside the FIPS boundary.
Change-Id: If93c20a70dc9dcbca49056f10915d3ce064f641f
Reviewed-on: https://boringssl-review.googlesource.com/30104
Reviewed-by: Adam Langley <agl@google.com>
|