| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Instead of bounding only bounding the group order, also bound the
cofactor using Hasse's theorem. This could probably be made a lot
tighter since all curves of cryptographic interest have small
cofactors, but for now this is good enough.
A timeout found by oss-fuzz creates a "group" with insane parameters
over a 40-bit field: the order is 14464, and the cofactor has 4196223
bits (which is obviously impossible by Hasse's theorem). These led to
running an expensive loop in ec_GFp_simple_mul_ct() millions of times.
Fixes oss-fuzz #46056
Diagnosed and fix joint with jsing
ok inoguchi jsing (previous version)
|
|
|
|
|
|
|
| |
The cofactor we tried to calculate should only be zeroed if we failed
to compute it.
ok inoguchi jsing
|
| |
|
|
|
|
| |
OK tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
An IA5STRING is a Pascal string that can have embedded NULs and is
not NUL terminated (except that for legacy reasons it happens to be).
Instead of taking the strlen(), use the already known ASN.1 length and
use strndup() instead of strdup() to generate NUL terminated strings
after some existing code has checked that there are no embedded NULs.
In v2i_GENERAL_NAME_ex() use %.*s to print the bytes. This is not
optimal and might be switched to using strvis() later.
ok beck inoguchi jsing
|
|
|
|
|
|
|
|
|
| |
Along the way, rather than having yet another piece of code that parses
OID arcs, reuse oid_parse_arc(). Always allocate a new ASN1_OBJECT rather
than doing a crazy dance with ASN1_OBJECT_FLAG_DYNAMIC and trying to free
parts of an ASN1_OBJECT if one is passed in.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
| |
This takes a CBS, gets the ASN.1 identifier and length, ensures the
resulting identifier is a valid primitive, then returns the tag number and
the content as a CBS.
ok inoguchi@ tb@
|
|
|
|
| |
ok tb@, deraadt@, kettenis@
|
|
|
|
|
|
|
|
|
| |
arm_arch.h and armcap.c are shared between armv7 and aarch64 which
results in an inscrutable #ifdef maze. Move copies of these files
into arch/{arm,aarch64}/ with appropriate names and some trivial
minor adjustments.
ok deraadt inoguchi kettenis
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
The current OBJ_txt2obj() implementation converts the text to ASN.1
object content octets, builds a full DER encoding from it, then feeds
the entire thing back through the DER to ASN.1 object conversion. Rather
than doing this crazy dance, provide an t2i_ASN1_OBJECT_internal() function
that converts the text to ASN.1 object content octets, then creates a new
ASN1_OBJECT and attaches the content octets to it.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
| |
Rewrite the ascii/text to ASN.1 object conversion code using CBB/CBS,
while also addressing some of the bizarre behaviour (such as allowing
mixed separators and treating '..' as a zero value).
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
|
|
| |
Rework ASN1_STRING_set() so that we always clear and free an existing
allocation, prior to storing the new data. This fixes a number of issues,
including a failure to zero data if the existing allocation was too small.
This also fixes other bugs such as leaving the allocation uninitialised
if NULL is passed for data. Require -1 where strlen() is expected and
improve length and overflow checks.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
|
| |
gcc 4 on sparc64 issues a few 'warning: value computed is not used'.
There are two cases: sk_set_cmp_function() returns the old comparison
function of the stack which we don't care about. The one warning about
an sk_delete() is about a return value that we know already and which
we will free a few lines down.
ok inoguchi miod
|
| |
|
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A bug in the implementation of the Tonelli-Shanks algorithm can lead to
an infinite loop. This loop can be hit in various ways, in particular on
decompressing an elliptic curve public key via EC_POINT_oct2point() - to
do this, one must solve y^2 = x^3 + ax + b for y, given x.
If a certificate uses explicit encoding for elliptic curve parameters,
this operation needs to be done during certificate verification, leading
to a DoS. In particular, everything dealing with untrusted certificates
is affected, notably TLS servers explicitly configured to request
client certificates (httpd, smtpd, various VPN implementations, ...).
Ordinary TLS servers do not consume untrusted certificates.
The problem is that we cannot assume that x^3 + ax + b is actually a
square on untrusted input and neither can we assume that the modulus
p is a prime. Ensuring that p is a prime is too expensive (it would
likely itself lead to a DoS). To avoid the infinite loop, fix the logic
to be more resilient and explicitly limit the number of iterations that
can be done. The bug is such that the infinite loop can also be hit for
primes = 3 (mod 4) but fortunately that case is optimized earlier.
It's also worth noting that there is a size bound on the field size
enforced via OPENSSL_ECC_MAX_FIELD_BITS (= 661), which help mitigate
further DoS vectors in presence of this fix.
Reported by Tavis Ormandy and David Benjamin, Google
Patch based on the fixes by David Benjamin and Tomas Mraz, OpenSSL
ok beck inoguchi
|
|
|
|
|
|
|
|
|
|
| |
Some things issue and expect that we support a non-standard extension of
accepting any email address from a host by prefixing an email name
constraint with @. This used to be the case with the old code as well.
Pointed out and based on a diff by Alex Wilson.
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
Instead of having the caller allocate and pass in a new
x509_constraints_name struct, handle allocation inside
x509_constraints_validate(). Also make the error optional.
All this is done to simplify the call sites and to make it
more obvious that there are no leaks.
ok jsing
|
|
|
|
|
|
|
| |
This fixes a bug in ASN1_STRING_set0() where it does not respect the
ASN1_STRING_FLAG_NDEF flag and potentially frees memory that we do not own.
ok inguchi@ tb@
|
|
|
|
|
|
|
| |
Use consistent variable names (astr/src) rather than 'a', 'bs', 'str', 'v'
or 'x', add some whitespace and remove some unneeded parentheses.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
| |
The dirname constraint must be a prefix in DER format, so relax the
check from requiring equal-length strings to allow shorter names also.
From Alex Wilson
ok jsing
|
|
|
|
|
|
| |
From Alex Wilson
ok jsing
|
|
|
|
|
|
|
|
| |
The previous versions were too strict and disallowed leading dots.
From Alex Wilson
ok jsing
|
| |
|
|
|
|
|
|
| |
From Alex Wilson
ok jsing
|
|
|
|
|
|
|
|
|
|
|
| |
The constructed ASN.1 handling in asn1_d2i_ex_primitive() and asn1_ex_c2i()
currently has code to potentially avoid a malloc/memcpy - this is a less
common code path and it introduces a bunch of complexity for minimal gain.
In particular, we're manually adding a trailing NUL when ASN1_STRING_set()
would already do that for us, plus we currently manually free() the data on
an ASN1_STRING, rather than using freezero().
ok inoguchi@ tb@
|
| |
|
| |
|
|
|
|
|
|
| |
defined behavior.
ok deraadt inoguchi
|
|
|
|
|
|
|
|
|
|
|
| |
The conversion to CBB made us write out an extra NUL since we no longer
use the return value of i2t_ASN1_OBJECT() (which returns strlen(data))
but rather the size of the CBB (which includes a terminal NUL) to write
out data.
Issue found by anton via an openssl-ruby test failure.
ok jsing
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Rewrite the ASN1_OBJECT content to ascii/text conversion code using CBB and
CBS. Currently there is a strange split with i2t_ASN1_OBJECT() calling
OBJ_obj2txt() which implements the conversion, while OBJ_txt2obj() calls
back into the misnamed a2d_ASN1_OBJECT() function. Move the conversion
code into asn1/a_object.c and have OBJ_txt2obj() call that instead.
ok inoguchi@ tb@
|
|
|
|
| |
ok tb@
|
| |
|
|
|
|
| |
i is a silly name for BN_num_bits(dsa->q); move a comment for readability.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
dsa_do_verify() has checks on dsa->p and dsa->q that ensure that p isn't
overly long and that q has one of the three allowed lengths specified in
FIPS 186-3, namely 160, 224, or 256.
Do these checks on deserialization of DSA keys without parameters. This
means that we will now reject keys we would previously deserialize. Such
keys are useless in that signatures generated by them would be rejected
by both LibreSSL and OpenSSL.
This avoids a timeout flagged in oss-fuzz #26899 due to a ridiculous
DSA key whose q has size 65KiB. The timeout comes from additional checks
on DSA keys added by miod in dsa_ameth.c r1.18, especially checking such
a humungous number for primality is expensive.
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
| |
This only occurs on very small payloads and tightly allocated buffers
that don't usually occur in practice.
This is OpenSSL f61c6804
ok inoguchi jsing
|
|
|
|
| |
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, a read/write memory BIO pulls up the data via memmove() on each
read. This becomes very expensive when a lot of small reads are performed,
especially if there is a reasonable amount of data stored in the memory
BIO.
Instead, store a read offset into the buffer and only perform a memmove()
to pull up the data on a write, if we have read more than 4096 bytes. This
way we only perform memmove() when the space saving will potentially be of
benefit, while avoiding frequent memmove() in the case of small interleaved
reads and writes.
Should address oss-fuzz #19881.
ok inoguchi@ tb@
|
| |
|
|
|
|
|
|
|
|
|
| |
In order to fix and improve the memory BIO, we need to be able to track
more than just a single BUF_MEM *. Provide a struct bio_mem (which
currently only contains a BUF_MEM *) and rework the internals to use this
struct.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
| |
This is a first pass that uses sensible and consistent names for variables.
Call the BIO 'bio' (instead of 'a', 'b', 'bp', or 'h'), drop a bunch of
unnecessary casts, simplify some logic and add additional error checking.
With input from and ok tb@
|
|
|
|
| |
ok gnezdo@ miod@ jmc@
|
|
|
|
| |
ok jmc@
|
|
|
|
|
|
|
|
|
|
| |
A fix for this was previously commited in r1.32, however while this added
a bounds check the logic means we still fall through and perform the
overread. Fix the logic such that we only log the error if the bounds check
fails. While here, flip the test around such that we check for validity then
print (which is more readable and matches earlier code).
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The current implementation uses an unsigned long, then switches to BN once
the arc exceeds its size. However, the complexity of BN_bn2dec() is
quadratic in the length of number being converted. This means that OIDs
with excessively large arcs take a lot of computation to convert to text.
While the X.660 specification states that arcs are unbounded, in reality
they are not overly large numbers - 640K^W64 bits ought to be enough for
any arc. Remove BN entirely, switch from unsigned long to uin64_t and fail
if an arc exceeds this size.
Identified via oss-fuzz timeouts - should fix #41028 and #44372.
ok tb@
|