Chiebot-Mirror/boringssl - boringssl - Gitea: Git with a cup of tea

Author	SHA1	Message	Date
David Benjamin	4f1fae3043	Fix the easy -Wformat-signedness errors. GCC has a warning that complains about even more type mismatches in printf. Some of these are a bit messy and will be fixed in separate CLs. This covers the easy ones. The .*s stuff is unfortunate, but printf has no size_t-clean string printer. ALPN protocol lengths are bound by uint8_t, so it doesn't really matter. The IPv6 printing one is obnoxious and arguably a false positive. It's really a C language flaw: all types smaller than int get converted to int when you do arithmetic. So something like this first doesn't overflow the shift because it computes over int, but then the result overall is stored as an int. uint8_t a, b; (a << 8) \| b On the one hand, this fixes a "missing" cast to uint16_t before the shift. At the same time, the incorrect final type means passing it to %x, which expects unsigned int. The compiler has forgotten this value actually fits in uint16_t and flags a warning. Mitigate this by storing in a uint16_t first. The story doesn't quite end here. Arguments passed to variadic functions go through integer promotion[0], so the argument is still passed to snprintf as an int! But then va_arg allows for a signedness mismatch[1], provided the value is representable in both types. The combination means that %x, though actually paired with unsigned, also accept uint8_t and uint16_t, because those are guaranteed to promote to an int that meets [1]. GCC recognizes [1] applies here. (There's also PRI16x, but that's a bit tedious to use and, in glibc, is defined as plain "x" anyway.) [0] https://en.cppreference.com/w/c/language/conversion#Default_argument_promotions [1] https://en.cppreference.com/w/c/variadic/va_arg Bug: 450 Change-Id: Ic1d41356755a18ab922956dd2e07b560470341f4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50765 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>	3 years ago
David Benjamin	16a94930ac	Add various OpenSSL compatibility functions. The non-_ex EVP_CIPHER_CTX Final functions are a bit interesting. Unlike EVP_DigestFinal(_ex), where the non-_ex version calls EVP_MD_CTX_cleanup for you, the EVP_CIPHER_CTX ones do not automatically cleanup. EVP_CipherFinal and EVP_CipherFinal_ex are identical in all releases where they exist. This appears to date to OpenSSL 0.9.7: Prior to OpenSSL 0.9.7, EVP_MD_CTX and EVP_CIPHER_CTX did not use void* data fields. Instead, they just had a union of context structures for every algorithm OpenSSL implemented. EVP_MD_CTX was truly cleanup-less. There were no EVP_MD_CTX_init or EVP_MD_CTX_cleanup functions at all. EVP_DigestInit filled things in without reference to the previous state. EVP_DigestFinal didn't cleanup because there was nothing to cleanup. EVP_CIPHER_CTX was also a union, but for some reason did include EVP_CIPHER_CTX_init and EVP_CIPHER_CTX_cleanup. EVP_CIPHER_CTX_init seemed to be optional: EVP_CipherInit with non-NULL EVP_CIPHER similarly didn't reference the previous state. EVP_CipherFinal did not call EVP_CIPHER_CTX_cleanup, but EVP_CIPHER_CTX_cleanup didn't do anything. It called an optional cleanup hook on the EVP_CIPHER, but as far as I can tell, no EVP_CIPHER implemented it. Then OpenSSL 0.9.7 introduced ENGINE. The union didn't work anymore, so EVP_MD_CTX and EVP_CIPHER_CTX contained void* with allocated type-specific data. The introduced EVP_MD_CTX_init and EVP_MD_CTX_cleanup. For (imperfect!) backwards compatibility, EVP_DigestInit and EVP_DigestFinal transparently called init/cleanup for you. EVP_DigestInit_ex and EVP_DigestFinal_ex became the more flexible versions that left init/cleanup to the caller. EVP_CIPHER_CTX got the same treatment with EVP_CipherInit/EVP_CipherInit_ex, but not EVP_CipherFinal/EVP_CipherFinal_ex. The latter did the same thing. The history seems to be that 581f1c84940d77451c2592e9fa470893f6c3c3eb introduced the Final/Final_ex split, with the former doing an auto-cleanup, then 544a2aea4ba1fad76f0802fb70d92a5a8e6ad85a undid it. Looks like the motivation is that EVP_CIPHER_CTX objects are often reused to do multiple operations with a single key. But they missed that the split functions are now unnecessary. Amusingly, OpenSSL's documentation incorrectly said that EVP_CipherFinal cleaned up after the call until it was fixed in 538860a3ce0b9fd142a7f1a62e597cccb74475d3. The fix says that some releases cleaned up, but there were, as far as I can tell, no actual releases with that behavior. I've put the new Final functions in the deprecated section, purely because there is no sense in recommending two different versions of the same function to users, and Final_ex seems to be more popular. But there isn't actually anything wrong with plain Final. Change-Id: Ic2bfda48fdcf30f292141add8c5f745348036852 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50485 Reviewed-by: Adam Langley <agl@google.com>	3 years ago
David Benjamin	9372f38cd0	Bound RSA and DSA key sizes better. Most asymmetric operations scale superlinearly, which makes them potential DoS vectors. This (and other problems) are mitigated with fixed sizes, like RSA-2048, P-256, or curve25519. In older algorithms like RSA and DSA, these sizes are conventions rather than well-defined algorithms. "Everyone" uses RSA-2048, but code which imports an RSA key may see an arbitrary key size, possibly from an untrusted source. This is commonly a public key, so we bound RSA key sizes in check_modulus_and_exponent_sizes. However, some applications import external private keys, and may need tighter bounds. These typically parse the key then check the result. However, parsing itself can perform superlinear work (RSA_check_key or recovering the DSA public key). This CL does the following: - Rename check_modulus_and_exponent_sizes to rsa_check_public_key and additionally call it from RSA_check_key. - Fix a bug where RSA_check_key, on CRT-less keys, did not bound d, and bound p and q before multiplying (quadratic). - Our DSA verifier had stricter checks on q (160-, 224-, and 256-bit only) than our DSA signer (multiple of 8 bits). Aligner the signer to the verifier's checks. - Validate DSA group sizes on parse, as well as priv_key < q, to bound the running time. Ideally these invariants would be checked exactly once at construction, but our RSA and DSA implementations suffer from some OpenSSL's API mistakes (https://crbug.com/boringssl/316), which means it is hard to consistently enforce invariants. This CL focuses on the parser, but later I'd like to better rationalize the freeze_private_key logic. Performance of parsing RSA and DSA keys, gathered on my laptop. Did 15130 RSA-2048 parse operations in 5022458us (3012.5 ops/sec) Did 4888 RSA-4096 parse operations in 5060606us (965.9 ops/sec) Did 354 RSA-16384 parse operations in 5043565us (70.2 ops/sec) Did 88 RSA-32768 parse operations in 5038293us (17.5 ops/sec) [rejected by this CL] Did 35000 DSA-1024/256 parse operations in 5030447us (6957.6 ops/sec) Did 11316 DSA-2048/256 parse operations in 5094664us (2221.1 ops/sec) Did 5488 DSA-3072/256 parse operations in 5096032us (1076.9 ops/sec) Did 3172 DSA-4096/256 parse operations in 5041220us (629.2 ops/sec) Did 840 DSA-8192/256 parse operations in 5070616us (165.7 ops/sec) Did 285 DSA-10000/256 parse operations in 5004033us (57.0 ops/sec) Did 74 DSA-20000/256 parse operations in 5066299us (14.6 ops/sec) [rejected by this CL] Update-Note: Some invalid or overly large RSA and DSA keys may previously have been accepted that are now rejected at parse time. For public keys, this only moves the error from verification to parsing. In some private key cases, we would previously allow signing with those keys, but the resulting signatures would not be accepted by BoringSSL anyway. This CL makes us behave more consistently. Bug: oss-fuzz:24730 Change-Id: I4ad2003ee61138b693e65d3da4c6aa00bc165251 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42504 Reviewed-by: Adam Langley <agl@google.com>	5 years ago
Adam Langley	fb0c05cac2	acvp: add CMAC-AES support. Change by Dan Janni. Change-Id: I3f059e7b1a822c6f97128ca92a693499a3f7fa8f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41984 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>	5 years ago