Release memory earlier when clearing ASN1_ENCODING.
ASN1_ENCODING has a 'modified' bit, but every time it is set, the
contents are both ignored and never filled in again (we don't fill in
the encoding except on parse). That means keeping the underlying buffer
around is just wasting memory. Remove the bit and use the len != 0 to
determine if there's a saved encoding. Replace all the modified bits
with a helper function that drops the encoding.
I don't think we need a separate "present" boolean and can just treat
empty as not saved; a cached value always has a tag and length, so it
cannot be empty. (Even if it could be empty, that would imply the
value's encoding is trivial enough that we probably don't need the saved
encoding to preserve the value.)
Change-Id: I6beda94d33f3799daf85f1397818b9a41e7dd18a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55267
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h
index 4f98a1a..a934bfb 100644
--- a/crypto/asn1/internal.h
+++ b/crypto/asn1/internal.h
@@ -137,8 +137,7 @@
// problems with invalid encodings which can break signatures.
typedef struct ASN1_ENCODING_st {
unsigned char *enc; // DER encoding
- long len; // Length of encoding
- int modified; // set to 1 if 'enc' is invalid
+ long len; // Length of encoding, or zero if not present.
// alias_only is zero if |enc| owns the buffer that it points to
// (although |enc| may still be NULL). If one, |enc| points into a
// buffer that is owned elsewhere.
@@ -208,6 +207,9 @@
int asn1_enc_save(ASN1_VALUE **pval, const unsigned char *in, int inlen,
const ASN1_ITEM *it);
+// asn1_encoding_clear clears the cached encoding in |enc|.
+void asn1_encoding_clear(ASN1_ENCODING *enc);
+
// asn1_type_value_as_pointer returns |a|'s value in pointer form. This is
// usually the value object but, for BOOLEAN values, is 0 or 0xff cast to
// a pointer.
diff --git a/crypto/asn1/tasn_utl.c b/crypto/asn1/tasn_utl.c
index 4a97574..b7604e2 100644
--- a/crypto/asn1/tasn_utl.c
+++ b/crypto/asn1/tasn_utl.c
@@ -131,29 +131,19 @@
}
void asn1_enc_init(ASN1_VALUE **pval, const ASN1_ITEM *it) {
- ASN1_ENCODING *enc;
- enc = asn1_get_enc_ptr(pval, it);
+ ASN1_ENCODING *enc = asn1_get_enc_ptr(pval, it);
if (enc) {
enc->enc = NULL;
enc->len = 0;
enc->alias_only = 0;
enc->alias_only_on_next_parse = 0;
- enc->modified = 1;
}
}
void asn1_enc_free(ASN1_VALUE **pval, const ASN1_ITEM *it) {
- ASN1_ENCODING *enc;
- enc = asn1_get_enc_ptr(pval, it);
+ ASN1_ENCODING *enc = asn1_get_enc_ptr(pval, it);
if (enc) {
- if (!enc->alias_only) {
- OPENSSL_free(enc->enc);
- }
- enc->enc = NULL;
- enc->len = 0;
- enc->alias_only = 0;
- enc->alias_only_on_next_parse = 0;
- enc->modified = 1;
+ asn1_encoding_clear(enc);
}
}
@@ -183,16 +173,23 @@
}
enc->len = inlen;
- enc->modified = 0;
-
return 1;
}
+void asn1_encoding_clear(ASN1_ENCODING *enc) {
+ if (!enc->alias_only) {
+ OPENSSL_free(enc->enc);
+ }
+ enc->enc = NULL;
+ enc->len = 0;
+ enc->alias_only = 0;
+ enc->alias_only_on_next_parse = 0;
+}
+
int asn1_enc_restore(int *len, unsigned char **out, ASN1_VALUE **pval,
const ASN1_ITEM *it) {
- ASN1_ENCODING *enc;
- enc = asn1_get_enc_ptr(pval, it);
- if (!enc || enc->modified) {
+ ASN1_ENCODING *enc = asn1_get_enc_ptr(pval, it);
+ if (!enc || enc->len == 0) {
return 0;
}
if (out) {
diff --git a/crypto/x509/x509_req.c b/crypto/x509/x509_req.c
index 6cd250d..a384a77 100644
--- a/crypto/x509/x509_req.c
+++ b/crypto/x509/x509_req.c
@@ -65,6 +65,7 @@
#include <openssl/pem.h>
#include <openssl/x509.h>
+#include "../asn1/internal.h"
#include "internal.h"
@@ -238,6 +239,6 @@
}
int i2d_re_X509_REQ_tbs(X509_REQ *req, unsigned char **pp) {
- req->req_info->enc.modified = 1;
+ asn1_encoding_clear(&req->req_info->enc);
return i2d_X509_REQ_INFO(req->req_info, pp);
}
diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c
index 6eba6d1..6dd1fc0 100644
--- a/crypto/x509/x509cset.c
+++ b/crypto/x509/x509cset.c
@@ -59,6 +59,7 @@
#include <openssl/obj.h>
#include <openssl/x509.h>
+#include "../asn1/internal.h"
#include "../internal.h"
#include "internal.h"
@@ -132,7 +133,7 @@
int X509_CRL_sort(X509_CRL *c) {
// Sort the data so it will be written in serial number order.
sk_X509_REVOKED_sort(c->crl->revoked);
- c->crl->enc.modified = 1;
+ asn1_encoding_clear(&c->crl->enc);
return 1;
}
@@ -241,7 +242,7 @@
}
int i2d_re_X509_CRL_tbs(X509_CRL *crl, unsigned char **outp) {
- crl->crl->enc.modified = 1;
+ asn1_encoding_clear(&crl->crl->enc);
return i2d_X509_CRL_INFO(crl->crl, outp);
}
diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c
index 551b216..23508c0 100644
--- a/crypto/x509/x_all.c
+++ b/crypto/x509/x_all.c
@@ -66,6 +66,7 @@
#include <openssl/rsa.h>
#include <openssl/stack.h>
+#include "../asn1/internal.h"
#include "internal.h"
@@ -84,37 +85,37 @@
}
int X509_sign(X509 *x, EVP_PKEY *pkey, const EVP_MD *md) {
- x->cert_info->enc.modified = 1;
+ asn1_encoding_clear(&x->cert_info->enc);
return (ASN1_item_sign(ASN1_ITEM_rptr(X509_CINF), x->cert_info->signature,
x->sig_alg, x->signature, x->cert_info, pkey, md));
}
int X509_sign_ctx(X509 *x, EVP_MD_CTX *ctx) {
- x->cert_info->enc.modified = 1;
+ asn1_encoding_clear(&x->cert_info->enc);
return ASN1_item_sign_ctx(ASN1_ITEM_rptr(X509_CINF), x->cert_info->signature,
x->sig_alg, x->signature, x->cert_info, ctx);
}
int X509_REQ_sign(X509_REQ *x, EVP_PKEY *pkey, const EVP_MD *md) {
- x->req_info->enc.modified = 1;
+ asn1_encoding_clear(&x->req_info->enc);
return (ASN1_item_sign(ASN1_ITEM_rptr(X509_REQ_INFO), x->sig_alg, NULL,
x->signature, x->req_info, pkey, md));
}
int X509_REQ_sign_ctx(X509_REQ *x, EVP_MD_CTX *ctx) {
- x->req_info->enc.modified = 1;
+ asn1_encoding_clear(&x->req_info->enc);
return ASN1_item_sign_ctx(ASN1_ITEM_rptr(X509_REQ_INFO), x->sig_alg, NULL,
x->signature, x->req_info, ctx);
}
int X509_CRL_sign(X509_CRL *x, EVP_PKEY *pkey, const EVP_MD *md) {
- x->crl->enc.modified = 1;
+ asn1_encoding_clear(&x->crl->enc);
return (ASN1_item_sign(ASN1_ITEM_rptr(X509_CRL_INFO), x->crl->sig_alg,
x->sig_alg, x->signature, x->crl, pkey, md));
}
int X509_CRL_sign_ctx(X509_CRL *x, EVP_MD_CTX *ctx) {
- x->crl->enc.modified = 1;
+ asn1_encoding_clear(&x->crl->enc);
return ASN1_item_sign_ctx(ASN1_ITEM_rptr(X509_CRL_INFO), x->crl->sig_alg,
x->sig_alg, x->signature, x->crl, ctx);
}
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index c153b5d..1480448 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -67,6 +67,7 @@
#include <assert.h>
+#include "../asn1/internal.h"
#include "../internal.h"
#include "internal.h"
@@ -374,7 +375,7 @@
OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
return 0;
}
- inf->enc.modified = 1;
+ asn1_encoding_clear(&inf->enc);
return 1;
}
diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c
index 27cbd99..c1c1a19 100644
--- a/crypto/x509/x_x509.c
+++ b/crypto/x509/x_x509.c
@@ -68,6 +68,7 @@
#include <openssl/x509.h>
#include <openssl/x509v3.h>
+#include "../asn1/internal.h"
#include "../internal.h"
#include "internal.h"
@@ -329,7 +330,7 @@
}
int i2d_re_X509_tbs(X509 *x509, unsigned char **outp) {
- x509->cert_info->enc.modified = 1;
+ asn1_encoding_clear(&x509->cert_info->enc);
return i2d_X509_CINF(x509->cert_info, outp);
}
