Browse Source

Bring ERR_ERROR_STRING_BUF_LEN down to 120.

Originally, the only OpenSSL API to stringify errors was:

  char *ERR_error_string(unsigned long e, char *buf);

This API leaves callers a choice to either be thread unsafe (buf = NULL)
or pass in a buffer with unknown size. Indeed the original
implementation was just a bunch of unchecked sprintfs with, in the buf =
NULL case, a static 256-byte buffer.

388f2f56f2/crypto/err/err.c (L374)

Then ERR_error_string was documented that the buffer must be size 120.
Nowhere in the code was 120 significant. I expect OpenSSL just made up a
number.

388f2f56f2

Then upstream added the ERR_error_string_n API. Although the
documentation stated 120 bytes, the internal buffer was 256, so the code
actually translates ERR_error_string to ERR_error_string_n(e, buf, 256),
not ERR_error_string_n(e, buf, 120)!

e5c84d5152

So the documentation was wrong all this time! OpenSSL 1.1.0 corrected
the documentation to 256, but, alas, a lot of code used the
documentation and sized the buffer at 120. We should fix all
ERR_error_string callers to ERR_error_string_n but, in the meantime,
using 120 is probably less effort.

Note this also affects ERR_print_errors_cb right now. We don't have
function codes, so 120 bytes leaves 60 bytes for the reason code. Our
longest one, TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST is 46 bytes,
so it's a little tight, but, if needed, we can recover 20-ish bytes by
shrinking the library names. We can also always make ERR_print_errors_cb
use a larger buffer.

Change-Id: I472a1a802f2e6281cc7515d2a452208d6bac1200
Reviewed-on: https://boringssl-review.googlesource.com/24184
Reviewed-by: Adam Langley <agl@google.com>
kris/onging/CECPQ3_patch15
David Benjamin 6 years ago
committed by Adam Langley
parent
commit
ebd87230ac
1 changed files with 1 additions and 1 deletions
  1. +1
    -1
      include/openssl/err.h

+ 1
- 1
include/openssl/err.h View File

@@ -395,7 +395,7 @@ OPENSSL_EXPORT const char *ERR_func_error_string(uint32_t packed_error);
// //
// TODO(fork): remove this function. // TODO(fork): remove this function.
OPENSSL_EXPORT char *ERR_error_string(uint32_t packed_error, char *buf); OPENSSL_EXPORT char *ERR_error_string(uint32_t packed_error, char *buf);
#define ERR_ERROR_STRING_BUF_LEN 256
#define ERR_ERROR_STRING_BUF_LEN 120


// ERR_GET_FUNC returns zero. BoringSSL errors do not report a function code. // ERR_GET_FUNC returns zero. BoringSSL errors do not report a function code.
#define ERR_GET_FUNC(packed_error) 0 #define ERR_GET_FUNC(packed_error) 0


Loading…
Cancel
Save