Commit Graph

16 Commits

Author SHA1 Message Date
David Benjamin
769b386e97 Fix error handling/cleanup
(Imported from upstream's 4d2df46cb38603c98fb49543738289c9176571d8.)

Change-Id: I62e5d6fa58c57c4f5d30d00baf14f2024278c1de
Reviewed-on: https://boringssl-review.googlesource.com/20104
Commit-Queue: David Benjamin <davidben@google.com>
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-09-07 22:06:50 +00:00
Adam Langley
ccf80574e9 Use functions that do not depend on the current locale.
X.509 functions and the like should not vary their behaviour based on
the configured locale, but tolower(3), strcasecmp(3) and strncasecmp(3)
change behaviour based on that.

For example, with tr_TR.utf8, 'I' is not the upper-case version of 'i'.

Change-Id: I896a285767ae0c22e6ce06b9908331c625e90af2
Reviewed-on: https://boringssl-review.googlesource.com/18412
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-07-25 22:53:33 +00:00
David Benjamin
01f8a8c2d5 Convert stack.h to use inline functions.
Instead of a script which generates macros, emit static inlines in
individual header (or C files). This solves a few issues with the
original setup:

- The documentation was off. We match the documentation now.

- The stack macros did not check constness; see some of the fixes in
  crypto/x509.

- Type errors did not look like usual type errors.

- Any type which participated in STACK_OF had to be made partially
  public. This allows stack types to be defined an internal header or
  even an individual file.

- One could not pass sk_FOO_free into something which expects a function
  pointer.

Thanks to upstream's 411abf2dd37974a5baa54859c1abcd287b3c1181 for the
idea.

Change-Id: Ie5431390ccad761c17596b0e93941b0d7a68f904
Reviewed-on: https://boringssl-review.googlesource.com/16087
Reviewed-by: Adam Langley <agl@google.com>
2017-05-22 15:06:04 +00:00
David Benjamin
c4d5925ca6 Fix various malloc failure checks.
asn1_template_noexp_d2i call ASN1_item_ex_free(&skfield,...) on error.

Reworked error handling in asn1_item_ex_combine_new:
- call ASN1_item_ex_free and return the correct error code if
  ASN1_template_new failed.
- dont call ASN1_item_ex_free if ASN1_OP_NEW_PRE failed.

Reworked error handing in x509_name_ex_d2i and x509_name_encode.

(Imported from upstream's 748cb9a17f4f2b77aad816cf658cd4025dc847ee.)

I believe the tasn1_new.c change is a no-op since we have no
ASN1_OP_NEW_PRE hooks anymore. I'm not sure what the commit message is
referring to with ASN1_template_new. It also seems odd as
ASN1_item_ex_free should probably be able to survive *pval being NULL.
Whatever.

We'd previously tried to fix x509_name_ex_d2i, but I think ours wasn't
quite right. (This thing is a mess...) I've aligned that function with
upstream.

Change-Id: Ie71521cd8a1ec357876caadd13be1ce247110f76
Reviewed-on: https://boringssl-review.googlesource.com/13831
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-02-17 01:11:21 +00:00
David Benjamin
17cf2cb1d2 Work around language and compiler bug in memcpy, etc.
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>
2016-12-21 20:34:47 +00:00
Jinguang Dong
8499621d21 Check for i2d_name_canon failures in x509_name_canon.
Change-Id: I0e1d79e85a2d20ab4105b81d39cdbbd692ba67da
Reviewed-on: https://boringssl-review.googlesource.com/12221
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>
2016-11-11 10:45:03 +00:00
David Benjamin
e77b16ef71 Remove ASN.1 print hooks.
These functions are never instantiated. (They're a remnant of the PKCS#7 and
CMS bits.) Next time upstream touches this code, we don't have to puzzle
through the diff and import it.

Change-Id: I67c2102ae13e8e0527d858e1c63637dd442a4ffb
Reviewed-on: https://boringssl-review.googlesource.com/8242
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-14 17:38:31 +00:00
David Benjamin
d0c677cd8e Avoid illegal pointers in asn1_string_canon.
(Imported from upstream's 3892b95750b6aa5ed4328a287068f7cdfb9e55bc.)

More reasonable would have been to drop |to| altogether and act on from[len-1],
but I suppose this works.

Change-Id: I280b4991042b4d330ba034f6a631f8421ddb2643
Reviewed-on: https://boringssl-review.googlesource.com/8241
Reviewed-by: Adam Langley <agl@google.com>
2016-06-13 21:57:05 +00:00
David Benjamin
e31e0123ea Add size limit to X509_NAME structure.
This adds an explicit limit to the size of an X509_NAME structure. Some
part of OpenSSL (e.g. TLS) already effectively limit the size due to
restrictions on certificate size.

See also upstream's 65cb92f4da37a3895437f0c9940ee0bcf9f28c8a, although this is
different from upstream's. Upstream's version bounds both the X509_NAME *and*
any data after it in the immediately containing structure. While adding a bound
on all of crypto/asn1 is almost certainly a good idea (will look into that for
a follow-up), it seems bizarre and unnecessary to have X509_NAME affect its
parent.

Change-Id: Ica2136bcd1455d7c501ccc6ef2a19bc5ed042501
Reviewed-on: https://boringssl-review.googlesource.com/7846
Reviewed-by: Adam Langley <agl@google.com>
2016-05-03 16:43:52 +00:00
Adam Langley
57707c70dc OpenSSL reformat x509/, x509v3/, pem/ and asn1/.
OpenSSL upstream did a bulk reformat. We still have some files that have
the old OpenSSL style and this makes applying patches to them more
manual, and thus more error-prone, than it should be.

This change is the result of running
  util/openssl-format-source -v -c .
in the enumerated directories. A few files were in BoringSSL style and
have not been touched.

This change should be formatting only; no semantic difference.

Change-Id: I75ced2970ae22b9facb930a79798350a09c5111e
Reviewed-on: https://boringssl-review.googlesource.com/6904
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2016-01-19 17:01:51 +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
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
9e128b06a1 Fix memory leak on malloc failure.
Found by running malloc tests with -valgrind. Unfortunately, the next one is
deep in crypto/asn1 itself, so I'm going to stop here for now.

Change-Id: I7a33971ee07c6b7b7a98715f2f18e0f29380c0a1
Reviewed-on: https://boringssl-review.googlesource.com/3350
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:23:34 +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
Adam Langley
69a01608f3 Add malloc failure tests.
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.

(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)

This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.

Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
2014-11-19 01:24:46 +00: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