Commit Graph

120 Commits

Author SHA1 Message Date
David Benjamin
2cdf398773 Remove pkey_base_id.
This is never accessed.

Change-Id: I4cade5e907ad4c03e9de7634b53ef965f7240087
Reviewed-on: https://boringssl-review.googlesource.com/6864
Reviewed-by: Adam Langley <agl@google.com>
2016-01-28 15:55:24 +00:00
David Benjamin
4f6acaf0da Use more C++11 features.
Finally, we can stick ScopedFOO in containers.

Change-Id: I3ed166575822af9f182e8be8f4db723e1f08ea31
Reviewed-on: https://boringssl-review.googlesource.com/6553
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:52:37 +00:00
David Benjamin
0a2c9938a5 Don't allow the specifiedCurve form of ECParameters in SPKIs.
Although RFC 3279 allows both, per RFC 5912, keys must use a named curve
rather than spelling out the curve parameters. Although we do not allow
arbitrary curves, we do have to (pretty hackishly) recognize built-in
curves in ECPrivateKeys.

It seems the cause of this was that OpenSSL, unless you set asn1_flag on
the EC_GROUP, likes to encode keys by spelling out the parameters. This
is in violation of RFC 5915, though probably not in violation of one of
the other redundant ECC specifications. For more fun, it appears
asn1_flag defaults to *off* in the API and *on* in the command-line
tools.

I think the original cause was these defaults meant the pre-BoringSSL
Android/OpenSSL Chromium port wrote out Channel ID keys in this format.
By now this should no longer by an issue, but it'll warrant a bit more
investigation to be sure we can drop it.

For now, keep this logic out of SPKIs by not calling d2i_ECParameters.
d2i_ECParameters is a fairly pointless function when only named curves
are allowed. In testing other implementations, none of Firefox, Safari,
or IE11/Win will parse such certificates (i.e. the error is fatal and
unbypassable). Likewise, because Mac and Windows' underlying libraries
reject this, Chrome on Mac and Windows already rejects such things. Thus
this change should be compatible.

The following is the certificate and key I constructed to test with:

-----BEGIN CERTIFICATE-----
MIICwjCCAmqgAwIBAgIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC
QVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdp
dHMgUHR5IEx0ZDAeFw0xNDA0MjMyMzIxNTdaFw0xNDA1MjMyMzIxNTdaMEUxCzAJ
BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l
dCBXaWRnaXRzIFB0eSBMdGQwggFLMIIBAwYHKoZIzj0CATCB9wIBATAsBgcqhkjO
PQEBAiEA/////wAAAAEAAAAAAAAAAAAAAAD///////////////8wWwQg/////wAA
AAEAAAAAAAAAAAAAAAD///////////////wEIFrGNdiqOpPns+u9VXaYhrxlHQaw
zFOw9jvOPD4n0mBLAxUAxJ02CIbnBJNqZnjhE50mt4GffpAEQQRrF9Hy4SxCR/i8
5uVjpEDydwN9gS3rM6D0oTlF2JjClk/jQuL+Gn+bjufrSnwPnhYrzjNXazFezsu2
QGg3v1H1AiEA/////wAAAAD//////////7zm+q2nF56E87nKwvxjJVECAQEDQgAE
5itp4r9ln5e+Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyD
SNsWGhz1HX7xlC1Lz3IiwaNQME4wHQYDVR0OBBYEFKuE0qyrlfCCThZ4B1VXX+Qm
jYLRMB8GA1UdIwQYMBaAFKuE0qyrlfCCThZ4B1VXX+QmjYLRMAwGA1UdEwQFMAMB
Af8wCQYHKoZIzj0EAQNHADBEAiBATB6aVJxDD6YAxEM4vf6Sbg2Ty334ldXpkNwc
TF+SngIgZ/f59kgDLf6YA04iLw1fUv5Wf1nLYJWwgrRFON5+zvw=
-----END CERTIFICATE-----
-----BEGIN EC PARAMETERS-----
MIH3AgEBMCwGByqGSM49AQECIQD/////AAAAAQAAAAAAAAAAAAAAAP//////////
/////zBbBCD/////AAAAAQAAAAAAAAAAAAAAAP///////////////AQgWsY12Ko6
k+ez671VdpiGvGUdBrDMU7D2O848PifSYEsDFQDEnTYIhucEk2pmeOETnSa3gZ9+
kARBBGsX0fLhLEJH+Lzm5WOkQPJ3A32BLeszoPShOUXYmMKWT+NC4v4af5uO5+tK
fA+eFivOM1drMV7Oy7ZAaDe/UfUCIQD/////AAAAAP//////////vOb6racXnoTz
ucrC/GMlUQIBAQ==
-----END EC PARAMETERS-----
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIAcPCHJ61KBKnN1ZyU2JaHcItW/JXTB3DujRyc4Ki7RqoAoGCCqGSM49
AwEHoUQDQgAE5itp4r9ln5e+Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY
+cPEox5W4nyDSNsWGhz1HX7xlC1Lz3IiwQ==
-----END EC PRIVATE KEY-----

BUG=522228

Change-Id: I3723411a633dc07c4640027de07500293f8f7913
Reviewed-on: https://boringssl-review.googlesource.com/6853
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:51:14 +00:00
David Benjamin
f6094e05ef Don't allow EVP_PKEY_RSA2.
OpenSSL accepts both OID 2.5.8.1.1 and OID 1.2.840.113549.1.1.1 for RSA
public keys. The latter comes from RFC 3279 and is widely implemented.
The former comes from the ITU-T version of X.509. Interestingly,
2.5.8.1.1 actually has a parameter, which OpenSSL ignores:

  rsa ALGORITHM ::= {
     KeySize
     IDENTIFIED BY id-ea-rsa
  }
  KeySize ::= INTEGER

Remove support for 2.5.8.1.1 completely. In tests with a self-signed
certificate and code inspection:

- IE11 on Win8 does not accept the certificate in a TLS handshake at
  all. Such a certificate is fatal and unbypassable. However Microsoft's
  libraries do seem to parse it, so Chrome on Windows allows one to
  click through the error. I'm guessing either the X.509 stack accepts
  it while the TLS stack doesn't recognize it as RSA or the X.509 stack
  is able to lightly parse it but not actually understand the key. (The
  system certificate UI didn't display it as an RSA key, so probably the
  latter?)

- Apple's certificate library on 10.11.2 does not parse the certificate
  at all. Both Safari and Chrome on Mac treat it as a fatal and
  unbypassable error.

- mozilla::pkix, from code inspection, does not accept such
  certificates. However, Firefox does allow clicking through the error.
  This is likely a consequence of mozilla::pkix and NSS having different
  ASN.1 stacks. I did not test this, but I expect this means Chrome on
  Linux also accepts it.

Given IE and Safari's results, it should be safe to simply remove this.
Firefox's data point is weak (perhaps someone is relying on being able
to click-through a self-signed 2.5.8.1.1 certificate), but it does
further ensure no valid certificate could be doing this.

The following is the 2.5.8.1.1 certificate I constructed to test with.
The private key is key.pem from ssl/test/runner:

-----BEGIN CERTIFICATE-----
MIICVTCCAb6gAwIBAgIJAPuwTC6rEJsMMA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV
BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX
aWRnaXRzIFB0eSBMdGQwHhcNMTQwNDIzMjA1MDQwWhcNMTcwNDIyMjA1MDQwWjBF
MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50
ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGcMAoGBFUIAQECAgQAA4GNADCBiQKBgQDY
K8imMuRi/03z0K1Zi0WnvfFHvwlYeyK9Na6XJYaUoIDAtB92kWdGMdAQhLciHnAj
kXLI6W15OoV3gA/ElRZ1xUpxTMhjP6PyY5wqT5r6y8FxbiiFKKAnHmUcrgfVW28t
Q+0rkLGMryRtrukXOgXBv7gcrmU7G1jC2a7WqmeI8QIDAQABo1AwTjAdBgNVHQ4E
FgQUi3XVrMsIvg4fZbf6Vr5sp3Xaha8wHwYDVR0jBBgwFoAUi3XVrMsIvg4fZbf6
Vr5sp3Xaha8wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQUFAAOBgQAIZuUICtYv
w3cbpCGX6HNCtyI0guOfbytcdwzRkQaCsYNSDrTxrSSWxHwqg3Dl/RlvS+T3Yaua
Xkioadstwt7GDP6MwpIpdbjchh0XZd3kjdJWqXSvihUDpRePNjNS2LmJW8GWfB3c
F6UVyNK+wcApRY+goREIhyYupAHUexR7FQ==
-----END CERTIFICATE-----

BUG=522228

Change-Id: I031d03c0f53a16cbc749c4a5d8be6efca50dc863
Reviewed-on: https://boringssl-review.googlesource.com/6852
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:43:37 +00:00
David Benjamin
c612e61e1d Fix minor stylistic problem.
Normally this would be pretty scary:
  if (...) {
  } if (...) {
  }
But it's an early return anyway.

Change-Id: I0a8965b5e294d3aaa803be47f4006ea0311c431d
Reviewed-on: https://boringssl-review.googlesource.com/6851
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:35:35 +00:00
David Benjamin
719594e512 Un-const EVP_PKEY_CTX_set0_rsa_oaep_label and fix overflow check.
It takes ownership of the buffer, so it's not actually const. The
const-ness gets dropped once it transits through EVP_PKEY_CTX_ctrl.

Also compare against INT_MAX explicitly for the overflow check. I'm not sure
whether the casting version is undefined, but comparing against INT_MAX matches
the rest of the codebase when transiting in and out of signed ints.

Change-Id: I131165a4b5f0ebe02c6db3e7e3e0d1af5b771710
Reviewed-on: https://boringssl-review.googlesource.com/6850
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:34:38 +00:00
David Benjamin
b6155e60f3 Remove app_data from EVP_PKEY_CTX.
It's never used. It's not clear why one would want such a thing.
EVP_PKEY_CTX has no way for callers to register callbacks, which means
there shouldn't be a way for the library to present you an EVP_PKEY_CTX
out-of-context. (Whereas app_data/ex_data makes sense on SSL because of
its numerous callbacks or RSA because of RSA_METHOD.)

Change-Id: I55af537ab101682677af34f6ac1f2c27b5899a89
Reviewed-on: https://boringssl-review.googlesource.com/6849
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:29:34 +00:00
David Benjamin
4e98e5c903 Implement pkey_ec_keygen with EC_KEY APIs.
This removes the last caller of EVP_PKEY_copy_parameters within the
library.

Change-Id: I6af138d364973b18f52baf55c36c50a24a56bd44
Reviewed-on: https://boringssl-review.googlesource.com/6848
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:28:43 +00:00
David Benjamin
692878a5f4 Remove EVP_PKEY_CTRL_EC_PARAMGEN_CURVE_NID.
This is never exposed.

Change-Id: I332bc45f724eb42d68a0839e72b21593d01005ee
Reviewed-on: https://boringssl-review.googlesource.com/6847
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:27:42 +00:00
David Benjamin
8ac35f0274 Remove unused EVP_PKEY_METHOD hooks.
foo_init hooks are never implemented. Even upstream never uses them. The
flags member is also never used. We also don't expose paramgen, so
remove it.

Change-Id: I51d9439316c5163520ab7168693c457f33e59417
Reviewed-on: https://boringssl-review.googlesource.com/6846
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:23:46 +00:00
Adam Langley
ef7dba6ac7 Fix error in ce9d85ee.
I used a size_t out of habit, but |RSA_public_decrypt| is an old-style
function.

Change-Id: Ibd94d03743fe0099d61578ec15c19fa5333127db
2016-01-26 15:31:23 -08:00
Adam Langley
ce9d85eedd Tweaks for node.js
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.

Changes:
  ∙ Support verify_recover. This is very obscure and the motivation
    appears to be https://github.com/nodejs/node/issues/477 – where it's
    not clear that anyone understands what it means :(
  ∙ Add a few, no-op #defines
  ∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
    reach into these structs in the future.
  ∙ Add EC_get_builtin_curves.
  ∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
    decrepit.

Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 23:23:42 +00:00
David Benjamin
5aae776ede Remove calls to ERR_load_crypto_strings.
Since the error string logic was rewritten, this hasn't done anything.

Change-Id: Icb73dca65e852bb3c7d04c260d591906ec72c15f
Reviewed-on: https://boringssl-review.googlesource.com/6961
Reviewed-by: Adam Langley <agl@google.com>
2016-01-25 23:09:08 +00:00
Adam Langley
62882187c9 Update comments to better document in-place semantics.
(Comment-only change; no functional difference.)

Some code was broken by the |d2i_ECDSA_SIG| change in 87897a8c. It was
passing in a pointer to an existing |ECDSA_SIG| as the first argument
and then simply assuming that the structure would be updated in place.
The comments on the function suggested that this was reasonable.

This change updates the comments that use similar wording to either note
that the function will never update in-place, or else to note that
depending on that is a bad idea for the future.

I've also audited all the uses of these functions that I can find and,
in addition to the one case with |d2i_ECDSA_SIG|, there are several
users of |d2i_PrivateKey| that could become a problem in the future.
I'll try to fix them before it does become an issue.

Change-Id: I769f7b2e0b5308d09ea07dd447e02fc161795071
Reviewed-on: https://boringssl-review.googlesource.com/6902
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2016-01-19 17:01:37 +00:00
David Benjamin
e13263d5e4 Resolve a few old TODOs.
A lot of commented-out code we haven't had to put them back, so these
can go now. Also remove the TODO about OAEP having a weird API. The API
is wrong, but upstream's shipped it with the wrong API, so that's what
it is now.

Change-Id: I7da607cf2d877cbede41ccdada31380f812f6dfa
Reviewed-on: https://boringssl-review.googlesource.com/6763
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 00:14:35 +00:00
David Benjamin
c3ae38b4f8 Remove DH EVP_PKEY hooks.
They would never work. Better notice when callers depend on it than fail at
runtime.

This depends on https://android-review.googlesource.com/#/c/183610/ in
Conscrypt.

Change-Id: I3411f291416df834cf85850890617625a2e76939
Reviewed-on: https://boringssl-review.googlesource.com/6552
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 17:38:06 +00:00
Brian Smith
a3d9de05fb Add |EC_GROUP_get0_order| to replace |EC_GROUP_get_order|.
|EC_GROUP_get0_order| doesn't require any heap allocations and never
fails, so it is much more convenient and more efficient for callers to
call.

Change-Id: Ic60f768875e7bc8e74362dacdb5cbbc6957b05a6
Reviewed-on: https://boringssl-review.googlesource.com/6532
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 18:18:13 +00:00
David Benjamin
28243c08db Add PSS parameter check.
Avoid seg fault by checking mgf1 parameter is not NULL. This can be
triggered during certificate verification so could be a DoS attack
against a client or a server enabling client authentication.

Thanks to Loïc Jonas Etienne (Qnective AG) for discovering this bug.

CVE-2015-3194

(Imported from upstream's c394a488942387246653833359a5c94b5832674e and test
data from 00456fded43eadd4bb94bf675ae4ea5d158a764f.)

Change-Id: Ic97059d42722fd810973ccb0c26c415c4eaae79a
Reviewed-on: https://boringssl-review.googlesource.com/6617
Reviewed-by: Adam Langley <agl@google.com>
2015-12-03 16:47:12 +00:00
David Benjamin
cb852981cd Fix leak with ASN.1 combine.
When parsing a combined structure pass a flag to the decode routine
so on error a pointer to the parent structure is not zeroed as
this will leak any additional components in the parent.

This can leak memory in any application parsing PKCS#7 or CMS structures.

CVE-2015-3195.

Thanks to Adam Langley (Google/BoringSSL) for discovering this bug using
libFuzzer.

PR#4131

(Imported from upstream's cc598f321fbac9c04da5766243ed55d55948637d, with test
from our original report. Verified ASan trips up on the test without the fix.)

Change-Id: I007d93f172b2f16bf6845d685d72717ed840276c
Reviewed-on: https://boringssl-review.googlesource.com/6615
Reviewed-by: Adam Langley <agl@google.com>
2015-12-03 16:43:34 +00:00
David Benjamin
758d12732a Add get0 getters for EVP_PKEY.
Right now your options are:
- Bounce on a reference and deal with cleanup needlessly.
- Manually check the type tag and peek into the union.

We probably have no hope of opaquifying this struct, but for new code, let's
recommend using this function rather than the more error-prone thing.

Change-Id: I9b39ff95fe4264a3f7d1e0d2894db337aa968f6c
Reviewed-on: https://boringssl-review.googlesource.com/6551
Reviewed-by: Adam Langley <agl@google.com>
2015-11-20 23:34:12 +00:00
David Benjamin
20c373118c Become partially -Wmissing-variable-declarations-clean.
There's a few things that will be kind of a nuisance and possibly not worth it
(crypto/asn1 dumps a lot of undeclared things, etc.). But it caught some
mistakes. Even without the warning, making sure to include the externs before
defining a function helps catch type mismatches.

Change-Id: I3dab282aaba6023e7cebc94ed7a767a5d7446b08
Reviewed-on: https://boringssl-review.googlesource.com/6484
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 20:09:20 +00:00
David Benjamin
ef14b2d86e Remove stl_compat.h.
Chromium's toolchains may now assume C++11 library support, so we may freely
use C++11 features. (Chromium's still in the process of deciding what to allow,
but we use Google's style guide directly, toolchain limitations aside.)

Change-Id: I1c7feb92b7f5f51d9091a4c686649fb574ac138d
Reviewed-on: https://boringssl-review.googlesource.com/6465
Reviewed-by: Adam Langley <agl@google.com>
2015-11-11 22:19:36 +00:00
Adam Langley
d9e817309a Fix several warnings that arise in Android.
Android is now using Ninja so it doesn't spew so much to the terminal
and thus any warnings in BoringSSL (which builds really early in the
process) and much more obvious.

Thus this change fixes a few warnings that appear in the Android build.

Change-Id: Id255ace90fece772a1c3a718c877559ce920b960
Reviewed-on: https://boringssl-review.googlesource.com/6400
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-10-30 21:11:48 +00:00
Brian Smith
659806d7ff Don't default to SHA-1 in |EVP_DigestSignInit|/|EVP_DigestVerifyInit|.
This removes a hard link-time dependency on the SHA-1 code. The code
was self-contradictory in whether it defaulted to SHA-1 or refused to
default to SHA-1.

Change-Id: I5ad7949bdd529df568904f87870313e3d8a57e72
Reviewed-on: https://boringssl-review.googlesource.com/5833
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 21:26:51 +00:00
David Benjamin
301afaf223 Add a run_tests target to run all tests.
It's very annoying having to remember the right incant every time I want
to switch around between my build, build-release, build-asan, etc.,
output directories.

Unfortunately, this target is pretty unfriendly without CMake 3.2+ (and
Ninja 1.5+). This combination gives a USES_TERMINAL flag to
add_custom_target which uses Ninja's "console" pool, otherwise the
output buffering gets in the way. Ubuntu LTS is still on an older CMake,
so do a version check in the meantime.

CMake also has its own test mechanism (CTest), but this doesn't use it.
It seems to prefer knowing what all the tests are and then tries to do
its own output management and parallelizing and such. We already have
our own runners. all_tests.go could actually be converted tidily, but
generate_build_files.py also needs to read it, and runner.go has very
specific needs.

Naming the target ninja -C build test would be nice, but CTest squats
that name and CMake grumps when you use a reserved name, so I've gone
with run_tests.

Change-Id: Ibd20ebd50febe1b4e91bb19921f3bbbd9fbcf66c
Reviewed-on: https://boringssl-review.googlesource.com/6270
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 20:33:44 +00:00
Eric Roman
63fa118f3a Reject iterations=0 when calling PKCS5_PBKDF2_HMAC().
BUG=https://crbug.com/534961

Change-Id: I69e2434bf8d5564711863c393ee3bafe3763cf24
Reviewed-on: https://boringssl-review.googlesource.com/5932
Reviewed-by: Adam Langley <agl@google.com>
2015-10-13 19:40:55 +00:00
David Benjamin
93d8cf557f Add various tests for d2i_PrivateKey.
Change-Id: I030022c240d17df08cf6f59eede0e94373152c40
Reviewed-on: https://boringssl-review.googlesource.com/5950
Reviewed-by: Adam Langley <agl@google.com>
2015-09-28 22:18:23 +00:00
David Benjamin
382bc29251 Defensively avoid assuming d2i functions don't advance on error.
Although the previous commit should ensure this doesn't happen, the
uint8_t** pattern is very error-prone and we're trying to avoid doing
much to the legacy ASN.1 stack. To that end, maintaining the strong
exception guarantee w.r.t. the input pointer-pointer is best effort and
we won't rely on it, so we needn't spend our time chasing down problems.

Change-Id: Ib78974eb94377fe0b0b379f57d9695dc81f344bb
Reviewed-on: https://boringssl-review.googlesource.com/5949
Reviewed-by: Adam Langley <agl@google.com>
2015-09-28 22:15:42 +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
4c60d356a9 Work around even more Estonian ID card misissuances.
Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.

This workaround will be removed in six months.

BUG=534766

Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>
2015-09-23 20:35:47 +00:00
David Benjamin
c71567dd50 Update the Estonian workaround comments.
Target date for removal of the workaround is 6 months.

BUG=532048

Change-Id: I402f75e46736936725575559cd8eb194115ab0df
Reviewed-on: https://boringssl-review.googlesource.com/5910
Reviewed-by: Adam Langley <agl@google.com>
2015-09-18 18:42:34 +00:00
Brian Smith
1f5e9456a9 Remove superfluous SHA-1 dependency from EVP ECDSA code.
The documentation for |ECDSA_sign| and |ECDSA_verify| says that the
|type| parameter should be zero.

Change-Id: I875d3405455c5443f5a5a5c2960a9a9f486ca5bb
Reviewed-on: https://boringssl-review.googlesource.com/5832
Reviewed-by: Adam Langley <agl@google.com>
2015-09-15 23:18:44 +00:00
David Benjamin
231cb82145 Work around broken Estonian smart cards. Again.
Estonian IDs issued between September 2014 to September 2015 are broken and use
negative moduli. They last five years and are common enough that we need to
work around this bug.

Add parallel "buggy" versions of BN_cbs2unsigned and RSA_parse_public_key which
tolerate this mistake, to align with OpenSSL's previous behavior. This code is
currently hooked up to rsa_pub_decode in RSA_ASN1_METHOD so that d2i_X509 is
tolerant. (This isn't a huge deal as the rest of that stack still uses the
legacy ASN.1 code which is overly lenient in many other ways.)

In future, when Chromium isn't using crypto/x509 and has more unified
certificate handling code, we can put client certificates under a slightly
different codepath, so this needn't hold for all certificates forever. Then in
September 2019, when the broken Estonian certificates all expire, we can purge
this codepath altogether.

BUG=532048

Change-Id: Iadb245048c71dba2eec45dd066c4a6e077140751
Reviewed-on: https://boringssl-review.googlesource.com/5894
Reviewed-by: Adam Langley <agl@google.com>
2015-09-15 21:18:15 +00:00
Adam Langley
73415b6aa0 Move arm_arch.h and fix up lots of include paths.
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.

It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.

Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
2015-08-26 01:57:59 +00:00
Matt Braithwaite
685402fadd Recognize PEM-encoded DSA private keys.
This change makes |EVP_PKEY_asn1_find_str|, which is used by
|PEM_read_bio_PrivateKey|, recognize "DSA" as well as "EC" and "RSA".

Change-Id: I39cce12f600cec6a71df75312a41f8395429af62
Reviewed-on: https://boringssl-review.googlesource.com/5743
Reviewed-by: Adam Langley <agl@google.com>
2015-08-21 17:39:05 +00:00
Brian Smith
21cdada609 Fix warning about unused |EVP_PKEY_METHOD.ctrl_str|.
Some compilers in some configurations warn about this structure member
not being assigned a value. Since it is never used anywhere, just
remove it.

Change-Id: I46064234961bf449fe5fcb88594ddb3ff390e7d7
Reviewed-on: https://boringssl-review.googlesource.com/5621
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-08-07 16:11:08 +00:00
Brian Smith
906e2993a8 Fix Windows build.
When using CMake to build with MSVC, MSVC complains about unreachable
code in the <xtree> header. This incantation silences that.

Change-Id: I5fc5305dc816a009a4c59501b212fd11e290637d
Reviewed-on: https://boringssl-review.googlesource.com/5552
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-08-05 21:26:34 +00:00
David Benjamin
50f1d00bee RT3774: double-free in DSA
(Imported from upstream's 374fd385c2347b965c3490aa1c10025e1339d265.)

This codepath is only reachable on malloc failure if putting DSA private
keys into a PKCS#8 PrivateKeyInfo.

Change-Id: I88052eab3f477c4cdf5749be525878278d966a69
Reviewed-on: https://boringssl-review.googlesource.com/5543
Reviewed-by: Adam Langley <agl@google.com>
2015-07-31 22:31:37 +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
34248d4cb7 Get rid of err function codes.
Running make_errors.go every time a function is renamed is incredibly
tedious. Plus we keep getting them wrong.

Instead, sample __func__ (__FUNCTION__ in MSVC) in the OPENSSL_PUT_ERROR macro
and store it alongside file and line number. This doesn't change the format of
ERR_print_errors, however ERR_error_string_n now uses the placeholder
"OPENSSL_internal" rather than an actual function name since that only takes
the uint32_t packed error code as input.

This updates err scripts to not emit the function string table. The
OPENSSL_PUT_ERROR invocations, for now, still include the extra
parameter. That will be removed in a follow-up.

BUG=468039

Change-Id: Iaa2ef56991fb58892fa8a1283b3b8b995fbb308d
Reviewed-on: https://boringssl-review.googlesource.com/5275
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:08 +00:00
David Benjamin
7f15ff53d8 Rename crypto/{bn,evp}/asn1.c.
gyp doesn't allow two files to share the same name to avoid bugs in OS X
libtool, so asn1.c's need to all get a prefix.

Change-Id: I3593597912c49dd02655cae329fb253ed4f6f56d
Reviewed-on: https://boringssl-review.googlesource.com/5431
Reviewed-by: Adam Langley <agl@google.com>
2015-07-13 21:18:26 +00:00
David Benjamin
74f711083d Parse RSAPrivateKey with CBS.
This removes the version field from RSA and instead handles versioning
as part of parsing. (As a bonus, we now correctly limit multi-prime RSA
to version 1 keys.)

Most consumers are also converted. old_rsa_priv_{de,en}code are left
alone for now. Those hooks are passed in parameters which match the old
d2i/i2d pattern (they're only used in d2i_PrivateKey and
i2d_PrivateKey).

Include a test which, among other things, checks that public keys being
serialized as private keys are handled properly.

BUG=499653

Change-Id: Icdd5f0382c4a84f9c8867024f29756e1a306ba08
Reviewed-on: https://boringssl-review.googlesource.com/5273
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 22:50:53 +00:00
David Benjamin
c0e245a546 Parse RSAPublicKey with CBS.
BUG=499653

Change-Id: If5d98ed23e65a84f9f0e303024f91cce078f3d18
Reviewed-on: https://boringssl-review.googlesource.com/5272
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 22:39:28 +00:00
David Benjamin
2e40091dd4 PKCS8_pkey_set0 doesn't take ownership on error.
It carefully NULLs all references to the buffer, so the failure cases
still need to call OPENSSL_free.

Change-Id: Ia14341ecea81296f94e467325ab6eff1362e987e
Reviewed-on: https://boringssl-review.googlesource.com/5271
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 02:30:37 +00:00
David Benjamin
87897a8cea Implement ECDSA_SIG_{parse,marshal} with crypto/bytestring.
This is the first structure to be implemented with the new BIGNUM ASN.1
routines. Object reuse in the legacy d2i/i2d functions is implemented by
releasing whatever was in *out before and setting it to the
newly-allocated object. As with the new d2i_SSL_SESSION, this is a
weaker form of object reuse, but should suffice for reasonable callers.

As ECDSA_SIG is more likely to be parsed alone than as part of another
structure (and using CBB is slightly tedious), add convenient functions
which take byte arrays. For consistency with SSL_SESSION, they are named
to/from_bytes. from_bytes, unlike the CBS variant, rejects trailing
data.

Note this changes some test expectations: BER signatures now push an
error code. That they didn't do this was probably a mistake.

BUG=499653

Change-Id: I9ec74db53e70d9a989412cc9e2b599be0454caec
Reviewed-on: https://boringssl-review.googlesource.com/5269
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 02:28:42 +00:00
David Benjamin
a31c5bf2cc Make pem_str const-correct.
They're always constant literals.

Change-Id: I8acaaf2a8c95b02bc8b9b13740ce40044a483394
Reviewed-on: https://boringssl-review.googlesource.com/5346
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:58 +00:00
David Benjamin
42ae3adcf6 Remove info field on EVP_PKEY_ASN1_METHOD.
Nothing ever reads it.

Change-Id: Id375c461aa2feb6877a14e19eb2daefec7a03f89
Reviewed-on: https://boringssl-review.googlesource.com/5345
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:46 +00:00
David Benjamin
cce5a98efb Remove EVP_PKEY_dup.
All callers have been moved to EVP_PKEY_up_ref. (Neither spelling exists
upstream so we only had our own callers to move.)

Change-Id: I267f14054780fe3d6dc1170b7b6ae3811a0d1a9a
Reviewed-on: https://boringssl-review.googlesource.com/5291
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 21:38:39 +00:00
David Benjamin
65ee9b7ce9 Remove EVP_PKEY_HMAC.
This removes EVP_PKEY_HMAC and all the support code around it. EVP_MD requires
a lot of extra glue to support HMAC. This lets us prune it all away.

As a bonus, it removes a (minor) dependency from EVP to the legacy ASN.1 stack.

Change-Id: I5a9e3e39f518429828dbf13d14647fb37d9dc35a
Reviewed-on: https://boringssl-review.googlesource.com/5120
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 00:03:02 +00:00
Matt Braithwaite
e65886a520 dsa_pub_encode: Write out DSA parameters (p, q, g) in addition to key.
Change-Id: Id5ea49fc43aacfd1d348b2a230c9745484bed852
Reviewed-on: https://boringssl-review.googlesource.com/5174
Reviewed-by: Adam Langley <agl@google.com>
2015-06-22 23:58:26 +00:00