Commit Graph

8 Commits

Author SHA1 Message Date
David Benjamin
5bcaa113e2 Tighten EC_KEY's association with its group.
This is to simplify
https://boringssl-review.googlesource.com/c/boringssl/+/24445/.

Setting or changing an EC_KEY's group after the public or private keys
have been configured is quite awkward w.r.t. consistency checks. It
becomes additionally messy if we mean to store private keys as
EC_SCALARs (and avoid the BIGNUM timing leak), whose size is
curve-dependent.

Instead, require that callers configure the group before setting either
half of the keypair. Additionally, reject EC_KEY_set_group calls that
change the group. This will simplify clearing one more BIGNUM timing
leak.

Update-Note: This will break code which sets the group and key in a
    weird order. I checked calls of EC_KEY_new and confirmed they all
    set the group first. If I missed any, let me know.

Change-Id: Ie89f90a318b31b6b98f71138e5ff3de5323bc9a6
Reviewed-on: https://boringssl-review.googlesource.com/24425
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 22:15:11 +00:00
David Benjamin
d8dbde79f9 Don't allow negative EC_KEY private keys.
We check that the private key is less than the order, but we forgot the
other end.

Update-Note: It's possible some caller was relying on this, but since
    that function already checked the other half of the range, I'm
    expecting this to be a no-op change.

Change-Id: I4a53357d7737735b3cfbe97d379c8ca4eca5d5ac
Reviewed-on: https://boringssl-review.googlesource.com/23665
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>
2017-12-05 19:46:27 +00:00
David Benjamin
48eaa28a12 Make EC_POINT_mul work with arbitrary BIGNUMs again.
Rejecting values where we'd previous called BN_nnmod may have been
overly ambitious. In the long run, all the supported ECC APIs (ECDSA*,
ECDH_compute_key, and probably some additional new ECDH API) will be
using the EC_SCALAR version anyway, so this doesn't really matter.

Change-Id: I79cd4015f2d6daf213e4413caa2a497608976f93
Reviewed-on: https://boringssl-review.googlesource.com/23584
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-11-30 21:58:17 +00:00
David Benjamin
0a5f006736 Test that EC_POINT_mul works with the order.
|EC_POINT_mul| is almost exclusively used with reduced scalars, with
this exception. This comes from consumers following NIST SP 800-56A
section 5.6.2.3.2. (Though all our curves have cofactor one, so this
check isn't useful.)

Add a test for this so we don't accidentally break it.

Change-Id: I42492db38a1ea03acec4febdd7945c8a3933530a
Reviewed-on: https://boringssl-review.googlesource.com/23084
Reviewed-by: Adam Langley <agl@google.com>
2017-11-20 18:32:30 +00:00
David Benjamin
cb16f17b36 Check EC_POINT/EC_GROUP compatibility more accurately.
Currently we only check that the underlying EC_METHODs match, which
avoids the points being in different forms, but not that the points are
on the same curves. (We fixed the APIs early on so off-curve EC_POINTs
cannot be created.)

In particular, this comes up with folks implementating Java's crypto
APIs with ECDH_compute_key. These APIs are both unfortunate and should
not be mimicked, as they allow folks to mismatch the groups on the two
multiple EC_POINTs. Instead, ECDH APIs should take the public value as a
byte string.

Thanks also to Java's poor crypto APIs, we must support custom curves,
which makes this particularly gnarly. This CL makes EC_GROUP_cmp work
with custom curves and adds an additional subtle requirement to
EC_GROUP_set_generator.

Annoyingly, this change is additionally subtle because we now have a
reference cycle to hack around.

Change-Id: I2efbc4bd5cb65fee5f66527bd6ccad6b9d5120b9
Reviewed-on: https://boringssl-review.googlesource.com/22245
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>
2017-10-28 08:02:50 +00:00
David Benjamin
0b80f7f287 Convert example_mul to GTest.
This is the last of the non-GTest tests. We never did end up writing
example files or doc.go tooling for them. And probably examples should
be in C++ at this point.

Bug: 129
Change-Id: Icbc43c9639cfed7423df20df1cdcb8c35f23fc1a
Reviewed-on: https://boringssl-review.googlesource.com/17669
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>
2017-07-10 19:28:29 +00:00
Steven Valdez
467d3220f8 Add FIPS-compliant key generation that calls check_fips for RSA and EC.
Change-Id: Ie466b7b55bdd679c5baf2127bd8de4a5058fc3b7
Reviewed-on: https://boringssl-review.googlesource.com/16346
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-05-17 16:30:48 +00:00
Adam Langley
aacb72c1b7 Move ec/ and ecdsa/ into fipsmodule/
The names in the P-224 code collided with the P-256 code and thus many
of the functions and constants in the P-224 code have been prefixed.

Change-Id: I6bcd304640c539d0483d129d5eaf1702894929a8
Reviewed-on: https://boringssl-review.googlesource.com/15847
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>
2017-05-04 20:27:23 +00:00