Commit Graph

18 Commits

Author SHA1 Message Date
David Benjamin
ce45588695 Speculatively remove __STDC_*_MACROS.
C99 added macros such as PRIu64 to inttypes.h, but it said to exclude them from
C++ unless __STDC_FORMAT_MACROS or __STDC_CONSTANT_MACROS was defined. This
text was never incorporated into any C++ standard and explicitly overruled in
C++11.

Some libc headers followed C99. Notably, glibc prior to 2.18
(https://sourceware.org/bugzilla/show_bug.cgi?id=15366) and old versions of the
Android NDK.

In the NDK, although it was fixed some time ago (API level 20), the NDK used to
use separate headers per API level. Only applications using minSdkVersion >= 20
would get the fix. Starting NDK r14, "unified" headers are available which,
among other things, make the fix available (opt-in) independent of
minSdkVersion. In r15, unified headers are opt-out, and in r16 they are
mandatory.

Try removing these and see if anyone notices. The former is past our five year
watermark. The latter is not and Android has hit
https://boringssl-review.googlesource.com/c/boringssl/+/32686 before, but
unless it is really widespread, it's probably simpler to ask consumers to
define __STDC_CONSTANT_MACROS and __STDC_FORMAT_MACROS globally.

Update-Note: If you see compile failures relating to PRIu64, UINT64_MAX, and
friends, update your glibc or NDK. As a short-term fix, add
__STDC_CONSTANT_MACROS and __STDC_FORMAT_MACROS to your build, but get in touch
so we have a sense of how widespread it is.

Bug: 198
Change-Id: I56cca5f9acdff803de1748254bc45096e4c959c2
Reviewed-on: https://boringssl-review.googlesource.com/c/33146
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-11-14 16:14:37 +00:00
Joshua Liebow-Feeser
8c7c6356e6 Support symbol prefixes
- In base.h, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols.h
- In all .S files, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols_asm.h
- In base.h, BSSL_NAMESPACE_BEGIN and BSSL_NAMESPACE_END are
  defined with appropriate values depending on whether
  BORINGSSL_PREFIX is defined; these macros are used in place
  of 'namespace bssl {' and '}'
- Add util/make_prefix_headers.go, which takes a list of symbols
  and auto-generates the header files mentioned above
- In CMakeLists.txt, if BORINGSSL_PREFIX and BORINGSSL_PREFIX_SYMBOLS
  are defined, run util/make_prefix_headers.go to generate header
  files
- In various CMakeLists.txt files, add "global_target" that all
  targets depend on to give us a place to hook logic that must run
  before all other targets (in particular, the header file generation
  logic)
- Document this in BUILDING.md, including the fact that it is
  the caller's responsibility to provide the symbol list and keep it
  up to date
- Note that this scheme has not been tested on Windows, and likely
  does not work on it; Windows support will need to be added in a
  future commit

Change-Id: If66a7157f46b5b66230ef91e15826b910cf979a2
Reviewed-on: https://boringssl-review.googlesource.com/31364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-09-06 20:07:52 +00:00
David Benjamin
bfdd1a9308 Give SSL_SESSION a destructor.
Previously we'd partially attempted the ssl_st / bssl::SSLConnection
subclassing split, but that gets messy when we actually try to add a
destructor, because CRYPTO_EX_DATA's cleanup function needs an ssl_st*,
not a bssl::SSLConnection*. Downcasting is technically undefined at this
point and will likely offend some CFI-like check.

Moreover, it appears that even with today's subclassing split,
New<SSL>() emits symbols like:

W ssl_st*& std::forward<ssl_st*&>(std::remove_reference<ssl_st*&>::type&)

The compiler does not bother emitting them in optimized builds, but it
does suggest we can't really avoid claiming the ssl_st type name at the
symbol level, short of doing reinterpret_casts at all API boundaries.
And, of course, we've already long claimed it at the #include level.

So I've just left this defining directly on ssl_session_st. The cost is
we need to write some silly "bssl::" prefixes in the headers, but so it
goes. In the likely event we change our minds again, we can always
revise this.

Change-Id: Ieb429e8eaabe7c2961ef7f8d9234fb71f19a5e2a
Reviewed-on: https://boringssl-review.googlesource.com/29587
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>
2018-07-03 22:57:56 +00:00
David Benjamin
2908dd141f Add bssl::UpRef.
bssl::UniquePtr and FOO_up_ref do not play well together. Add a helper
to simplify this. This allows us to write things like:

   foo->cert = UpRef(bar->cert);

instead of:

   if (bar->cert) {
     X509_up_ref(bar->cert.get());
   }
   foo->cert.reset(bar->cert.get());

This also plays well with PushToStack. To append something to a stack
while taking a reference, it's just:

   PushToStack(certs, UpRef(cert))

Change-Id: I99ae8de22b837588a2d8ffb58f86edc1d03ed46a
Reviewed-on: https://boringssl-review.googlesource.com/29584
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>
2018-07-03 22:47:36 +00:00
David Benjamin
a3a71e9d33 Flip SSL_SESSION fields to bool.
Change-Id: I0a3648437f78f37a4b710aaea73084a19be607c5
Reviewed-on: https://boringssl-review.googlesource.com/29485
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-07-03 21:26:16 +00:00
Adam Langley
e8d2439cd3 Expose ssl_session_serialize to libssl.
This function can serialise a session to a |CBB|.

Change-Id: Icdb7aef900f03f947c3fa4625dd218401eb8eafc
Reviewed-on: https://boringssl-review.googlesource.com/25385
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-01-26 22:31:47 +00:00
Adam Langley
c61b577197 Add some more utility functions to bytestring.
Change-Id: I7932258890b0b2226ff6841af45926e1b11979ba
Reviewed-on: https://boringssl-review.googlesource.com/24844
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-25 23:51:36 +00:00
David Benjamin
66801feb17 Support high tag numbers in CBS/CBB.
Android's attestion format uses some ludicrously large tag numbers:
https://developer.android.com/training/articles/security-key-attestation.html#certificate_schema

Add support for these in CBS/CBB. The public API does not change for
callers who were using the CBS_ASN1_* constants, but it is no longer the
case that tag representations match their DER encodings for small tag
numbers.

Chromium needs https://chromium-review.googlesource.com/#/c/chromium/src/+/783254,
but otherwise I don't expect this to break things.

Bug: 214
Change-Id: I9b5dc27ae3ea020e9edaabec4d665fd73da7d31e
Reviewed-on: https://boringssl-review.googlesource.com/23304
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-11-22 22:34:05 +00:00
David Benjamin
a861460c89 Make SNI per-connection, not per-session.
Right now we report the per-connection value during the handshake and
the per-session value after the handshake. This also trims our tickets
slightly by removing a largely unused field from SSL_SESSION.

Putting it on SSL_HANDSHAKE would be better, but sadly a number of
bindings-type APIs expose it after the handshake.

Change-Id: I6a1383f95da9b1b141b9d6adadc05ee1e458a326
Reviewed-on: https://boringssl-review.googlesource.com/20064
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-06 20:25:26 +00:00
David Benjamin
c11ea942b7 Convert comments in ssl.
That's the last of it!

Change-Id: I93d1f5ab7e95b2ad105c34b24297a0bf77625263
Reviewed-on: https://boringssl-review.googlesource.com/19784
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-08-29 21:33:32 +00:00
David Benjamin
8fc2dc07d8 Put SCTs and OCSP responses in CRYPTO_BUFFERs.
They both can be moderately large. This should hopefully relieve a little
memory pressure from both connections to hosts which serve SCTs and
TLS 1.3's single-use tickets.

Change-Id: I034bbf057fe5a064015a0f554b3ae9ea7797cd4e
Reviewed-on: https://boringssl-review.googlesource.com/19584
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-08-23 15:58:52 +00:00
David Benjamin
ee910bfe24 Use new STACK_OF helpers.
Bug: 132
Change-Id: Ib9bc3ce5f60d0c5bf7922b3d3ccfcd15ef4972a1
Reviewed-on: https://boringssl-review.googlesource.com/18466
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-28 21:43:13 +00:00
David Benjamin
d6a8a5a54d Remove obsolete TODOs.
Looks like they're using the pool now.

Change-Id: Ieeb1cacb9cb039d35ff091bc9742262f0fc5b146
Reviewed-on: https://boringssl-review.googlesource.com/18364
Reviewed-by: Adam Langley <agl@google.com>
2017-07-24 22:28:34 +00:00
David Benjamin
31b0c9be30 Add a bunch of scopers.
I started by switching a couple fields to SSL_HANDSHAKE and then kept
following transitive bits.

Bug: 132
Change-Id: I640dadd3558615fa38c7e8498d4efe7449b0658f
Reviewed-on: https://boringssl-review.googlesource.com/18245
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-20 20:38:55 +00:00
David Benjamin
e39ac8fb59 Switch BORINGSSL_INTERNAL_CXX_TYPES in favor of subclassing games.
The previous attempt around the 'struct ssl_st' compatibility mess
offended OSS-Fuzz and UBSan because one compilation unit passed a
function pointer with ssl_st* and another called it with
bssl::SSLConnection*.

Linkers don't retain such types, of course, but to silence this alert,
instead make C-visible types be separate from the implementation and
subclass the public type. This does mean we risk polluting the symbol
namespace, but hopefully the compiler is smart enough to inline the
visible struct's constructor and destructor.

Bug: 132
Change-Id: Ia75a89b3a22a202883ad671a630b72d0aeef680e
Reviewed-on: https://boringssl-review.googlesource.com/18224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-07-20 17:24:12 +00:00
David Benjamin
1386aad102 Switch various things to scopers.
Clear out some of the easy cases.

Bug: 132
Change-Id: Icd5c246cb6bec4a96c72eccd6569235c3d030ebd
Reviewed-on: https://boringssl-review.googlesource.com/18204
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-07-20 16:29:33 +00:00
David Benjamin
86e95b852e Move libssl's internals into the bssl namespace.
This is horrible, but everything else I tried was worse. The goal with
this CL is to take the extern "C" out of ssl/internal.h and move most
symbols to namespace bssl, so we can start using C++ helpers and
destructors without worry.

Complications:

- Public API functions must be extern "C" and match their declaration in
  ssl.h, which is unnamespaced. C++ really does not want you to
  interleave namespaced and unnamespaced things. One can actually write
  a namespaced extern "C" function, but this means, from C++'s
  perspective, the function is namespaced. Trying to namespace the
  public header would worked but ended up too deep a rabbithole.

- Our STACK_OF macros do not work right in namespaces.

- The typedefs for our exposed but opaque types are visible in the
  header files and copied into consuming projects as forward
  declarations. We ultimately want to give SSL a destructor, but
  clobbering an unnamespaced ssl_st::~ssl_st seems bad manners.

- MSVC complains about ambiguous names if one typedefs SSL to bssl::SSL.

This CL opts for:

- ssl/*.cc must begin with #define BORINGSSL_INTERNAL_CXX_TYPES. This
  informs the public headers to create forward declarations which are
  compatible with our namespaces.

- For now, C++-defined type FOO ends up at bssl::FOO with a typedef
  outside. Later I imagine we'll rename many of them.

- Internal functions get namespace bssl, so we stop worrying about
  stomping the tls1_prf symbol. Exported C functions are stuck as they
  are. Rather than try anything weird, bite the bullet and reorder files
  which have a mix of public and private functions. I expect that over
  time, the public functions will become fairly small as we move logic
  to more idiomatic C++.

  Files without any public C functions can just be written normally.

- To avoid MSVC troubles, some bssl types are renamed to CPlusPlusStyle
  in advance of them being made idiomatic C++.

Bug: 132
Change-Id: Ic931895e117c38b14ff8d6e5a273e868796c7581
Reviewed-on: https://boringssl-review.googlesource.com/18124
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-19 19:10:59 +00:00
David Benjamin
e8703a3708 Switch a number of files to C++.
http://i1.kym-cdn.com/photos/images/original/000/242/631/382.gif

In the first step, switch C files to C++ individually, keeping
everything in internal.h C-compatible. We'll make minimal changes needed
to get things compiling (notably a lot of goto errs will need to turn to
bssl::UniquePtr right away), but more aggressive changes will happen in
later steps.

(To avoid a rebase, I'm intentionally avoiding files that would conflict
with CLs in flight right now.)

Bug: 132
Change-Id: Id4cfd722e7b57d1df11f27236b4658b5d39b5fd2
Reviewed-on: https://boringssl-review.googlesource.com/17667
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-12 20:54:02 +00:00