From b0d7397aba8d173cc5fec8957a0ae8d53739aac0 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Wed, 7 Aug 2024 15:12:39 -0700 Subject: [PATCH 1/9] fixed BIO_get_mem_data documentation --- include/openssl/bio.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/openssl/bio.h b/include/openssl/bio.h index 794ce0961d..cca742e50c 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -445,9 +445,9 @@ OPENSSL_EXPORT int BIO_mem_contents(const BIO *bio, // and returns the length of the data. Despite being a macro, this function // should always take |char *| as a value and nothing else. // -// WARNING: don't use this, use |BIO_mem_contents|. A return value of zero from -// this function can mean either that it failed or that the memory buffer is -// empty. +// WARNING: don't use this, use |BIO_mem_contents|. A negative return value +// or zero from this function can mean either that it failed or that the +// memory buffer is empty. #define BIO_get_mem_data(bio, contents) BIO_ctrl(bio, BIO_CTRL_INFO, 0, \ (char *)(contents)) // BIO_get_mem_ptr sets |*out| to a BUF_MEM containing the current contents of From 6e8789c12e4a58f9c1cbac36a87db67dfdb0b779 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Wed, 7 Aug 2024 15:20:55 -0700 Subject: [PATCH 2/9] use BIO_mem_contents instead of BIO_get_mem_data --- crypto/ocsp/ocsp_http.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crypto/ocsp/ocsp_http.c b/crypto/ocsp/ocsp_http.c index 284b28cc01..e097a4e487 100644 --- a/crypto/ocsp/ocsp_http.c +++ b/crypto/ocsp/ocsp_http.c @@ -147,7 +147,7 @@ static int parse_http_line(char *line) { // read, |OCSP_REQ_CTX_nbio| will finish in the state of OHS_DONE. // |OCSP_REQ_CTX_nbio| will not return 1 until we reach OHS_DONE. int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { - int ret, data_len; + int ret, data_len, asn1_len; const unsigned char *data; next_io: if (!(rctx->state & OHS_NOREAD)) { @@ -176,7 +176,9 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { OPENSSL_FALLTHROUGH; case OHS_ASN1_WRITE_INIT: - rctx->asn1_len = BIO_get_mem_data(rctx->mem, NULL); + if(!BIO_mem_contents(rctx->mem, NULL, &rctx->asn1_len)) { + return 0; + } rctx->state = OHS_ASN1_WRITE; OPENSSL_FALLTHROUGH; From 5e0f2de5904768182463caa95279bb05ad7868fd Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Wed, 7 Aug 2024 15:49:06 -0700 Subject: [PATCH 3/9] change other uses of BIO_get_mem_data --- crypto/ocsp/ocsp_http.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/crypto/ocsp/ocsp_http.c b/crypto/ocsp/ocsp_http.c index e097a4e487..671d932098 100644 --- a/crypto/ocsp/ocsp_http.c +++ b/crypto/ocsp/ocsp_http.c @@ -147,7 +147,7 @@ static int parse_http_line(char *line) { // read, |OCSP_REQ_CTX_nbio| will finish in the state of OHS_DONE. // |OCSP_REQ_CTX_nbio| will not return 1 until we reach OHS_DONE. int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { - int ret, data_len, asn1_len; + int ret, data_len; const unsigned char *data; next_io: if (!(rctx->state & OHS_NOREAD)) { @@ -165,6 +165,7 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { } } + unsigned long tmp_data_len; switch (rctx->state) { case OHS_HTTP_HEADER: // Last operation was adding headers: need a final "\r\n". @@ -177,13 +178,18 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { OPENSSL_FALLTHROUGH; case OHS_ASN1_WRITE_INIT: if(!BIO_mem_contents(rctx->mem, NULL, &rctx->asn1_len)) { + rctx->state = OHS_ERROR; return 0; } rctx->state = OHS_ASN1_WRITE; OPENSSL_FALLTHROUGH; case OHS_ASN1_WRITE: - data_len = BIO_get_mem_data(rctx->mem, &data); + if(!BIO_mem_contents(rctx->mem, &data, &tmp_data_len)) { + rctx->state = OHS_ERROR; + return 0; + } + data_len = (int)tmp_data_len; int write_len = BIO_write(rctx->io, data + (data_len - rctx->asn1_len), (int)rctx->asn1_len); @@ -229,7 +235,11 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { // Due to strange memory BIO behaviour with BIO_gets we have to // check there's a complete line in there before calling BIO_gets // or we'll just get a partial read. - data_len = BIO_get_mem_data(rctx->mem, &data); + if(!BIO_mem_contents(rctx->mem, &data, &tmp_data_len)) { + rctx->state = OHS_ERROR; + return 0; + } + data_len = (int)tmp_data_len; if ((data_len <= 0) || !memchr(data, '\n', data_len)) { if (data_len >= rctx->iobuflen) { rctx->state = OHS_ERROR; @@ -281,7 +291,11 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { // Now reading ASN1 header: can read at least 2 bytes which is // enough for ASN1 SEQUENCE header and either length field or at // least the length of the length field. - data_len = BIO_get_mem_data(rctx->mem, &data); + if(!BIO_mem_contents(rctx->mem, &data, &tmp_data_len)) { + rctx->state = OHS_ERROR; + return 0; + } + data_len = (int)tmp_data_len; if (data_len < 2) { goto next_io; } @@ -326,7 +340,11 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { OPENSSL_FALLTHROUGH; case OHS_ASN1_CONTENT: - data_len = BIO_get_mem_data(rctx->mem, NULL); + if(!BIO_mem_contents(rctx->mem, NULL, &tmp_data_len)) { + rctx->state = OHS_ERROR; + return 0; + } + data_len = (int)tmp_data_len; if (data_len < (int)rctx->asn1_len) { goto next_io; } From 757dc6dfc7e00025354e1e1f03e456f4e15d25b3 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Wed, 7 Aug 2024 15:53:49 -0700 Subject: [PATCH 4/9] changed var type --- crypto/ocsp/ocsp_http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/ocsp/ocsp_http.c b/crypto/ocsp/ocsp_http.c index 671d932098..94426037fd 100644 --- a/crypto/ocsp/ocsp_http.c +++ b/crypto/ocsp/ocsp_http.c @@ -165,7 +165,7 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { } } - unsigned long tmp_data_len; + size_t tmp_data_len; switch (rctx->state) { case OHS_HTTP_HEADER: // Last operation was adding headers: need a final "\r\n". From dafe6f0a86303fd8c311bdcf2df6ab61f65ecb47 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Wed, 7 Aug 2024 15:58:00 -0700 Subject: [PATCH 5/9] change var type --- crypto/ocsp/ocsp_http.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crypto/ocsp/ocsp_http.c b/crypto/ocsp/ocsp_http.c index 94426037fd..792bb4eac8 100644 --- a/crypto/ocsp/ocsp_http.c +++ b/crypto/ocsp/ocsp_http.c @@ -177,10 +177,11 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { OPENSSL_FALLTHROUGH; case OHS_ASN1_WRITE_INIT: - if(!BIO_mem_contents(rctx->mem, NULL, &rctx->asn1_len)) { + if(!BIO_mem_contents(rctx->mem, NULL, &tmp_data_len)) { rctx->state = OHS_ERROR; return 0; } + rctx->asn1_len = (unsigned long)tmp_data_len; rctx->state = OHS_ASN1_WRITE; OPENSSL_FALLTHROUGH; From 65b0a14187b8423b1ac23dab0f23e8e1e6eb422f Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Wed, 7 Aug 2024 16:11:24 -0700 Subject: [PATCH 6/9] added null check for out to BIO_mem_contents --- crypto/bio/bio_mem.c | 4 +++- include/openssl/bio.h | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/crypto/bio/bio_mem.c b/crypto/bio/bio_mem.c index f4c01d1dff..8256277332 100644 --- a/crypto/bio/bio_mem.c +++ b/crypto/bio/bio_mem.c @@ -287,7 +287,9 @@ int BIO_mem_contents(const BIO *bio, const uint8_t **out_contents, } b = (BUF_MEM *)bio->ptr; - *out_contents = (uint8_t *)b->data; + if (out_contents != NULL) { + *out_contents = (uint8_t *)b->data; + } *out_len = b->length; return 1; } diff --git a/include/openssl/bio.h b/include/openssl/bio.h index cca742e50c..712c53f4f8 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -434,9 +434,9 @@ OPENSSL_EXPORT const BIO_METHOD *BIO_s_mem(void); // don't depend on this in new code. OPENSSL_EXPORT BIO *BIO_new_mem_buf(const void *buf, ossl_ssize_t len); -// BIO_mem_contents sets |*out_contents| to point to the current contents of -// |bio| and |*out_len| to contain the length of that data. It returns one on -// success and zero otherwise. +// BIO_mem_contents sets |*out_contents|, if not null, to point to the current +// contents of|bio| and |*out_len| to contain the length of that data. +// It returns one on success and zero otherwise. OPENSSL_EXPORT int BIO_mem_contents(const BIO *bio, const uint8_t **out_contents, size_t *out_len); From 6050263581c61da2f6db4fbbdd0237d17f51fa92 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Fri, 16 Aug 2024 14:39:38 -0700 Subject: [PATCH 7/9] refactored var from size_t to int, added checks to BIO_mem_contents and updated documentation --- crypto/bio/bio_mem.c | 6 ++++-- crypto/ocsp/ocsp_http.c | 43 ++++++++++++++++++++--------------------- include/openssl/bio.h | 4 +++- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/crypto/bio/bio_mem.c b/crypto/bio/bio_mem.c index 8256277332..54243f9df4 100644 --- a/crypto/bio/bio_mem.c +++ b/crypto/bio/bio_mem.c @@ -282,7 +282,7 @@ const BIO_METHOD *BIO_s_mem(void) { return &mem_method; } int BIO_mem_contents(const BIO *bio, const uint8_t **out_contents, size_t *out_len) { const BUF_MEM *b; - if (bio->method != &mem_method) { + if (!bio || bio->method != &mem_method) { return 0; } @@ -290,7 +290,9 @@ int BIO_mem_contents(const BIO *bio, const uint8_t **out_contents, if (out_contents != NULL) { *out_contents = (uint8_t *)b->data; } - *out_len = b->length; + if(out_len) { + *out_len = b->length; + } return 1; } diff --git a/crypto/ocsp/ocsp_http.c b/crypto/ocsp/ocsp_http.c index 792bb4eac8..179ec7d6af 100644 --- a/crypto/ocsp/ocsp_http.c +++ b/crypto/ocsp/ocsp_http.c @@ -147,12 +147,13 @@ static int parse_http_line(char *line) { // read, |OCSP_REQ_CTX_nbio| will finish in the state of OHS_DONE. // |OCSP_REQ_CTX_nbio| will not return 1 until we reach OHS_DONE. int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { - int ret, data_len; + int ret, tmp_data_len; + size_t data_len; const unsigned char *data; -next_io: + next_io: if (!(rctx->state & OHS_NOREAD)) { - data_len = BIO_read(rctx->io, rctx->iobuf, rctx->iobuflen); - if (data_len <= 0) { + tmp_data_len = BIO_read(rctx->io, rctx->iobuf, rctx->iobuflen); + if (tmp_data_len <= 0) { if (BIO_should_retry(rctx->io)) { return -1; } @@ -160,12 +161,12 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { } // Write data to memory BIO. - if (BIO_write(rctx->mem, rctx->iobuf, data_len) != data_len) { + if (BIO_write(rctx->mem, rctx->iobuf, tmp_data_len) != tmp_data_len) { return 0; } + data_len = (size_t)tmp_data_len; } - size_t tmp_data_len; switch (rctx->state) { case OHS_HTTP_HEADER: // Last operation was adding headers: need a final "\r\n". @@ -177,20 +178,19 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { OPENSSL_FALLTHROUGH; case OHS_ASN1_WRITE_INIT: - if(!BIO_mem_contents(rctx->mem, NULL, &tmp_data_len)) { + if(!BIO_mem_contents(rctx->mem, NULL, &data_len)) { rctx->state = OHS_ERROR; return 0; } - rctx->asn1_len = (unsigned long)tmp_data_len; + rctx->asn1_len = data_len; rctx->state = OHS_ASN1_WRITE; OPENSSL_FALLTHROUGH; case OHS_ASN1_WRITE: - if(!BIO_mem_contents(rctx->mem, &data, &tmp_data_len)) { + if(!BIO_mem_contents(rctx->mem, &data, &data_len)) { rctx->state = OHS_ERROR; return 0; } - data_len = (int)tmp_data_len; int write_len = BIO_write(rctx->io, data + (data_len - rctx->asn1_len), (int)rctx->asn1_len); @@ -236,21 +236,20 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { // Due to strange memory BIO behaviour with BIO_gets we have to // check there's a complete line in there before calling BIO_gets // or we'll just get a partial read. - if(!BIO_mem_contents(rctx->mem, &data, &tmp_data_len)) { + if(!BIO_mem_contents(rctx->mem, &data, &data_len)) { rctx->state = OHS_ERROR; return 0; } - data_len = (int)tmp_data_len; if ((data_len <= 0) || !memchr(data, '\n', data_len)) { - if (data_len >= rctx->iobuflen) { + if (data_len >= (size_t)rctx->iobuflen) { rctx->state = OHS_ERROR; return 0; } goto next_io; } - data_len = BIO_gets(rctx->mem, (char *)rctx->iobuf, rctx->iobuflen); + tmp_data_len = BIO_gets(rctx->mem, (char *)rctx->iobuf, rctx->iobuflen); - if (data_len <= 0) { + if (tmp_data_len <= 0) { if (BIO_should_retry(rctx->mem)) { goto next_io; } @@ -259,10 +258,11 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { } // Don't allow excessive lines. - if (data_len >= rctx->iobuflen) { + if (tmp_data_len >= rctx->iobuflen) { rctx->state = OHS_ERROR; return 0; } + data_len = (size_t)tmp_data_len; // First line. if (rctx->state == OHS_FIRSTLINE) { @@ -292,11 +292,10 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { // Now reading ASN1 header: can read at least 2 bytes which is // enough for ASN1 SEQUENCE header and either length field or at // least the length of the length field. - if(!BIO_mem_contents(rctx->mem, &data, &tmp_data_len)) { + if(!BIO_mem_contents(rctx->mem, &data, &data_len)) { rctx->state = OHS_ERROR; return 0; } - data_len = (int)tmp_data_len; if (data_len < 2) { goto next_io; } @@ -325,7 +324,7 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { } data++; rctx->asn1_len = 0; - for (int i = 0; i < data_len; i++) { + for (int i = 0; i < (int)data_len; i++) { rctx->asn1_len <<= 8; rctx->asn1_len |= *data++; } @@ -341,12 +340,11 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { OPENSSL_FALLTHROUGH; case OHS_ASN1_CONTENT: - if(!BIO_mem_contents(rctx->mem, NULL, &tmp_data_len)) { + if(!BIO_mem_contents(rctx->mem, NULL, &data_len)) { rctx->state = OHS_ERROR; return 0; } - data_len = (int)tmp_data_len; - if (data_len < (int)rctx->asn1_len) { + if (data_len < rctx->asn1_len) { goto next_io; } rctx->state = OHS_DONE; @@ -359,6 +357,7 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { return 0; } + int OCSP_sendreq_nbio(OCSP_RESPONSE **presp, OCSP_REQ_CTX *rctx) { return OCSP_REQ_CTX_nbio_d2i(rctx, (ASN1_VALUE **)presp, ASN1_ITEM_rptr(OCSP_RESPONSE)); diff --git a/include/openssl/bio.h b/include/openssl/bio.h index 712c53f4f8..96f59d6b9c 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -435,7 +435,9 @@ OPENSSL_EXPORT const BIO_METHOD *BIO_s_mem(void); OPENSSL_EXPORT BIO *BIO_new_mem_buf(const void *buf, ossl_ssize_t len); // BIO_mem_contents sets |*out_contents|, if not null, to point to the current -// contents of|bio| and |*out_len| to contain the length of that data. +// contents of|bio| and |*out_len|, if not null, to contain the length of +// that data. +// // It returns one on success and zero otherwise. OPENSSL_EXPORT int BIO_mem_contents(const BIO *bio, const uint8_t **out_contents, From 5aeadee5e721bcf788b9c485a8b7728c72c02c44 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Fri, 16 Aug 2024 14:41:35 -0700 Subject: [PATCH 8/9] fixed indent --- crypto/ocsp/ocsp_http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/ocsp/ocsp_http.c b/crypto/ocsp/ocsp_http.c index 179ec7d6af..426f59f34b 100644 --- a/crypto/ocsp/ocsp_http.c +++ b/crypto/ocsp/ocsp_http.c @@ -150,7 +150,7 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { int ret, tmp_data_len; size_t data_len; const unsigned char *data; - next_io: +next_io: if (!(rctx->state & OHS_NOREAD)) { tmp_data_len = BIO_read(rctx->io, rctx->iobuf, rctx->iobuflen); if (tmp_data_len <= 0) { From fe740cf9dce4c4e83cc8c8a12bc13f0b4580edf9 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Mon, 19 Aug 2024 20:11:55 -0700 Subject: [PATCH 9/9] changed var type in for loop --- crypto/ocsp/ocsp_http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/ocsp/ocsp_http.c b/crypto/ocsp/ocsp_http.c index 426f59f34b..5c43c53b1b 100644 --- a/crypto/ocsp/ocsp_http.c +++ b/crypto/ocsp/ocsp_http.c @@ -324,7 +324,7 @@ int OCSP_REQ_CTX_nbio(OCSP_REQ_CTX *rctx) { } data++; rctx->asn1_len = 0; - for (int i = 0; i < (int)data_len; i++) { + for (size_t i = 0; i < data_len; i++) { rctx->asn1_len <<= 8; rctx->asn1_len |= *data++; }