Commit Graph

19 Commits

Author SHA1 Message Date
David Benjamin
2f6410ba4e Rewrite ECPrivateKey serialization.
Functions which lose object reuse and need auditing:
- d2i_ECParameters
- d2i_ECPrivateKey

This adds a handful of bytestring-based APIs to handle EC key
serialization. Deprecate all the old serialization APIs. Notes:

- An EC_KEY has additional state that controls its encoding, enc_flags
  and conv_form. conv_form is left alone, but enc_flags in the new API
  is an explicit parameter.

- d2i_ECPrivateKey interpreted its T** argument unlike nearly every
  other d2i function. This is an explicit EC_GROUP parameter in the new
  function.

- The new specified curve code is much stricter and should parse enough
  to uniquely identify the curve.

- I've not bothered with a new version of i2d_ECParameters. It just
  writes an OID. This may change later when decoupling from the giant
  OID table.

- Likewise, I've not bothered with new APIs for the public key since the
  EC_POINT APIs should suffice.

- Previously, d2i_ECPrivateKey would not call EC_KEY_check_key and it
  was possible for the imported public and private key to mismatch. It
  now calls it.

BUG=499653

Change-Id: I30b4dd2841ae76c56ab0e1808360b2628dee0615
Reviewed-on: https://boringssl-review.googlesource.com/6859
Reviewed-by: Adam Langley <agl@google.com>
2016-02-16 23:51:09 +00:00
Brian Smith
a0ef7b0a56 Enforce that |EC_KEY| private key is in [0, group->order).
Change-Id: I16abea5769737c7edd1be717f9a4f38678af43ce
Reviewed-on: https://boringssl-review.googlesource.com/6564
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 18:45:03 +00:00
Adam Langley
96c2a28171 Fix all sign/unsigned warnings with Clang and GCC.
Change-Id: If2a83698236f7b0dcd46701ccd257a85463d6ce5
Reviewed-on: https://boringssl-review.googlesource.com/4992
Reviewed-by: Adam Langley <agl@google.com>
2015-10-27 22:48:00 +00:00
David Benjamin
15e4deb165 d2i: don't update input pointer on failure
(Imported from upstream's 728bcd59d3d41e152aead0d15acc51a8958536d3.)

Actually this one was reported by us, but the commit message doesn't
mention this.

This is slightly modified from upstream's version to fix some problems
noticed in import. Specifically one of d2i_X509_AUX's success paths is
bust and d2i_PrivateKey still updates on one error path. Resolve the
latter by changing both it and d2i_AutoPrivateKey to explicitly hit the
error path on ret == NULL. This lets us remove the NULL check in
d2i_AutoPrivateKey.

We'll want to report the problems back upstream.

Change-Id: Ifcfc965ca6d5ec0a08ac154854bd351cafbaba25
Reviewed-on: https://boringssl-review.googlesource.com/5948
Reviewed-by: Adam Langley <agl@google.com>
2015-09-28 22:15:17 +00:00
David Benjamin
3570d73bf1 Remove the func parameter to OPENSSL_PUT_ERROR.
Much of this was done automatically with
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/'
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+,  ([a-zA-Z_0-9]+\);)/\1\2/'

BUG=468039

Change-Id: I4c75fd95dff85ab1d4a546b05e6aed1aeeb499d8
Reviewed-on: https://boringssl-review.googlesource.com/5276
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:37 +00:00
David Benjamin
cfaf7ff9bf Remove unnecessary NULL checks, part 2.
Stuff in crypto/ec.

Change-Id: I3bd238c365c4766ed8abc6f835a107478b43b159
Reviewed-on: https://boringssl-review.googlesource.com/4515
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:08:22 +00:00
David Benjamin
7743c026cb Ensure EC private keys retain leading zeros
RFC 5915 requires the use of the I2OSP primitive as defined in RFC 3447
for encoding ECPrivateKey. Fix this and add a test.

See also upstream's 30cd4ff294252c4b6a4b69cbef6a5b4117705d22, though it mixes
up degree and order.

Change-Id: I81ba14da3c8d69e3799422c669fab7f16956f322
Reviewed-on: https://boringssl-review.googlesource.com/4469
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 21:03:27 +00:00
David Benjamin
4b1510c71e Fix a failure to NULL a pointer freed on error.
Reported by the LibreSSL project as a follow on to CVE-2015-0209

(Imported from upstream's 5e5d53d341fd9a9b9cc0a58eb3690832ca7a511f.)

Change-Id: Ic2e5dc5c96e316c55f76bedc6ea55b416be3287a
Reviewed-on: https://boringssl-review.googlesource.com/4049
Reviewed-by: Adam Langley <agl@google.com>
2015-03-19 19:50:32 +00:00
David Benjamin
5ae21bc02b Remove pointless free, and use preferred way of calling d2i_* functions
(Imported from upstream's 535bc8faf69dc4ff39e2ee99195b268cf99b9569)

Change-Id: Ia5abf4dce9dc8cdf5a9b77a3e360a40b5f815adf
Reviewed-on: https://boringssl-review.googlesource.com/3672
Reviewed-by: Adam Langley <agl@google.com>
2015-02-27 19:47:36 +00:00
David Benjamin
29b50eab6c Align d2i_ECPrivateKey error-handling with upstream.
Upstream decided to make the caller free the scratch space rather than the
callee. May as well match. (Existing code is pretty inconsistent. This API
pattern needs to go.)

See upstream's 9e442d485008046933cdc7da65080f436a4af089.

Change-Id: I7c9fcae5778a74d6ae8e9f546e03fb2cf6e48426
Reviewed-on: https://boringssl-review.googlesource.com/3671
Reviewed-by: Adam Langley <agl@google.com>
2015-02-27 19:45:42 +00:00
Adam Langley
2b2d66d409 Remove string.h from base.h.
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.

This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.

Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-02 19:14:15 +00:00
Eric Roman
517073cd4b Set output EC_KEY to NULL when d2i_ECPrivateKey() fails.
BUG=crbug.com/445679

Change-Id: Ia012d806964bd7240148779797eccd326484f364
Reviewed-on: https://boringssl-review.googlesource.com/2722
Reviewed-by: Adam Langley <agl@google.com>
2015-01-06 00:55:19 +00:00
Adam Langley
9398f168f9 Explicitly check for empty ASN.1 strings in d2i_ECPrivateKey.
The old code implicitly relies on the ASN.1 code returning a \0-prefixed
buffer when the buffer length is 0. Change this to verify explicitly
that the ASN.1 string has positive length.

(Imported from upstream's 7f7c05ca638c3cc6d261961fae439cd91e3c1d27)

Change-Id: Icc6c44b874bdcb02374016a36d209830d6162a8a
2014-11-10 13:45:32 -08:00
Adam Langley
9f5a314d35 Automatically generate a missing EC public key.
When d2i_ECPrivateKey reads a private key with a missing (optional)
public key, generate one automatically from the group and private key.

(Imported from upstream's 2083f7c465d07867dd9867b8742bb71c03d1f203)

Change-Id: I9e5090de87cf846ab92e4be5b6bf64e6091d02e4
2014-11-10 13:45:31 -08:00
David Benjamin
775b75f9de Fix Windows shared library build.
Windows is much pickier about dllimport/dllexport. Declare it on
the declaration, not the definition. Also ensure that the declaration
precedes the definition. Finally, remove a stray OPENSSL_EXPORT.

Change-Id: Id50b9de5acbe5adf1b15b22dd60b7a5c13a80cce
Reviewed-on: https://boringssl-review.googlesource.com/1862
Reviewed-by: Adam Langley <agl@google.com>
2014-10-01 02:01:53 +00:00
Adam Langley
c0d9484902 ec: recognise known parameters when written in full.
Some EC ASN.1 structures are using a named curve, but include the full
parameters anyway. With this change, BoringSSL will recognise the order
of the curve.

Change-Id: Iff057178453f9fdc98c8c03bcabbccef89709887
Reviewed-on: https://boringssl-review.googlesource.com/1270
Reviewed-by: Adam Langley <agl@google.com>
2014-07-19 00:03:13 +00:00
Adam Langley
cdf96e5886 Double free in i2o_ECPublicKey
PR: 3338

(Imported from upstream's e34af3ec2b1230a8a523d383f7de505f7cbd381d)
2014-06-20 13:17:40 -07:00
Adam Langley
f71a27920a Fix EC crash.
This change saves several EC routines from crashing when an EC_KEY is
missing a public key. The public key is optional in the EC private key
format and, without this patch, running the following through `openssl
ec` causes a crash:

-----BEGIN EC PRIVATE KEY-----
MBkCAQEECAECAwQFBgcIoAoGCCqGSM49AwEH
-----END EC PRIVATE KEY-----
2014-06-20 13:17:34 -07:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00