Mirror of BoringSSL (grpc依赖) https://boringssl.googlesource.com/boringssl
You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

695 lines
23 KiB

/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
* This package is an SSL implementation written
* by Eric Young (eay@cryptsoft.com).
* The implementation was written so as to conform with Netscapes SSL.
*
* This library is free for commercial and non-commercial use as long as
* the following conditions are aheared to. The following conditions
* apply to all code found in this distribution, be it the RC4, RSA,
* lhash, DES, etc., code; not just the SSL code. The SSL documentation
* included with this distribution is covered by the same copyright terms
* except that the holder is Tim Hudson (tjh@cryptsoft.com).
*
* Copyright remains Eric Young's, and as such any Copyright notices in
* the code are not to be removed.
* If this package is used in a product, Eric Young should be given attribution
* as the author of the parts of the library used.
* This can be in the form of a textual message at program startup or
* in documentation (online or textual) provided with the package.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* 3. All advertising materials mentioning features or use of this software
* must display the following acknowledgement:
* "This product includes cryptographic software written by
* Eric Young (eay@cryptsoft.com)"
* The word 'cryptographic' can be left out if the rouines from the library
* being used are not cryptographic related :-).
* 4. If you include any Windows specific code (or a derivative thereof) from
* the apps directory (application code) you must include an acknowledgement:
* "This product includes software written by Tim Hudson (tjh@cryptsoft.com)"
*
* THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
* The licence and distribution terms for any publically available version or
* derivative of this code cannot be changed. i.e. this code cannot simply be
* copied and put under another distribution licence
* [including the GNU Public Licence.] */
#include <openssl/asn1.h>
#include <assert.h>
#include <limits.h>
#include <string.h>
#include <openssl/asn1t.h>
#include <openssl/mem.h>
#include "../internal.h"
#include "internal.h"
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
static int asn1_item_ex_i2d_opt(ASN1_VALUE **pval, unsigned char **out,
const ASN1_ITEM *it, int tag, int aclass,
int optional);
static int asn1_i2d_ex_primitive(ASN1_VALUE **pval, unsigned char **out,
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
const ASN1_ITEM *it, int tag, int aclass,
int optional);
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
static int asn1_ex_i2c(ASN1_VALUE **pval, unsigned char *cont, int *out_omit,
int *putype, const ASN1_ITEM *it);
static int asn1_set_seq_out(STACK_OF(ASN1_VALUE) *sk, unsigned char **out,
int skcontlen, const ASN1_ITEM *item, int do_sort);
static int asn1_template_ex_i2d(ASN1_VALUE **pval, unsigned char **out,
const ASN1_TEMPLATE *tt, int tag, int aclass);
/*
* Top level i2d equivalents
*/
int ASN1_item_i2d(ASN1_VALUE *val, unsigned char **out, const ASN1_ITEM *it) {
if (out && !*out) {
unsigned char *p, *buf;
int len = ASN1_item_ex_i2d(&val, NULL, it, /*tag=*/-1, /*aclass=*/0);
if (len <= 0) {
return len;
}
buf = OPENSSL_malloc(len);
if (!buf) {
OPENSSL_PUT_ERROR(ASN1, ERR_R_MALLOC_FAILURE);
return -1;
}
p = buf;
int len2 = ASN1_item_ex_i2d(&val, &p, it, /*tag=*/-1, /*aclass=*/0);
if (len2 <= 0) {
return len2;
}
assert(len == len2);
*out = buf;
return len;
}
return ASN1_item_ex_i2d(&val, out, it, /*tag=*/-1, /*aclass=*/0);
}
/*
* Encode an item, taking care of IMPLICIT tagging (if any). This function
* performs the normal item handling: it can be used in external types.
*/
int ASN1_item_ex_i2d(ASN1_VALUE **pval, unsigned char **out,
const ASN1_ITEM *it, int tag, int aclass) {
int ret = asn1_item_ex_i2d_opt(pval, out, it, tag, aclass, /*optional=*/0);
assert(ret != 0);
return ret;
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
}
/* asn1_item_ex_i2d_opt behaves like |ASN1_item_ex_i2d| but, if |optional| is
* non-zero and |*pval| is omitted, it returns zero and writes no bytes. */
int asn1_item_ex_i2d_opt(ASN1_VALUE **pval, unsigned char **out,
const ASN1_ITEM *it, int tag, int aclass,
int optional) {
const ASN1_TEMPLATE *tt = NULL;
int i, seqcontlen, seqlen;
/* Historically, |aclass| was repurposed to pass additional flags into the
* encoding process. */
assert((aclass & ASN1_TFLG_TAG_CLASS) == aclass);
/* If not overridding the tag, |aclass| is ignored and should be zero. */
assert(tag != -1 || aclass == 0);
/* All fields are pointers, except for boolean |ASN1_ITYPE_PRIMITIVE|s.
* Optional primitives are handled later. */
if ((it->itype != ASN1_ITYPE_PRIMITIVE) && !*pval) {
if (optional) {
return 0;
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
}
OPENSSL_PUT_ERROR(ASN1, ASN1_R_MISSING_VALUE);
return -1;
}
switch (it->itype) {
case ASN1_ITYPE_PRIMITIVE:
if (it->templates) {
if (it->templates->flags & ASN1_TFLG_OPTIONAL) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
return -1;
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
}
return asn1_template_ex_i2d(pval, out, it->templates, tag, aclass);
}
return asn1_i2d_ex_primitive(pval, out, it, tag, aclass, optional);
case ASN1_ITYPE_MSTRING:
/*
* It never makes sense for multi-strings to have implicit tagging, so
* if tag != -1, then this looks like an error in the template.
*/
if (tag != -1) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
return -1;
}
return asn1_i2d_ex_primitive(pval, out, it, -1, 0, optional);
case ASN1_ITYPE_CHOICE: {
/*
* It never makes sense for CHOICE types to have implicit tagging, so if
* tag != -1, then this looks like an error in the template.
*/
if (tag != -1) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
return -1;
}
i = asn1_get_choice_selector(pval, it);
if (i < 0 || i >= it->tcount) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_NO_MATCHING_CHOICE_TYPE);
return -1;
}
const ASN1_TEMPLATE *chtt = it->templates + i;
if (chtt->flags & ASN1_TFLG_OPTIONAL) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
return -1;
}
ASN1_VALUE **pchval = asn1_get_field_ptr(pval, chtt);
return asn1_template_ex_i2d(pchval, out, chtt, -1, 0);
}
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
case ASN1_ITYPE_EXTERN: {
/* If new style i2d it does all the work */
const ASN1_EXTERN_FUNCS *ef = it->funcs;
int ret = ef->asn1_ex_i2d(pval, out, it, tag, aclass);
if (ret == 0) {
/* |asn1_ex_i2d| should never return zero. We have already checked
* for optional values generically, and |ASN1_ITYPE_EXTERN| fields
* must be pointers. */
OPENSSL_PUT_ERROR(ASN1, ERR_R_INTERNAL_ERROR);
return -1;
}
return ret;
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
}
case ASN1_ITYPE_SEQUENCE: {
i = asn1_enc_restore(&seqcontlen, out, pval, it);
/* An error occurred */
if (i < 0)
return -1;
/* We have a valid cached encoding... */
if (i > 0)
return seqcontlen;
/* Otherwise carry on */
seqcontlen = 0;
/* If no IMPLICIT tagging set to SEQUENCE, UNIVERSAL */
if (tag == -1) {
tag = V_ASN1_SEQUENCE;
aclass = V_ASN1_UNIVERSAL;
}
/* First work out sequence content length */
for (i = 0, tt = it->templates; i < it->tcount; tt++, i++) {
const ASN1_TEMPLATE *seqtt;
ASN1_VALUE **pseqval;
int tmplen;
seqtt = asn1_do_adb(pval, tt, 1);
if (!seqtt)
return -1;
pseqval = asn1_get_field_ptr(pval, seqtt);
tmplen = asn1_template_ex_i2d(pseqval, NULL, seqtt, -1, 0);
if (tmplen == -1 || (tmplen > INT_MAX - seqcontlen))
return -1;
seqcontlen += tmplen;
}
seqlen = ASN1_object_size(/*constructed=*/1, seqcontlen, tag);
if (!out || seqlen == -1)
return seqlen;
/* Output SEQUENCE header */
ASN1_put_object(out, /*constructed=*/1, seqcontlen, tag, aclass);
for (i = 0, tt = it->templates; i < it->tcount; tt++, i++) {
const ASN1_TEMPLATE *seqtt;
ASN1_VALUE **pseqval;
seqtt = asn1_do_adb(pval, tt, 1);
if (!seqtt)
return -1;
pseqval = asn1_get_field_ptr(pval, seqtt);
if (asn1_template_ex_i2d(pseqval, out, seqtt, -1, 0) < 0) {
return -1;
}
}
return seqlen;
}
default:
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
return -1;
}
}
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
/* asn1_template_ex_i2d behaves like |asn1_item_ex_i2d_opt| but uses an
* |ASN1_TEMPLATE| instead of an |ASN1_ITEM|. An |ASN1_TEMPLATE| wraps an
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
* |ASN1_ITEM| with modifiers such as tagging, SEQUENCE or SET, etc. Instead of
* taking an |optional| parameter, it uses the |ASN1_TFLG_OPTIONAL| flag. */
static int asn1_template_ex_i2d(ASN1_VALUE **pval, unsigned char **out,
const ASN1_TEMPLATE *tt, int tag, int iclass) {
int i, ret, flags, ttag, tclass;
size_t j;
flags = tt->flags;
/* Historically, |iclass| was repurposed to pass additional flags into the
* encoding process. */
assert((iclass & ASN1_TFLG_TAG_CLASS) == iclass);
/* If not overridding the tag, |iclass| is ignored and should be zero. */
assert(tag != -1 || iclass == 0);
/*
* Work out tag and class to use: tagging may come either from the
* template or the arguments, not both because this would create
* ambiguity.
*/
if (flags & ASN1_TFLG_TAG_MASK) {
/* Error if argument and template tagging */
if (tag != -1) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
return -1;
}
/* Get tagging from template */
ttag = tt->tag;
tclass = flags & ASN1_TFLG_TAG_CLASS;
} else if (tag != -1) {
/* No template tagging, get from arguments */
ttag = tag;
tclass = iclass & ASN1_TFLG_TAG_CLASS;
} else {
ttag = -1;
tclass = 0;
}
const int optional = (flags & ASN1_TFLG_OPTIONAL) != 0;
/*
* At this point 'ttag' contains the outer tag to use, and 'tclass' is the
* class.
*/
if (flags & ASN1_TFLG_SK_MASK) {
/* SET OF, SEQUENCE OF */
STACK_OF(ASN1_VALUE) *sk = (STACK_OF(ASN1_VALUE) *)*pval;
int isset, sktag, skaclass;
int skcontlen, sklen;
ASN1_VALUE *skitem;
if (!*pval) {
if (optional) {
return 0;
}
OPENSSL_PUT_ERROR(ASN1, ASN1_R_MISSING_VALUE);
return -1;
}
if (flags & ASN1_TFLG_SET_OF) {
isset = 1;
/* Historically, types with both bits set were mutated when
* serialized to apply the sort. We no longer support this. */
assert((flags & ASN1_TFLG_SEQUENCE_OF) == 0);
} else {
isset = 0;
}
/*
* Work out inner tag value: if EXPLICIT or no tagging use underlying
* type.
*/
if ((ttag != -1) && !(flags & ASN1_TFLG_EXPTAG)) {
sktag = ttag;
skaclass = tclass;
} else {
skaclass = V_ASN1_UNIVERSAL;
if (isset)
sktag = V_ASN1_SET;
else
sktag = V_ASN1_SEQUENCE;
}
/* Determine total length of items */
skcontlen = 0;
for (j = 0; j < sk_ASN1_VALUE_num(sk); j++) {
int tmplen;
skitem = sk_ASN1_VALUE_value(sk, j);
tmplen = ASN1_item_ex_i2d(&skitem, NULL, ASN1_ITEM_ptr(tt->item), -1, 0);
if (tmplen == -1 || (skcontlen > INT_MAX - tmplen))
return -1;
skcontlen += tmplen;
}
sklen = ASN1_object_size(/*constructed=*/1, skcontlen, sktag);
if (sklen == -1)
return -1;
/* If EXPLICIT need length of surrounding tag */
if (flags & ASN1_TFLG_EXPTAG)
ret = ASN1_object_size(/*constructed=*/1, sklen, ttag);
else
ret = sklen;
if (!out || ret == -1)
return ret;
/* Now encode this lot... */
/* EXPLICIT tag */
if (flags & ASN1_TFLG_EXPTAG)
ASN1_put_object(out, /*constructed=*/1, sklen, ttag, tclass);
/* SET or SEQUENCE and IMPLICIT tag */
ASN1_put_object(out, /*constructed=*/1, skcontlen, sktag, skaclass);
/* And the stuff itself */
if (!asn1_set_seq_out(sk, out, skcontlen, ASN1_ITEM_ptr(tt->item), isset)) {
return -1;
}
return ret;
}
if (flags & ASN1_TFLG_EXPTAG) {
/* EXPLICIT tagging */
/* Find length of tagged item */
i = asn1_item_ex_i2d_opt(pval, NULL, ASN1_ITEM_ptr(tt->item), -1, 0,
optional);
if (i <= 0)
return i;
/* Find length of EXPLICIT tag */
ret = ASN1_object_size(/*constructed=*/1, i, ttag);
if (out && ret != -1) {
/* Output tag and item */
ASN1_put_object(out, /*constructed=*/1, i, ttag, tclass);
if (ASN1_item_ex_i2d(pval, out, ASN1_ITEM_ptr(tt->item), -1, 0) < 0) {
return -1;
}
}
return ret;
}
/* Either normal or IMPLICIT tagging */
return asn1_item_ex_i2d_opt(pval, out, ASN1_ITEM_ptr(tt->item), ttag, tclass,
optional);
}
/* Temporary structure used to hold DER encoding of items for SET OF */
typedef struct {
unsigned char *data;
int length;
} DER_ENC;
static int der_cmp(const void *a, const void *b) {
const DER_ENC *d1 = a, *d2 = b;
int cmplen, i;
cmplen = (d1->length < d2->length) ? d1->length : d2->length;
i = OPENSSL_memcmp(d1->data, d2->data, cmplen);
if (i)
return i;
return d1->length - d2->length;
}
/* asn1_set_seq_out writes |sk| to |out| under the i2d output convention,
* excluding the tag and length. It returns one on success and zero on error.
* |skcontlen| must be the total encoded size. If |do_sort| is non-zero, the
* elements are sorted for a SET OF type. Each element of |sk| has type
* |item|. */
static int asn1_set_seq_out(STACK_OF(ASN1_VALUE) *sk, unsigned char **out,
int skcontlen, const ASN1_ITEM *item, int do_sort) {
/* No need to sort if there are fewer than two items. */
if (!do_sort || sk_ASN1_VALUE_num(sk) < 2) {
for (size_t i = 0; i < sk_ASN1_VALUE_num(sk); i++) {
ASN1_VALUE *skitem = sk_ASN1_VALUE_value(sk, i);
if (ASN1_item_ex_i2d(&skitem, out, item, -1, 0) < 0) {
return 0;
}
}
return 1;
}
if (sk_ASN1_VALUE_num(sk) > ((size_t)-1) / sizeof(DER_ENC)) {
OPENSSL_PUT_ERROR(ASN1, ERR_R_OVERFLOW);
return 0;
}
int ret = 0;
unsigned char *const buf = OPENSSL_malloc(skcontlen);
DER_ENC *encoded = OPENSSL_malloc(sk_ASN1_VALUE_num(sk) * sizeof(*encoded));
if (encoded == NULL || buf == NULL) {
OPENSSL_PUT_ERROR(ASN1, ERR_R_MALLOC_FAILURE);
goto err;
}
/* Encode all the elements into |buf| and populate |encoded|. */
unsigned char *p = buf;
for (size_t i = 0; i < sk_ASN1_VALUE_num(sk); i++) {
ASN1_VALUE *skitem = sk_ASN1_VALUE_value(sk, i);
encoded[i].data = p;
encoded[i].length = ASN1_item_ex_i2d(&skitem, &p, item, -1, 0);
if (encoded[i].length < 0) {
goto err;
}
assert(p - buf <= skcontlen);
}
qsort(encoded, sk_ASN1_VALUE_num(sk), sizeof(*encoded), der_cmp);
/* Output the elements in sorted order. */
p = *out;
for (size_t i = 0; i < sk_ASN1_VALUE_num(sk); i++) {
OPENSSL_memcpy(p, encoded[i].data, encoded[i].length);
p += encoded[i].length;
}
*out = p;
ret = 1;
err:
OPENSSL_free(encoded);
OPENSSL_free(buf);
return ret;
}
/* asn1_i2d_ex_primitive behaves like |ASN1_item_ex_i2d| but |item| must be a
* a PRIMITIVE or MSTRING type that is not an |ASN1_ITEM_TEMPLATE|. */
static int asn1_i2d_ex_primitive(ASN1_VALUE **pval, unsigned char **out,
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
const ASN1_ITEM *it, int tag, int aclass,
int optional) {
/* Get length of content octets and maybe find out the underlying type. */
int omit;
int utype = it->utype;
int len = asn1_ex_i2c(pval, NULL, &omit, &utype, it);
if (len < 0) {
return -1;
}
if (omit) {
if (optional) {
return 0;
}
OPENSSL_PUT_ERROR(ASN1, ASN1_R_MISSING_VALUE);
return -1;
}
/*
* If SEQUENCE, SET or OTHER then header is included in pseudo content
* octets so don't include tag+length. We need to check here because the
* call to asn1_ex_i2c() could change utype.
*/
int usetag =
utype != V_ASN1_SEQUENCE && utype != V_ASN1_SET && utype != V_ASN1_OTHER;
/* If not implicitly tagged get tag from underlying type */
if (tag == -1)
tag = utype;
/* Output tag+length followed by content octets */
if (out) {
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
if (usetag) {
ASN1_put_object(out, /*constructed=*/0, len, tag, aclass);
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
}
int len2 = asn1_ex_i2c(pval, *out, &omit, &utype, it);
if (len2 < 0) {
return -1;
}
assert(len == len2);
assert(!omit);
*out += len;
}
if (usetag) {
return ASN1_object_size(/*constructed=*/0, len, tag);
}
return len;
}
/* asn1_ex_i2c writes the |*pval| to |cout| under the i2d output convention,
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
* excluding the tag and length. It returns the number of bytes written,
* possibly zero, on success or -1 on error. If |*pval| should be omitted, it
* returns zero and sets |*out_omit| to true.
*
* If |it| is an MSTRING or ANY type, it gets the underlying type from |*pval|,
* which must be an |ASN1_STRING| or |ASN1_TYPE|, respectively. It then updates
* |*putype| with the tag number of type used, or |V_ASN1_OTHER| if it was not a
* universal type. If |*putype| is set to |V_ASN1_SEQUENCE|, |V_ASN1_SET|, or
* |V_ASN1_OTHER|, it additionally outputs the tag and length, so the caller
* must not do so.
*
* Otherwise, |*putype| must contain |it->utype|.
*
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
* WARNING: Unlike most functions in this file, |asn1_ex_i2c| can return zero
* without omitting the element. ASN.1 values may have empty contents. */
static int asn1_ex_i2c(ASN1_VALUE **pval, unsigned char *cout, int *out_omit,
int *putype, const ASN1_ITEM *it) {
ASN1_BOOLEAN *tbool = NULL;
ASN1_STRING *strtmp;
ASN1_OBJECT *otmp;
int utype;
const unsigned char *cont;
unsigned char c;
int len;
/* Historically, |it->funcs| for primitive types contained an
* |ASN1_PRIMITIVE_FUNCS| table of callbacks. */
assert(it->funcs == NULL);
*out_omit = 0;
/* Should type be omitted? */
if ((it->itype != ASN1_ITYPE_PRIMITIVE) || (it->utype != V_ASN1_BOOLEAN)) {
if (!*pval) {
*out_omit = 1;
return 0;
}
}
if (it->itype == ASN1_ITYPE_MSTRING) {
/* If MSTRING type set the underlying type */
strtmp = (ASN1_STRING *)*pval;
utype = strtmp->type;
if (utype < 0 && utype != V_ASN1_OTHER) {
/* MSTRINGs can have type -1 when default-constructed. */
OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
return -1;
}
/* Negative INTEGER and ENUMERATED values use |ASN1_STRING| type values
* that do not match their corresponding utype values. INTEGERs cannot
* participate in MSTRING types, but ENUMERATEDs can.
*
* TODO(davidben): Is this a bug? Although arguably one of the MSTRING
* types should contain more values, rather than less. See
* https://crbug.com/boringssl/412. But it is not possible to fit all
* possible ANY values into an |ASN1_STRING|, so matching the spec here
* is somewhat hopeless. */
if (utype == V_ASN1_NEG_INTEGER) {
utype = V_ASN1_INTEGER;
} else if (utype == V_ASN1_NEG_ENUMERATED) {
utype = V_ASN1_ENUMERATED;
}
*putype = utype;
} else if (it->utype == V_ASN1_ANY) {
/* If ANY set type and pointer to value */
ASN1_TYPE *typ;
typ = (ASN1_TYPE *)*pval;
utype = typ->type;
if (utype < 0 && utype != V_ASN1_OTHER) {
/* |ASN1_TYPE|s can have type -1 when default-constructed. */
OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
return -1;
}
*putype = utype;
pval = &typ->value.asn1_value;
} else
utype = *putype;
switch (utype) {
case V_ASN1_OBJECT:
otmp = (ASN1_OBJECT *)*pval;
cont = otmp->data;
len = otmp->length;
if (len == 0) {
/* Some |ASN1_OBJECT|s do not have OIDs and cannot be serialized. */
OPENSSL_PUT_ERROR(ASN1, ASN1_R_ILLEGAL_OBJECT);
return -1;
}
break;
case V_ASN1_NULL:
cont = NULL;
len = 0;
break;
case V_ASN1_BOOLEAN:
tbool = (ASN1_BOOLEAN *)pval;
if (*tbool == -1) {
*out_omit = 1;
return 0;
}
if (it->utype != V_ASN1_ANY) {
/*
* Default handling if value == size field then omit
*/
if ((*tbool && (it->size > 0)) || (!*tbool && !it->size)) {
*out_omit = 1;
return 0;
}
}
c = *tbool ? 0xff : 0x00;
cont = &c;
len = 1;
break;
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
case V_ASN1_BIT_STRING: {
int ret =
i2c_ASN1_BIT_STRING((ASN1_BIT_STRING *)*pval, cout ? &cout : NULL);
/* |i2c_ASN1_BIT_STRING| returns zero on error instead of -1. */
return ret <= 0 ? -1 : ret;
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
}
case V_ASN1_INTEGER:
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
case V_ASN1_ENUMERATED: {
/* |i2c_ASN1_INTEGER| also handles ENUMERATED. */
int ret = i2c_ASN1_INTEGER((ASN1_INTEGER *)*pval, cout ? &cout : NULL);
/* |i2c_ASN1_INTEGER| returns zero on error instead of -1. */
return ret <= 0 ? -1 : ret;
Correctly handle invalid ASN1_OBJECTs when encoding. asn1_ex_i2c actually does have an error condition, it just wasn't being handled. 628b3c7f2fdf68519c27dc087c400ca616616f4e, imported from upstream's f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less ASN1_OBJECTs and return an error. But it and the upstream change didn't actually work. -1 in this function means to omit the object, so OpenSSL was silently misinterpreting the input structure. This changes the calling convention for asn1_ex_i2c to support this. It is, unfortunately, a little messy because: 1. One cannot check for object presense without walking the ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be able to report an omitted element. 2. While the i2d functions report an omitted element by successfully writing zero bytes, i2c only writes the contents. It thus must distinguish between an omitted element and an element with zero-length contents. 3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather than -1. Those error paths are not actually reachable because they only check for NULL. In fact, OpenSSL has even unexported them. But I found a few callers. Rather than unwind all this and change the calling convention, I've just made it handle 0 and map to -1 for now. It's all a no-op anyway, and hopefully we can redo all this with CBB later. I've just added an output parameter for now. In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed that. Update-Note: A default-constructed object with a required ASN1_OBJECT field can no longer be encoded without initializing the ASN1_OBJECT. Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests that try to serialize an X509_new() must fill in all required fields. (Production code is unlikely to be affected because the output was unparsable anyway, while tests sometimes wouldn't notice.) Bug: 429 Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
4 years ago
}
case V_ASN1_OCTET_STRING:
case V_ASN1_NUMERICSTRING:
case V_ASN1_PRINTABLESTRING:
case V_ASN1_T61STRING:
case V_ASN1_VIDEOTEXSTRING:
case V_ASN1_IA5STRING:
case V_ASN1_UTCTIME:
case V_ASN1_GENERALIZEDTIME:
case V_ASN1_GRAPHICSTRING:
case V_ASN1_VISIBLESTRING:
case V_ASN1_GENERALSTRING:
case V_ASN1_UNIVERSALSTRING:
case V_ASN1_BMPSTRING:
case V_ASN1_UTF8STRING:
case V_ASN1_SEQUENCE:
case V_ASN1_SET:
default:
/* All based on ASN1_STRING and handled the same */
strtmp = (ASN1_STRING *)*pval;
cont = strtmp->data;
len = strtmp->length;
break;
}
if (cout && len)
OPENSSL_memcpy(cout, cont, len);
return len;
}