aboutsummaryrefslogtreecommitdiff
path: root/crypto/asn1
AgeCommit message (Collapse)AuthorFilesLines
2022-09-23Add int64 ASN1_INTEGER setters too.David Benjamin2-3/+16
https://boringssl-review.googlesource.com/c/boringssl/+/54307 added just the getters because no one was using the setters yet. But our long setter *already* implements the int64 version, so just complete the whole set and deprecate the old long-based APIs. Change-Id: Ieb793f3cf90d4214c6416ba2f10e641c46403188 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54526 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-09-06Add ASN1_INTEGER_get_int64 and ASN1_ENUMERATED_get_int64.David Benjamin2-23/+54
Node uses this. Change-Id: I13e1734a8f60d4ad0c6a7bcab830c3a0406542b1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54307 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2022-08-02Remove stale comment.David Benjamin1-2/+0
This comment refers to something that was removed in https://boringssl-review.googlesource.com/c/boringssl/+/43889 Change-Id: Icf10ed5eb2ce552f2c1dbcdb89853cddb1183ad1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53786 Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2022-07-30Replace OPENSSL_STATIC_ASSERT with static_assert.David Benjamin1-2/+2
The C11 change has survived for three months now. Let's start freely using static_assert. In C files, we need to include <assert.h> because it is a macro. In C++ files, it is a keyword and we can just use it. (In MSVC C, it is actually also a keyword as in C++, but close enough.) I moved one assert from ssl3.h to ssl_lib.cc. We haven't yet required C11 in our public headers, just our internal files. Change-Id: Ic59978be43b699f2c997858179a9691606784ea5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53665 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2022-07-29Make time_t conversions. Give up on the OS provided ones.Bob Beck6-230/+351
We only care about dates within years 0000 to 9999 for RFC5280. timegm() is only semi-standard. Some things require the setting awkward defines to get libc to give it to you. Other things let you have it but make it stop working at year 3000. Still other things have 32 bit time_t..... Let's just make our own that actually works. all the time, does everything with an int64_t, and fails if you want to send something out that would overflow a 32 bit time_t. In the process of doing this, we get rid of the old Julian date stuff from OpenSSL, which while functional was a bit awkward dealing only with days, and using the Julian calendar as the reference point instead of potentially something more useful. Julian seconds since Jan 1 1970 00:00:00 UCT are much more useful to us than Julian days since a Julian epoch. The OS implementations of timegm() and gmtime() also can be pretty complex, due to the nature of needing multiple timezone, daylight saving, day of week, and other stuff we simply do not need for doing things with certificate times. A small microbenchmark of 10000000 of each operation comparing this implementation to the system version on my M1 mac gives: bbe-macbookpro:tmp bbe$ time ./openssl_gmtime real 0m0.152s user 0m0.127s sys 0m0.018s bbe-macbookpro:tmp bbe$ time ./gmtime real 0m0.422s user 0m0.403s sys 0m0.014s bbe-macbookpro:tmp bbe$ time ./openssl_timegm real 0m0.041s user 0m0.015s sys 0m0.019s bbe-macbookpro:tmp bbe$ time ./timegm real 0m30.432s user 0m30.383s sys 0m0.040s Similarly On a glinux machine: bbe@bbe-glinux1:~$ time ./openssl_gmtime real 0m0.157s user 0m0.152s sys 0m0.008s bbe@bbe-glinux1:~$ time ./gmtime real 0m0.336s user 0m0.336s sys 0m0.002s bbe@bbe-glinux1:~$ time ./openssl_timegm real 0m0.018s user 0m0.019s sys 0m0.002s bbe@bbe-glinux1:~$ time ./timegm real 0m0.680s user 0m0.671s sys 0m0.011s bbe@bbe-glinux1:~$ Bug: 501 Change-Id: If445272d365f2c9673b5f3264d082af1a342e0a1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53245 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2022-07-07Tidy up ASN1_GENERALIZEDTIME_adj and ASN1_UTCTIME_adj.David Benjamin2-77/+46
ASN1_STRING_set saves having to manually manage the ASN1_STRING's buffer. Also size that buffer correctly and align the free_s vs tmps mismatch. We can also assume OPENSSL_gmtime writes to the passed-in struct tm. Every other caller already assumes this, and both POSIX (gmtime_r) and C (gmtime_s) agree this is valid. Finally, the cleanup logic is much simpler if we do all the time stuff before handling the maybe-object-reuse bits. Change-Id: I25befc8fa93a1a60246c6abcea6e5d58ee563b48 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53227 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-07-07Check for invalid UCS-2 and UTF-32 in ASN1_STRING_print_ex.David Benjamin4-178/+44
Switch to the CBS functions, which do all the checks together. This resolves a TODO that ASN1_STRING_print_ex was inconsistently checking for invalid codepoints. It also removes an optimization when round-tripping UTF-8. This optimization was incorrect if the input was invalid. Finally, this removes UTF8_getc, which no longer has any callers. (I've left UTF8_putc for now because CBB would force a malloc on every character, even with CBB_init_fixed. We should either decide we don't care, or make it possible to stack-allocate the cbb_buffer_st.) Update-Note: This will make ASN1_STRING_print_ex newly fail, but such inputs should be unreachable from the parser as of an earlier change. Change-Id: I52d747c500c6f5f9ef659cdee3ef5d241f38ed21 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53226 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2022-07-07Check Unicode string encodings in crypto/asn1.David Benjamin2-11/+101
This checks validity for UTF8String, BMPString, and UniversalString. When we detach the core types from ASN1_ITEM, this will likely also be reshuffled around, probably into type-specific functions. But for now just get the checks in place. Update-Note: Invalid strings in X.509 certificates and other ASN.1 structures will now be rejected. This change is less risky than it seems because most strings in X.509 are in X509_NAME, which already rejected invalid instances of these string types (but not other string types) during canonicalization. See https://crbug.com/boringssl/412 for a discussion of that mess. Bug: 427 Change-Id: I0d7e24dfd841703d2a8581ec4e78ed5bc3862d75 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53225 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2022-06-25Remove some unnecessary NULL checks.David Benjamin3-5/+3
Our free functions all tolerate NULL. Change-Id: Ifcb3185c8d2f34afb83f7286c3463136edc926fa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53125 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-06-25Reimplement ASN1_TIME_print with the new parser.David Benjamin2-136/+57
No sense in keeping two around. This does cause the functions to reject some previously accepted invalid inputs. These were intentionally accepted by https://boringssl-review.googlesource.com/c/boringssl/+/13082 for an old version of M2Crypto, but I belive we no longer need to be compatible with that. Update-Note: ASN1_TIME_print, ASN1_UTCTIME_print, and ASN1_GENERALIZEDTIME_print will no longer accept various invalid inputs. Change-Id: I4606d0b39585a19eb4b984ac809706e497a3f799 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53090 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2022-06-23Set is_first and is_last correctly with ASN1_STRFLGS_UTF8_CONVERT.David Benjamin2-5/+4
The comment says (in the now outdated orflags terms) that we don't need to worry about this case because is_first/is_last only affect ASCII codepoints, but it's easier to just set it correctly. Change-Id: Ib6db66adb162a555da50f563ffc9af9da4a878ec Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53126 Reviewed-by: Adam Langley <agl@google.com>
2022-06-22Remove unnecessary parens on return.David Benjamin11-41/+41
This is a mechanical change generated from the following command: find crypto/{asn1,pem,x509,x509v3} -name '*.c' -o -name '*.h' | xargs sed -i -e 's/return (\([^;()]*\));/return \1;/' Change-Id: I957295af96c4aa08d6006e27093fd3a07fb6fe75 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53089 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2022-06-22Run convert_comments.go on the recently-converted filesDavid Benjamin21-622/+522
This CL is the result of the following commands: for d in asn1 x509 x509v3 pem; do go run util/convert_comments.go crypto/$d/*.h go run util/convert_comments.go crypto/$d/*.c done Change-Id: If78433f68cb2f913b0de06ded744a5a65540e1cf Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53087 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2022-06-22Re-run clang-format with InsertBraces.David Benjamin20-266/+500
This CL runs the same command as in the preceding CL, but with 'IncludeBraces: true' added to .clang-format. I've split this out separately because the documentation says: > Setting this option to true could lead to incorrect code formatting > due to clang-format’s lack of complete semantic information. As such, > extra care should be taken to review code changes made by this option. I've also kept InsertBraces out of .clang-format for now because it's a fairly recent option, and clang-format fails when it sees unrecognized options. Change-Id: I305ea7bb2633704053a1f8de1e11b037b9fc8a76 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53086 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2022-06-22clang-format remaining directories.David Benjamin30-3902/+3686
Previously, we did not clang-format a few directories because we had left them largely untouched. clang-format them now so we're finally more uniform. This CL is the result of the following commands: for d in asn1 x509 x509v3 pem; do clang-format -i crypto/$d/*.h clang-format -i crypto/$d/*.c done (Written in this funny way because crypto/pem/*.h doesn't match anything.) Change-Id: I7f4ca9b3a9c8f07d6556e00e9e84b3c0880ee12e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53085 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2022-06-16Fix duplicate declarationsDavid Benjamin1-5/+3
Change-Id: Id5fda00fe27eb9bc8313dd81a5b0c720323e3903 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53045 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2022-06-16Validate ASN.1 times according to RFC 5280Bob Beck5-212/+77
Refuse to parse times that are invalid according to RFC 5280, with a few exceptions for compatibility. This can affect test code that relies on making and parsing certificates that contain invalid times. Update-Note: Certificates containing invalid ASN.1 times will no longer parse. Bug: 491, 427 Change-Id: I2a3fe3a4d359ac662340a225d05b360718eb8c29 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52665 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2022-05-30Manually fix a few tables in advance of clang-format.David Benjamin2-28/+64
clang-format gets very confused by the comments in these tables. (The comments seem to have already gotten a little messed up from upstream's reformatted.) Reformat them ahead of time. I removed the tag2str number comments as they aren't really doing much good at this point. Also remove the last entry in tag2bits because it's not actually used. ASN1_tag2bit only reads the first 31 entries. Change-Id: If50770fd79b9d6ccab5558d24b0ee3a27c81a452 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52731 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2022-05-30Rewrite ASN1_STRING_print_ex escaping.David Benjamin3-245/+61
The original implementation uses a table generated by a Perl script, and then relies on some subset of ASN1_STRFLGS_* constants overlapping with CHARTYPE_* constants, while masking off the ones that don't align. Allocating ASN1_STRFLGS_* constants is already complex with the XN_FLAG_* interaction. Avoid the additional CHARTYPE_* interaction by just writing out what it's recognizing in code. If you ignore CHARTYPE_PRINTABLESTRING (which is unused), that table is just recognizing 9 characters anyway. Also this gets charmap.h out of the way so I can clang-format every file in here without having to constantly exclude it. Change-Id: I73f31324e4b8a815887afba459e50ed091a9f999 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52729 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2022-05-20limit the feature macro stuff to __linux__Bob Beck1-1/+1
These definitions are to get access to getaddrinfo() and gmtime_r() when using glibc. This in turn conflicts with other places (which would have these things in their libc anyway) where using these feature flags turns off C11 functionality we would like to use. Bug:490 Change-Id: I66fdb7292cda788df19508d99e7303ed0d4f4bdd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52545 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2022-04-29Tidy up how ASN1_STRING_print_ex figures out the type.David Benjamin2-114/+119
Between the lookup table, the multiple layers of reuse of the "type" variable, it is a little hard to follow what's going on with ASN1_STRING_print_ex. Replace the lookup table with a switch-case (implicitly handles the bounds check, and we can let the compiler figure out the best spelling). Then, rather than returning a "character width", which doen't represent UTF-8, just use the already-defined MBSTRING_* constants. (These changes should be covered by the existing ASN1Test.StringPrintEx test.) Change-Id: Ie3b2557bfae0f65db969e90cd0c76bc8ade963d4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52365 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2022-04-29Remove the ASN1_TLC cache. It appears to not help performance.Bob Beck1-85/+37
Inspired by Joel Sing's work in libre. Change-Id: I17267af926b7d42472f7dae3205fda9aabdfa73d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52385 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-04-19Reject [UNIVERSAL 0] in DER/BER element parsers.David Benjamin2-31/+41
[UNIVERSAL 0] is reserved by X.680 for the encoding to use. BER uses this to encode indefinite-length EOCs, but it is possible to encode it in a definite-length element or in a non-EOC form (non-zero length, or constructed). Whether we accept such encodings is normally moot: parsers will reject the tag as unsuitable for the type. However, the ANY type matches all tags. Previously, we would allow this, but crypto/asn1 has some ad-hoc checks for unexpected EOCs, in some contexts, but not others. Generalize this check to simply rejecting [UNIVERSAL 0] in all forms. This avoids a weird hole in the abstraction where tags are sometimes representable in BER and sometimes not. It also means we'll preserve this check when migrating parsers from crypto/asn1. Update-Note: There are two kinds of impacts I might expect from this change. The first is BER parsers might be relying on the CBS DER/BER element parser to pick up EOCs, as our ber.c does. This should be caught by the most basic unit test and can be fixed by detecting EOCs externally. The second is code might be trying to parse "actual" elements with tag [UNIVERSAL 0]. No actual types use this tag, so any non-ANY field is already rejecting such inputs. However, it is possible some input has this tag in a field with type ANY. This CL will cause us to reject that input. Note, however, that crypto/asn1 already rejects unexpected EOCs inside sequences, so many cases were already rejected anyway. Such inputs are also invalid as the ANY should match some actual, unknown ASN.1 type, and that type cannot use the reserved tag. Fixed: 455 Change-Id: If42cacc01840439059baa0e67179d0f198234fc4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52245 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-08Remove ASN1_ADB_INTEGER.David Benjamin1-7/+3
Nothing uses it, and I've never seen an ASN.1 spec use ANY DEFINED BY with an integer selector. (Although X.680 1997 does seem to allow it.) Change-Id: Ie1076f58838e4b889c5e6e12e9ca6dd1012472e7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51636 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-08Correctly handle LONG_MIN in ASN1_INTEGER_get.David Benjamin2-32/+77
Along the way, add ASN1_INTEGER_get_uint64 from upstream, which has much better error-handling. Also fold the IntegerSetting test into the main integer test as the test data is largely redundant. Change-Id: I7ec84629264ebf405bea4bce59a13c4495d81ed7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51634 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-08Implement ASN1_INTEGER_set_uint64 with ASN1_STRING_set.David Benjamin1-18/+9
It's a little simpler to avoid messing around with malloc. It also allows ASN1_STRING to internally reuse its buffer or realloc. Change-Id: I12c9f8f7c1a22f3bcc919f5fcc8b00d442cf10f9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51633 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-08Rewrite and tighten ASN1_INTEGER encoding and decoding.David Benjamin2-193/+198
This fixes several issues around ASN1_INTEGER handling. First, invalid INTEGERs (not allowed in BER or DER) will no longer be accepted by d2i_ASN1_INTEGER. This aligns with upstream OpenSSL, which became strict in 6c5b6cb035666d46495ccbe4a4f3d5e3a659cd40, part of OpenSSL 1.1.0. In addition to matching the standard, this is needed to avoid round-tripping issues: ASN1_INTEGER uses a sign-and-magnitude representation, different from the DER two's complement representation. That means we cannot represent invalid DER INTEGERs. Attempting to do so messes up some invariants and causes values to not round-trip correctly when re-encoded. Thanks to Tavis Ormandy for catching this. Next, this CL tidies the story around invalid ASN1_INTEGERs (non-minimal and negative zero). Although we will never produce them in parsing, it is still possible to manually construct them with ASN1_STRING APIs. Historically (CVE-2016-2108), it was possible to get them out of the parser, due to a different bug, *and* i2d_ASN1_INTEGER had a memory error in doing so. That different bug has since been fixed, but we should still handle them correctly and test this. (To that end, this CL adds a test we ought to have added importing upstream's 3661bb4e7934668bd99ca777ea8b30eedfafa871 back in c4eec0c16b02c97a62a95b6a08656c3a9ddb6baa.) As the two's complement invariants are subtle as it is, I've opted to just fix the invalid values before encoding. However, invalid ASN1_INTEGERs still do not quite work right because ASN1_INTEGER_get, ASN1_INTEGER_cmp, and ASN1_STRING_cmp will all return surprising values with them. I've left those alone. Finally, that leads to the zero value. Almost every function believes the representation of 0 is a "\0" rather than "". However, a default-constructed INTEGER, like any other string type, is "". Those do not compare as equal. crypto/asn1 treats ASN1_INTEGER generically as ASN1_STRING enough that I think changing the other functions to match is cleaner than changing default-constructed ASN1_INTEGERs. Thus this CL removes all the special cases around zero. Update-Note: Invalid INTEGERs will no longer parse, but they already would not have parsed in OpenSSL. Additionally, zero is now internally represented as "" rather than "\0". Bug: 354 Change-Id: Id4d51a18f32afe90fd4df7455b21e0c8bdbc5389 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51632 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-08Deduplicate the rest of ASN1_INTEGER and ASN1_ENUMERATED.David Benjamin2-208/+72
These functions need some work, but first avoid the duplicate versions. See also upstream's 6c5b6cb035666d46495ccbe4a4f3d5e3a659cd40. Update-Note: ASN1_INTEGER_to_BN and ASN1_ENUMERATED_to_BN will now fail when called on an ASN1_STRING/ASN1_INTEGER/ASN1_ENUMERATED (they're all the same type) with the wrong runtime type value. Previously, callers that mixed them up would get the right answer on positive values and silently misinterpret the input on negative values. This change matches OpenSSL's 1.1.0's behavior. Change-Id: Ie01366003f7b2e49477cb73eaf7eaac26d86675d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51631 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-08Fix theoretical overflow in ASN1_INTEGER_cmp.David Benjamin1-12/+16
While unlikely, ASN1_STRING_cmp is allowed to return INT_MIN (by way of memcmp), in which case negating would overflow. Change-Id: Iec63a6acfad2c662493d22a0acea39ca630881c8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51630 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-07Rewrite ASN1_INTEGER tests.David Benjamin1-28/+325
This extends the old ASN1_INTEGER_set tests to cover all integers. There are a whole bunch of ways to construct and convert ASN1_INTEGERs (DER, BIGNUM, uint64_t, long, etc.). Rather than maintain one set of test vectors for small numbers and another for BIGNUMs, this CL makes a single set of BIGNUM-based test vectors. Notably, this test now covers: - Serialization and deserialization - ASN1_INTEGER_get, not just ASN1_INTEGER_set - BIGNUM conversions - ASN1_INTEGER_cmp Later CLs will add to this or change code covered by it. Change-Id: I05bd6bc9e70c392927937c2f727cee25092802a1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51629 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-07Reimplement ASN1_get_object with CBS.David Benjamin1-86/+39
Now we only have one BER/DER TLV parser. Annoyingly, this uses the CBS BER function, not the DER one. This is because Android sometimes needs allow a non-minimal length in certificate signature fields (see b/18228011). For now, this CL calls CBS_get_any_ber_asn1_element. This is still an improvement over the old parser because we'll reject non-minimal tags (which are actually even forbidden in BER). Later, we should move the special case to just the signature field, and ultimately to a preprocessing step specific to that part of Android. Update-Note: Invalid certificates (and the few external structures using asn1t.h) with incorrectly-encoded tags will now be rejected. Bug: 354 Change-Id: I56a7faa1ffd51ee38cc315ebaddaef98079fd90e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51626 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2022-03-07Use ctype(3) in a more standards-conformant way.Thomas Klausner1-1/+2
Fixes build on NetBSD. Fixed: 483 Change-Id: I329eb327b67590828a3891f77a2cbbee5ec7affc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51705 Reviewed-by: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-11-04Add missing assert.h include.David Benjamin1-0/+1
Change-Id: I4af18ce3de2a01a8a5184096b07354bcbd24caf1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50265 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-11-01Check tag class and constructed bit in d2i_ASN1_OBJECT.David Benjamin2-17/+49
d2i_ASN1_OBJECT had a similar set of bugs in as in https://boringssl-review.googlesource.com/c/boringssl/+/49866. This does not affect any other d2i functions. Those already go through the ASN1_ITEM machinery. Update-Note: d2i_ASN1_OBJECT will now notice more incorrect tags. It was already checking for tag number 6, so it is unlikely anyone was relying on this as a non-tag-checking parser. Change-Id: I30f9ad28e3859aeb7a38c0ea299cd2e30002abce Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50290 Reviewed-by: Adam Langley <agl@google.com>
2021-11-01Enforce DER rules for BIT STRING values.David Benjamin2-10/+16
DER requires BIT STRING padding bits be zero. Bug: 354 Change-Id: Id59154cc4e77f91df8b9ff1eb1b09514116808da Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50288 Reviewed-by: Adam Langley <agl@google.com>
2021-11-01Remove support for indefinite lengths in crypto/asn1.David Benjamin3-163/+59
This simplifies the ASN1_get_object calling convention and removes another significant source of tasn_dec.c complexity. This change does not affect our PKCS#7 and PKCS#12 parsers. Update-Note: Invalid certificates (and the few external structures using asn1t.h) with BER indefinite lengths will now be rejected. Bug: 354 Change-Id: I723036798fc3254d0a289c77b105fcbdcda309b2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50287 Reviewed-by: Adam Langley <agl@google.com>
2021-11-01Remove support for constructed strings in crypto/asn1.David Benjamin1-141/+14
Constructed strings are a BER mechanism where a string type can be represented as a tree of constructed nodes and primitive leaves, that then have to be concatenated by the parser. This is prohibited in DER and a significant source of complexity in our parser. Note this change does not affect our PKCS#7 and PKCS#12 parsers (where BER is sadly necessary for interop) because those use CBS. Update-Note: Invalid certificates (and the few external structures using asn1t.h) with BER constructed strings will now be rejected. Bug: 354 Change-Id: I5a8ee028ec89ed4f2d5c099a0588f2029b864580 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50286 Reviewed-by: Adam Langley <agl@google.com>
2021-10-21Trim some undocumented symbols from asn1.h.David Benjamin1-0/+18
ASN1_ENCODING can be unexported because all types using it are now hidden. This does mean external uses of <openssl/asn1t.h> can no longer use ASN1_SEQUENCE_enc, but there do not seem to be any such uses. ASN1_TLC and ASN1_TEMPLATE typedefs are only necessary for users of asn1t.h. I'm hopeful we can do away with ASN1_TLC once I get to reworking tasn_dec.c. ASN1_TEMPLATE is somewhat stuck, though all references should be hidden behind macros. ASN1_generate_* appear to only referenced within the library. Remove the unused one and move the other to x509/internal.h. (asn1_gen.c is currently in crypto/x509 rather than crypto/asn1, so I put it in x509/internal.h to match. I'll leave figuring out that file to later.) Annoyingly, asn1/internal.h now pulls in asn1t.h, but so it goes. Change-Id: I8b43de3fa9647883103006e27907730d5531fd7d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50106 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-10-21Document and const-correct multi-string types.David Benjamin1-3/+5
While I'm here, add missing parentheses around the B_ASN1_* bitmasks. I've tossed ASN1_PRINTABLE into the deprecated bucket, though X509_NAME relies on it, because it is a mess. Bug: 407, 426 Change-Id: I287f60e98d6c9f237908011e1a816f4b4fb4433e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50105 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-10-20Unexport ASN1_OBJECT_new.David Benjamin1-0/+2
Outside the library, this function is practically useless. It creates an empty ASN1_OBJECT, which can never be filled in because the struct is private and there are no mutating setters. (See https://boringssl-review.googlesource.com/c/boringssl/+/46164 and https://boringssl-review.googlesource.com/c/boringssl/+/48326 for a discussion on why it's important ASN1_OBJECTs are immutable.) Update-Note: ASN1_OBJECT_new is no longer exported. While this function does remain in OpenSSL, it is extremely unlikely anyone has found a use for this function. Bug: 452 Change-Id: I111a9a1ce3ca4d7aa717a3c3a03d34c05af8fdbd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50025 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-10-20Return 0x80 in all ASN1_get_object error paths.David Benjamin2-6/+13
If the header is valid, but the body is truncated, ASN1_get_object intentionally preserves the indefinite-length and constructed output bits. This means callers who check for error with == 0x80 may read off the end of the buffer on accident. This is unlikely to break callers: 0x80 was already a possible error value, so callers already needed to handle it. The original function's aim in returning more information is unlikely to matter because callers cannot distinguish 0x80 (could not parse header) and 0x80 (header was valid, definite-length, and primitive, but length was too long). Update-Note: ASN1_get_object's calling convention is slightly simplified. Bug: 451 Change-Id: If2b45c47e6b8864aef9fd5e04f313219639991ed Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50005 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-10-19Document and const-correct ASN1_TYPE functions.David Benjamin1-1/+1
Also fill in docs for some easy ASN1_STRING wrappers while I'm here. (Not sure why they exist, but removing them is probably more trouble than is worth it.) Bug: 407, 426 Change-Id: Id12c5fbc84982728435d105d66a3b63e5f3a1d15 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49945 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-10-18Fix error-handling for i2a_ASN1_OBJECT.David Benjamin2-15/+108
Some BIO_write failures weren't handled. Otherwise would successfully write truncated results. The other i2a functions all report -1 on truncation, so match. While I'm here, write a test to make sure I didn't break this. Change-Id: If17d0209e75c15b3f37bceb1cdfb480fd2c62c4d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49931 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-10-18Fold i2a_ASN1_ENUMERATED into i2a_ASN1_INTEGER.David Benjamin2-93/+5
They do the same thing, except i2a_ASN1_ENUMERATED has a bug and doesn't handle negative values. Change-Id: Ifb22aa4e4d6c441a39cf6b3702cce7f6d12a94ae Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49929 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-10-18Fix BIT STRING comparison in ASN1_STRING_cmp.David Benjamin4-14/+160
The comparison should notice differences in bit count. Update-Note: ASN1_STRING_cmp no longer incorrectly treats BIT STRINGs with different padding bits as equal. Bug: 446 Change-Id: I22b3fcc5d369540d029ca234e9b3b02402cec4c3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49928 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-10-18Rewrite ASN1_item_pack and ASN1_item_unpack.David Benjamin2-29/+84
ASN1_item_unpack was missing checks for trailing data. ASN1_item_pack's error handling was all wrong. (Leaking the temporary on error, checking the the wrong return value for i2d, would-be redundant check for NULL, were the other check not wrong.) Update-Note: ASN1_item_unpack now checks for trailing data. Change-Id: Ibaa19ba2b264fca36dd21109e66f9558d373c58b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49927 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
2021-10-18Document some more ASN1_ITEM-associated functions.David Benjamin1-0/+1
In doing so, fix ASN1_item_pack to not use the ASN1_OCTET_STRING typedef. The function makes an untyped ASN1_STRING. With all these caveats, one might think that ASN1_BOOLEAN ASN1_ITEMs are pretty useless. This is about right. They're really only usable embedded as a field in another struct. Bug: 426 Change-Id: Id7830b91b2d011038ce79ec848e17ad6241423e1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49926 Reviewed-by: Adam Langley <agl@google.com>
2021-10-15Const-correct the low-level ASN1 i2d functions.David Benjamin3-5/+11
This is completely unchecked for now, as it all goes through tasn_enc.c. But the only non-const encoders now are X509_NAME, and the functions that call into it, so we can fix up the ones at the bottom. I haven't done the macros that use the "name" or "fname" variants. The set of macros for const are a little weird. But before expanding the header macros out, I wanted to change the signatures on the macro side once, so the compiler checks they're expanded correctly. Update-Note: The type signature of some i2d functions, such as i2d_ASN1_OCTET_STRING, is now const-correct. Bug: 407 Change-Id: I03988f5591191b41ab4e7f014bd8d41cb071b39a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49908 Reviewed-by: Adam Langley <agl@google.com>
2021-10-12Check tag class and constructed bit in d2i_ASN1_BOOLEAN.David Benjamin2-3/+42
d2i_ASN1_BOOLEAN and i2d_ASN1_BOOLEAN don't go through the macros because ASN1_BOOLEAN is a slightly weird type (int instead of pointer). Their tag checks were missing a few bits. This does not affect any other d2i functions. Those already go through the ASN1_ITEM machinery. Change-Id: Ic892cd2a8b8f9ceb11e43d931f8aa6df921997d3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49866 Reviewed-by: Adam Langley <agl@google.com>
2021-10-12Use typedefs in i2d and d2i_ASN1_BOOLEAN.David Benjamin1-20/+14
This makes it slightly clearer which ints are lengths and which are substituting for T*. (ASN1_BOOLEAN is weird. It is the one non-pointer representation in crypto/asn1.) Change-Id: I93ff87264835e64c9f8613edae63e93731e77548 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49865 Reviewed-by: Adam Langley <agl@google.com>
generated by cgit v1.1 at 2024-07-10 09:24:36 +0000