From: Arran Cudbard-Bell Date: Mon, 28 Jun 2021 23:18:14 +0000 (-0500) Subject: Rework TLS BIOs to allow bidirectional producer/consumer, and multiple heap allocated... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7dd0e0d6c625ee65d4f139d3231e526df97308c7;p=thirdparty%2Ffreeradius-server.git Rework TLS BIOs to allow bidirectional producer/consumer, and multiple heap allocated BIOs --- diff --git a/src/lib/tls/bio.c b/src/lib/tls/bio.c index 5ecedf2f9bf..8177f87d7a1 100644 --- a/src/lib/tls/bio.c +++ b/src/lib/tls/bio.c @@ -30,30 +30,25 @@ USES_APPLE_DEPRECATED_API /* OpenSSL API has been deprecated by Apple */ #include "bio.h" -/** Holds the state of a talloc aggregation BIO +/** Holds the state of a talloc aggregation 'write' BIO * - * Most of these fields are expected to change between uses of the BIO. - * - * BIOs do not have indexed extension structures like other structures in OpenSSL, - * so we're forced to place all information in a structure, and populate it just - * prior to a BIO being used. - * - * These BIOs are thread local to avoid conflicts or locking issues. + * With these BIOs OpenSSL is the producer, and we're the consumer. */ -typedef struct { +struct fr_tls_bio_dbuff_s { BIO *bio; //!< Logging bio to write to. - TALLOC_CTX *ctx; //!< Talloc ctx - fr_dbuff_t dbuff; //!< Used to aggregate data. + fr_dbuff_t dbuff_in; //!< dbuff used to write data to our talloced buffer. + fr_dbuff_t dbuff_out; //!< dbuff used to read data from our talloced buffer. fr_dbuff_uctx_talloc_t tctx; //!< extra talloc information for the dbuff. -} fr_tls_bio_talloc_agg_t; + bool free_buff; //!< Free the talloced buffer when this structure is freed. +}; /** Template for the thread local request log BIOs */ -static BIO_METHOD *tls_bio_talloc_agg_meth; +static BIO_METHOD *tls_bio_talloc_meth; /** Thread local aggregation BIO */ -static _Thread_local fr_tls_bio_talloc_agg_t *tls_bio_talloc_agg; +static _Thread_local fr_tls_bio_dbuff_t *tls_bio_talloc_agg; /** Aggregates BIO_write() calls into a talloc'd buffer * @@ -61,13 +56,19 @@ static _Thread_local fr_tls_bio_talloc_agg_t *tls_bio_talloc_agg; * @param[in] in data being written to BIO. * @param[in] len Length of data being written. */ -static int tls_bio_talloc_agg_write_cb(BIO *bio, char const *in, int len) +static int _tls_bio_talloc_write_cb(BIO *bio, char const *in, int len) { - fr_tls_bio_talloc_agg_t *ab = talloc_get_type_abort(BIO_get_data(bio), fr_tls_bio_talloc_agg_t); + fr_tls_bio_dbuff_t *bd = talloc_get_type_abort(BIO_get_data(bio), fr_tls_bio_dbuff_t); - fr_assert_msg(ab->dbuff.buff, "BIO not initialised"); + fr_assert_msg(bd->dbuff_in.buff, "BIO not initialised"); - return fr_dbuff_in_memcpy_partial(&ab->dbuff, (uint8_t const *)in, (size_t)len); + /* + * Shift out any data which has been read + * since we were last called. + */ + fr_dbuff_shift(&bd->dbuff_in, fr_dbuff_used(&bd->dbuff_out)); + + return fr_dbuff_in_memcpy_partial(&bd->dbuff_in, (uint8_t const *)in, (size_t)len); } /** Aggregates BIO_puts() calls into a talloc'd buffer @@ -75,55 +76,91 @@ static int tls_bio_talloc_agg_write_cb(BIO *bio, char const *in, int len) * @param[in] bio that was written to. * @param[in] in data being written to BIO. */ -static int tls_bio_talloc_agg_puts_cb(BIO *bio, char const *in) +static int _tls_bio_talloc_puts_cb(BIO *bio, char const *in) { - return tls_bio_talloc_agg_write_cb(bio, in, strlen(in)); + return _tls_bio_talloc_write_cb(bio, in, strlen(in)); } -/** Frees a logging bio and its underlying OpenSSL BIO * +/** Serves BIO_read() from a talloced buffer * + * @param[in] bio performing the read operation. + * @param[out] buf to write data to. + * @param[in] size of data to write (maximum). + * @return The amount of data written. */ -static void _fr_tls_bio_talloc_agg_free(void *bio_talloc_agg) +static int _tls_bio_talloc_read_cb(BIO *bio, char *buf, int size) { - fr_tls_bio_talloc_agg_t *our_bio_talloc_agg = talloc_get_type_abort(bio_talloc_agg, fr_tls_bio_talloc_agg_t); + fr_tls_bio_dbuff_t *bd = talloc_get_type_abort(BIO_get_data(bio), fr_tls_bio_dbuff_t); + size_t to_copy; + ssize_t slen; + + fr_assert_msg(bd->dbuff_out.buff, "BIO not initialised"); + + to_copy = fr_dbuff_remaining(&bd->dbuff_out); + if (to_copy > (size_t)size) to_copy = (size_t)size; - BIO_free(our_bio_talloc_agg->bio); - our_bio_talloc_agg->bio = NULL; - talloc_free(our_bio_talloc_agg); + slen = fr_dbuff_out_memcpy((uint8_t *)buf, &bd->dbuff_out, to_copy); + if (!fr_cond_assert(slen >= 0)) { /* Shouldn't happen */ + buf[0] = '\0'; + return (int)slen; + } + + fr_dbuff_shift(&bd->dbuff_in, (size_t)slen); /* Shift contents */ + + return (int)slen; } -/** Return a BIO which will aggregate data in an expandable talloc buffer +/** Serves BIO_gets() from a talloced buffer * - * @note Only one of these BIOs may be in use at a given time. + * Writes all data up to size, or up to and including the next \n to + * the provided buffer. * - * @param[in] init how much memory to allocate initially. - * @param[in] max the maximum amount of memory to allocate (0 for unlimited). - * @return A thread local BIO to pass to OpenSSL logging functions. + * @param[in] bio performing the gets operation. + * @param[out] buf to write data to. + * @param[in] size of data to write (maximum). + * @return The amount of data written. */ -BIO *fr_tls_bio_talloc_agg(TALLOC_CTX *ctx, size_t init, size_t max) +static int _tls_bio_talloc_gets_cb(BIO *bio, char *buf, int size) { - if (unlikely(!tls_bio_talloc_agg)) { - fr_tls_bio_talloc_agg_t *ab; + fr_tls_bio_dbuff_t *bd = talloc_get_type_abort(BIO_get_data(bio), fr_tls_bio_dbuff_t); + size_t to_copy; + uint8_t *p; + ssize_t slen; - MEM(ab = talloc(NULL, fr_tls_bio_talloc_agg_t)); - *ab = (fr_tls_bio_talloc_agg_t) { - .bio = BIO_new(tls_bio_talloc_agg_meth), - .ctx = ctx, - }; - MEM(ab->bio); - BIO_set_data(ab->bio, ab); /* So we can retrieve the fr_tls_bio_talloc_agg_t in the callbacks */ + fr_assert_msg(bd->dbuff_out.buff, "BIO not initialised"); - MEM(fr_dbuff_init_talloc(ctx, &ab->dbuff, &ab->tctx, init, max)); - fr_atexit_thread_local(tls_bio_talloc_agg, _fr_tls_bio_talloc_agg_free, ab); + /* + * Deal with stupid corner case + */ + if (unlikely(size == 1)) { + buf[0] = '\0'; + return 0; + } else if (unlikely(size == 0)) { + return 0; + } - return ab->bio; + /* + * Copy up to the next line, or the end of the buffer + */ + p = memchr(fr_dbuff_current(&bd->dbuff_out), '\n', fr_dbuff_remaining(&bd->dbuff_out)); + if (!p) { + to_copy = fr_dbuff_remaining(&bd->dbuff_out); + } else { + to_copy = (p - fr_dbuff_current(&bd->dbuff_out)) + 1; /* Preserve the \n as per BIO_read() man page */ } - fr_assert_msg(!tls_bio_talloc_agg->dbuff.buff, "BIO not finialised"); + if (to_copy >= (size_t)size) to_copy = (size_t)size - 1; /* Space for \0 */ - MEM(fr_dbuff_init_talloc(ctx, &tls_bio_talloc_agg->dbuff, &tls_bio_talloc_agg->tctx, init, max)); + slen = fr_dbuff_out_memcpy((uint8_t *)buf, &bd->dbuff_out, to_copy); + if (!fr_cond_assert(slen > 0)) { /* Shouldn't happen */ + buf[0] = '\0'; + return (int)slen; + } - return tls_bio_talloc_agg->bio; + buf[to_copy] = '\0'; + fr_dbuff_shift(&bd->dbuff_in, (size_t)slen); /* Shift contents */ + + return (int)to_copy; } /** Finalise a talloc aggregation buffer, returning the underlying talloc array holding the data @@ -132,55 +169,192 @@ BIO *fr_tls_bio_talloc_agg(TALLOC_CTX *ctx, size_t init, size_t max) * - NULL if the aggregation buffer wasn't initialised. * - A talloc_array holding the aggregated data. */ -uint8_t *fr_tls_bio_talloc_agg_finalise(void) +uint8_t *fr_tls_bio_dbuff_finalise(fr_tls_bio_dbuff_t *bd) { uint8_t *buff; - if (unlikely(!tls_bio_talloc_agg)) return NULL; - if (unlikely(!tls_bio_talloc_agg->dbuff.buff)) return NULL; + if (unlikely(!bd)) return NULL; + if (unlikely(!bd->dbuff_in.buff)) return NULL; - fr_dbuff_trim_talloc(&tls_bio_talloc_agg->dbuff, SIZE_MAX); + fr_dbuff_trim_talloc(&bd->dbuff_in, SIZE_MAX); - buff = tls_bio_talloc_agg->dbuff.buff; - tls_bio_talloc_agg->dbuff.buff = NULL; + buff = bd->dbuff_in.buff; + bd->dbuff_in.buff = NULL; + bd->dbuff_out.buff = NULL; return buff; } /** Finalise a talloc aggregation buffer, returning the underlying talloc array holding the data - * - * This variant adds an additional \0 byte, and sets the talloc chunk type to char. * * @return * - NULL if the aggregation buffer wasn't initialised. * - A talloc_array holding the aggregated data. */ -char *fr_tls_bio_talloc_agg_finalise_bstr(void) +char *fr_tls_bio_dbuff_finalise_bstr(fr_tls_bio_dbuff_t *bd) { uint8_t *buff; - if (unlikely(!tls_bio_talloc_agg)) return NULL; - if (unlikely(!tls_bio_talloc_agg->dbuff.buff)) return NULL; - - fr_dbuff_in_bytes(&tls_bio_talloc_agg->dbuff, 0x00); + if (unlikely(!bd)) return NULL; + if (unlikely(!bd->dbuff_in.buff)) return NULL; - fr_dbuff_trim_talloc(&tls_bio_talloc_agg->dbuff, SIZE_MAX); + fr_dbuff_in_bytes(&bd->dbuff_in, 0x00); + fr_dbuff_trim_talloc(&bd->dbuff_in, SIZE_MAX); - buff = tls_bio_talloc_agg->dbuff.buff; - tls_bio_talloc_agg->dbuff.buff = NULL; + buff = bd->dbuff_in.buff; + bd->dbuff_in.buff = NULL; + bd->dbuff_out.buff = NULL; talloc_set_type(buff, char); return (char *)buff; } +/* Reset pointer positions for in/out + * + * Leaves the underlying buffer intact to avoid useless free/malloc. + */ +void fr_tls_bio_dbuff_reset(fr_tls_bio_dbuff_t *bd) +{ + fr_dbuff_set_to_start(&bd->dbuff_in); +} + +/** Free the underlying BIO, and the buffer if it wasn't finalised + * + */ +static int _fr_tls_bio_dbuff_free(fr_tls_bio_dbuff_t *bd) +{ + BIO_free(bd->bio); + if (bd->free_buff) fr_dbuff_free_talloc(&bd->dbuff_out); + + return 0; +} + +/** Return the output dbuff + * + */ +fr_dbuff_t *fr_tls_bio_dbuff_out(fr_tls_bio_dbuff_t *bd) +{ + return &bd->dbuff_out; +} + +/** Return the input dbuff + * + */ +fr_dbuff_t *fr_tls_bio_dbuff_in(fr_tls_bio_dbuff_t *bd) +{ + return &bd->dbuff_in; +} + +/** Allocate a new BIO/talloc buffer + * + * @param[out] out Where to write a pointer to the #fr_tls_bio_dbuff_t. + * When this structure is freed the underlying BIO * + * will also be freed. May be NULL. + * @param[in] bio_ctx to allocate the BIO and wrapper struct in. May be NULL. + * @param[in] buff_ctx to allocate the expanding buffer in. May be NULL. + * @param[in] init how much memory to allocate initially. + * @param[in] max the maximum amount of memory to allocate (0 for unlimited). + * @param[in] free_buff free the talloced buffer when the #fr_tls_bio_dbuff_t is + * freed. + * @return + * - A new BIO - Do not free manually, free the #fr_tls_bio_dbuff_t or + * the ctx containing it instead. + */ +BIO *fr_tls_bio_dbuff_alloc(fr_tls_bio_dbuff_t **out, TALLOC_CTX *bio_ctx, TALLOC_CTX *buff_ctx, + size_t init, size_t max, bool free_buff) +{ + fr_tls_bio_dbuff_t *bd; + + MEM(bd = talloc_zero(bio_ctx, fr_tls_bio_dbuff_t)); + MEM(bd->bio = BIO_new(tls_bio_talloc_meth)); + BIO_set_data(bd->bio, bd); + + /* + * Initialise the dbuffs + */ + MEM(fr_dbuff_init_talloc(buff_ctx, &bd->dbuff_out, &bd->tctx, init, max)); /* Where we read from */ + bd->dbuff_in = FR_DBUFF_BIND_END_ABS(&bd->dbuff_out); /* Where we write to */ + bd->dbuff_out.is_const = 1; + bd->free_buff = free_buff; + + talloc_set_destructor(bd, _fr_tls_bio_dbuff_free); + + if (out) *out = bd; + + return bd->bio; +} + +/** Finalise a talloc aggregation buffer, returning the underlying talloc array holding the data + * + * @return + * - NULL if the aggregation buffer wasn't initialised. + * - A talloc_array holding the aggregated data. + */ +uint8_t *fr_tls_bio_dbuff_thread_local_finalise(void) +{ + return fr_tls_bio_dbuff_finalise(tls_bio_talloc_agg); +} + +/** Finalise a talloc aggregation buffer, returning the underlying talloc array holding the data + * + * This variant adds an additional \0 byte, and sets the talloc chunk type to char. + * + * @return + * - NULL if the aggregation buffer wasn't initialised. + * - A talloc_array holding the aggregated data. + */ +char *fr_tls_bio_dbuff_thread_local_finalise_bstr(void) +{ + return fr_tls_bio_dbuff_finalise_bstr(tls_bio_talloc_agg); +} + /** Discard any data in a talloc aggregation buffer * + * fr_tls_bio_dbuff_thread_local must be called again before using the BIO + */ +void fr_tls_bio_dbuff_thread_local_clear(void) +{ + fr_tls_bio_dbuff_t *bd = tls_bio_talloc_agg; + + if (unlikely(!bd)) return; + if (unlikely(!bd->dbuff_in.buff)) return; + + fr_dbuff_free_talloc(&bd->dbuff_in); +} + +/** Frees the thread local TALLOC bio and its underlying OpenSSL BIO * + * + */ +static void _fr_tls_bio_dbuff_thread_local_free(void *bio_talloc_agg) +{ + fr_tls_bio_dbuff_t *our_bio_talloc_agg = talloc_get_type_abort(bio_talloc_agg, fr_tls_bio_dbuff_t); + + talloc_free(our_bio_talloc_agg); /* Frees the #fr_tls_bio_dbuff_t and BIO */ +} + +/** Return a BIO which will aggregate data in an expandable talloc buffer + * + * @note Only one of these BIOs may be in use at a given time. + * + * @param[in] init how much memory to allocate initially. + * @param[in] max the maximum amount of memory to allocate (0 for unlimited). + * @return A thread local BIO to pass to OpenSSL logging functions. */ -void fr_tls_bio_talloc_agg_clear(void) +BIO *fr_tls_bio_dbuff_thread_local(TALLOC_CTX *ctx, size_t init, size_t max) { - if (unlikely(!tls_bio_talloc_agg)) return; - if (unlikely(!tls_bio_talloc_agg->dbuff.buff)) return; + fr_tls_bio_dbuff_t *bd = tls_bio_talloc_agg; + + if (unlikely(!bd)) { + fr_tls_bio_dbuff_alloc(&bd, NULL, ctx, init, max, true); + fr_atexit_thread_local(tls_bio_talloc_agg, _fr_tls_bio_dbuff_thread_local_free, bd); + + return bd->bio; + } + + fr_assert_msg(!tls_bio_talloc_agg->dbuff_out.buff, "BIO not finialised"); + MEM(fr_dbuff_init_talloc(ctx, &bd->dbuff_out, &bd->tctx, init, max)); /* Where we read from */ + bd->dbuff_in = FR_DBUFF_BIND_END_ABS(&bd->dbuff_out); /* Where we write to */ - TALLOC_FREE(tls_bio_talloc_agg->dbuff.buff); + return tls_bio_talloc_agg->bio; } /** Initialise the BIO logging meths which are used to create thread local logging BIOs @@ -198,11 +372,13 @@ int fr_tls_bio_init(void) * The low byte here defines the BIO ID, and the high byte * defines its capabilities. */ - tls_bio_talloc_agg_meth = BIO_meth_new(BIO_get_new_index() | BIO_TYPE_SOURCE_SINK, "fr_tls_bio_talloc_agg"); - if (unlikely(!tls_bio_talloc_agg_meth)) return -1; + tls_bio_talloc_meth = BIO_meth_new(BIO_get_new_index() | BIO_TYPE_SOURCE_SINK, "fr_tls_bio_dbuff_t"); + if (unlikely(!tls_bio_talloc_meth)) return -1; - BIO_meth_set_write(tls_bio_talloc_agg_meth, tls_bio_talloc_agg_write_cb); - BIO_meth_set_puts(tls_bio_talloc_agg_meth, tls_bio_talloc_agg_puts_cb); + BIO_meth_set_write(tls_bio_talloc_meth, _tls_bio_talloc_write_cb); + BIO_meth_set_puts(tls_bio_talloc_meth, _tls_bio_talloc_puts_cb); + BIO_meth_set_read(tls_bio_talloc_meth, _tls_bio_talloc_read_cb); + BIO_meth_set_gets(tls_bio_talloc_meth, _tls_bio_talloc_gets_cb); return 0; } @@ -212,9 +388,9 @@ int fr_tls_bio_init(void) */ void fr_tls_bio_free(void) { - if (tls_bio_talloc_agg_meth) { - BIO_meth_free(tls_bio_talloc_agg_meth); - tls_bio_talloc_agg_meth = NULL; + if (tls_bio_talloc_meth) { + BIO_meth_free(tls_bio_talloc_meth); + tls_bio_talloc_meth = NULL; } } #endif /* WITH_TLS */ diff --git a/src/lib/tls/bio.h b/src/lib/tls/bio.h index 039ebda999e..d058609db07 100644 --- a/src/lib/tls/bio.h +++ b/src/lib/tls/bio.h @@ -28,12 +28,28 @@ RCSIDH(bio_h, "$Id$") #include #include -BIO *fr_tls_bio_talloc_agg(TALLOC_CTX *ctx, size_t init, size_t max); +typedef struct fr_tls_bio_dbuff_s fr_tls_bio_dbuff_t; -uint8_t *fr_tls_bio_talloc_agg_finalise(void); -char *fr_tls_bio_talloc_agg_finalise_bstr(void); +uint8_t *fr_tls_bio_dbuff_finalise(fr_tls_bio_dbuff_t *bd); -void fr_tls_bio_talloc_agg_clear(void); +char *fr_tls_bio_dbuff_finalise_bstr(fr_tls_bio_dbuff_t *bd); + +fr_dbuff_t *fr_tls_bio_dbuff_out(fr_tls_bio_dbuff_t *bd); + +fr_dbuff_t *fr_tls_bio_dbuff_in(fr_tls_bio_dbuff_t *bd); + +void fr_tls_bio_dbuff_reset(fr_tls_bio_dbuff_t *bd); + +BIO *fr_tls_bio_dbuff_alloc(fr_tls_bio_dbuff_t **out, TALLOC_CTX *bio_ctx, TALLOC_CTX *buff_ctx, + size_t init, size_t max, bool free_buff); + +uint8_t *fr_tls_bio_dbuff_thread_local_finalise(void); + +char *fr_tls_bio_dbuff_thread_local_finalise_bstr(void); + +void fr_tls_bio_dbuff_thread_local_clear(void); + +BIO *fr_tls_bio_dbuff_thread_local(TALLOC_CTX *ctx, size_t init, size_t max); int fr_tls_bio_init(void); diff --git a/src/lib/tls/pairs.c b/src/lib/tls/pairs.c index b2699b8abb6..05b0ef15693 100644 --- a/src/lib/tls/pairs.c +++ b/src/lib/tls/pairs.c @@ -71,15 +71,15 @@ int fr_tls_session_pairs_from_x509_cert(fr_pair_list_t *pair_list, TALLOC_CTX *c * Subject */ MEM(fr_pair_append_by_da(ctx, &vp, pair_list, attr_tls_cert_subject) == 0); - if (unlikely(X509_NAME_print_ex(fr_tls_bio_talloc_agg(vp, 256, 0), + if (unlikely(X509_NAME_print_ex(fr_tls_bio_dbuff_thread_local(vp, 256, 0), X509_get_subject_name(cert), 0, XN_FLAG_ONELINE) < 0)) { - fr_tls_bio_talloc_agg_clear(); + fr_tls_bio_dbuff_thread_local_clear(); fr_tls_log_error(request, "Failed retrieving certificate subject"); error: fr_pair_list_free(pair_list); return -1; } - fr_pair_value_bstrdup_buffer_shallow(vp, fr_tls_bio_talloc_agg_finalise_bstr(), true); + fr_pair_value_bstrdup_buffer_shallow(vp, fr_tls_bio_dbuff_thread_local_finalise_bstr(), true); RDEBUG3("Creating attributes for \"%pV\":", fr_box_strvalue_buffer(vp->vp_strvalue)); @@ -124,13 +124,13 @@ int fr_tls_session_pairs_from_x509_cert(fr_pair_list_t *pair_list, TALLOC_CTX *c * Issuer */ MEM(fr_pair_append_by_da(ctx, &vp, pair_list, attr_tls_cert_issuer) == 0); - if (unlikely(X509_NAME_print_ex(fr_tls_bio_talloc_agg(vp, 256, 0), + if (unlikely(X509_NAME_print_ex(fr_tls_bio_dbuff_thread_local(vp, 256, 0), X509_get_issuer_name(cert), 0, XN_FLAG_ONELINE) < 0)) { - fr_tls_bio_talloc_agg_clear(); + fr_tls_bio_dbuff_thread_local_clear(); fr_tls_log_error(request, "Failed retrieving certificate issuer"); goto error; } - fr_pair_value_bstrdup_buffer_shallow(vp, fr_tls_bio_talloc_agg_finalise_bstr(), true); + fr_pair_value_bstrdup_buffer_shallow(vp, fr_tls_bio_dbuff_thread_local_finalise_bstr(), true); /* * Serial number @@ -250,56 +250,76 @@ skip_alt: * For laziness, we re-use the OpenSSL names */ if (sk_X509_EXTENSION_num(ext_list) > 0) { - int i, len; - char *p; - BIO *out; + int i; + BIO *bio; + fr_tls_bio_dbuff_t *bd; + fr_dbuff_t *in, *out; - MEM(out = BIO_new(BIO_s_mem())); + bio = fr_tls_bio_dbuff_alloc(&bd, NULL, NULL, 257, 4097, true); + in = fr_tls_bio_dbuff_in(bd); + out = fr_tls_bio_dbuff_out(bd); for (i = 0; i < sk_X509_EXTENSION_num(ext_list); i++) { ASN1_OBJECT *obj; X509_EXTENSION *ext; fr_dict_attr_t const *da; + char *p; ext = sk_X509_EXTENSION_value(ext_list, i); obj = X509_EXTENSION_get_object(ext); - if (i2a_ASN1_OBJECT(out, obj) <= 0) { + if (i2a_ASN1_OBJECT(bio, obj) <= 0) { RPWDEBUG("Skipping X509 Extension (%i) conversion to attribute. " "Conversion from ASN1 failed...", i); + again: + fr_tls_bio_dbuff_reset(bd); continue; } - len = BIO_read(out, buff, sizeof(buff) - 1); - if (len <= 0) continue; - - buff[len] = '\0'; - - for (p = buff; *p != '\0'; p++) if (*p == ' ') *p = '-'; + if (fr_dbuff_remaining(out) == 0) goto again; /* Nothing written ? */ + + /* + * All disallowed chars get mashed to '-' + */ + for (p = (char *)fr_dbuff_current(out); + p < (char *)fr_dbuff_end(out); + p++) if (!fr_dict_attr_allowed_chars[(uint8_t)*p]) *p = '-'; + + /* + * Terminate the buffer (after char replacement, + * so we do don't replace the \0) + */ + if (unlikely(fr_dbuff_in_bytes(in, (uint8_t)'\0') <= 0)) { + RWDEBUG("Attribute name too long"); + goto again; + } - da = fr_dict_attr_by_name(NULL, attr_tls_cert, buff); + da = fr_dict_attr_by_name(NULL, attr_tls_cert, (char *)fr_dbuff_current(out)); if (!da) { - RWDEBUG3("Skipping attribute %s: " - "Add a dictionary definition if you want to access it", buff); - continue; + RWDEBUG3("Skipping attribute %pV: " + "Add a dictionary definition if you want to access it", + fr_box_strvalue_len((char *)fr_dbuff_current(out), + fr_dbuff_remaining(out))); + goto again; } - X509V3_EXT_print(out, ext, 0, 0); - len = BIO_read(out, buff, sizeof(buff) - 1); - if (len <= 0) continue; + fr_tls_bio_dbuff_reset(bd); /* 'free' any data used */ - buff[len] = '\0'; + X509V3_EXT_print(bio, ext, 0, 0); MEM(vp = fr_pair_afrom_da(ctx, da)); - if (fr_pair_value_from_str(vp, buff, len, '\0', true) < 0) { - RPWDEBUG3("Skipping: %s += '%s'", da->name, buff); + if (fr_pair_value_from_str(vp, (char *)fr_dbuff_current(out), fr_dbuff_remaining(out), + '\0', true) < 0) { + RPWDEBUG3("Skipping: %s += '%pV'", + da->name, fr_box_strvalue_len((char *)fr_dbuff_current(out), + fr_dbuff_remaining(out))); talloc_free(vp); - continue; + goto again; } fr_pair_append(pair_list, vp); } - BIO_free_all(out); + talloc_free(bd); } done: