Age | Commit message (Collapse) | Author |
|
ok tb@
|
|
No need for an inconsistently named local variable and a ternary operator.
ok jsing
|
|
This used to be a dangerous implementation detail of BIO_new() that was
never used outside of libcrypto.
ok jsing
|
|
ok tb@
|
|
BIO_set() is a dangerous function that cannot be used safely. Thankfully,
the only consumer is BIO_new(), hence inline the functionality and disable
the BIO_set() function (for complete removal in the near future).
ok tb@
|
|
Rather than 'a' or 'b', use 'bio' more consistently - there are still some
more complex cases that have been left alone for now. Also use fewer
parentheses.
No change to generated assembly other than line numbers.
|
|
If CRYPTO_dup_ex_data() fails, the new_bio is leaked. If an error occurs
after the first iteration, all members of the new chain except the head
are leaked.
ok jsing
|
|
ok beck jsing millert
|
|
me aliasing symbols not in the headers I was procesing.
This unbreaks the namespace build so it will pass again
ok tb@
|
|
ok jsing@
|
|
At least SMIME_text() relies on this. Pushing an error on the stack trips
PKCS7 regress in py-cryptography, so indicate nothing was written instead
of throwing an error.
Reported by Alex Gaynor a while back
ok jsing
|
|
jsing@ worries that cycle prevention might increase risk because
software that is not checking return values (and indeed, not checking
is likely common in practice) might silently behave incorrectly
with cycle prevention whereas without, it will likely either crash
right away through infinite recursion or at least hang in an infinite
loop when trying to use the cyclic chain, in both cases making it
likely that the bug will be found and fixed.
Besides, tb@ points out that BIO_set_next(3) ought to behave as
similarly as possible to BIO_push(3), but adding cycle prevention
to BIO_set_next(3) would be even less convincing because that
function does not provide a return value, encouraging users to
expect that it will always succeed. While a safe idiom for checking
the success of BIO_set_next(3) could easily be designed, let's be
realistic: application software would be highly unlikely to pick up
such an idiom.
|
|
and reports failure if a call would result in a cycle.
The algorithm used was originally suggested by jsing@.
Feedback and OK tb@.
|
|
and next_bio fields of all BIO objects in all affected chains, no
matter what the arguments are.
In particular, if the second argument (the one to be appended) is
not at the beginning of its chain, properly detach the beginning
of its chain before appending.
We have weak indications that this bug might affect real-world code.
For example, in FreeRDP, file libfreerdp/crypto/tls.c, function
bio_rdp_tls_ctrl(), case BIO_C_SET_SSL, BIO_push(3) is definitely
called with a second argument that is *not* at the beginning of its
chain. Admittedly, that code is hard to fathom, but it does appear
to result in a bogus prev_bio pointer without this patch.
The practical impact of this bug in this and other software remains
unknown; the consequences might possibly escalate up to use-after-free
issues if BIO_pop(3) is afterwards called on corrupted BIO objects.
OK tb@
|
|
invariants of the prev_bio and next_bio fields of all BIO objects
in all involved chains, no matter which arguments this function is
called with.
Both real-world uses of this function (in libssl and freerdp) have
been audited to make sure this makes nothing worse. We believe libssl
behaves correctly before and after the patch (mostly because the second
argument is NULL there), and we believe the code in freerdp behaves
incorrectly before and after the patch, leaving a prev_bio pointer in
place that is becoming bogus, only in a different object before and
after the patch. But after the patch, that bogus pointer is due to a
separate bug in BIO_push(3), which we are planning to fix afterwards.
Joint work with and OK tb@.
|
|
As schwarze points out, you can pop any BIO in a chain, not just the first
one (bonus points for a great name for this API).
The internal doubly linked was used to fix up the BIO chain bio was part
of when you BIO_pop() a bio that wasn't in the first position, which is
explicitly allowed in our documentation and implied by OpenSSL's.
|
|
For various historical reasons, there are a number of cases where our
BIO_read() and BIO_write() return slightly different values to what
OpenSSL 3.x does (of course OpenSSL 1.0 differs from OpenSSL 1.1 which
differs from OpenSSL 3.x). Mostly align these - some further work will be
needed.
Issue raised by tb@ who also wrote some test code.
|
|
While BIO chains are doubly linked lists, nothing has ever made use of this
fact internally. Even libssl has failed to maintain prev_bio properly in
two places for a long time. When BIO was made opaque, the opportunity to
fix that was missed. Instead, BIO_set_next() now allows breaking the lists
from outside the library, which freerdp has long done.
Problem found by schwarze while trying to document BIO_set_next().
schwarze likes the idea
ok jsing
|
|
If the bgets() callback returns <= 0, we currently rely on the user
provided callback to set readbytes, which isn't ideal. This also
matches what's done in BIO_read() and BIO_write().
ok jsing
|
|
This provides support for new-style BIO callbacks in
BIO_{read,write,gets,puts}() and a helper function to
work out whether it should call the new or the old
style callback. It also adds a few typedefs and minor
code cleanup as well as the BIO_{get,set}_callback_ex()
from jsing, ok tb
|
|
that will need it in the upcoming bump.
discussed with jsing
|
|
Needed by freerdp.
ok inoguchi jsing
|
|
This will be needed in libssl and freerdp after the next bump.
ok inoguchi jsing
|
|
how application software uses the API function BIO_indent(3):
If the caller asks for some output, but not more than some negative
number of bytes, give them zero bytes of output rather than drowning
them in nearly INT_MAX bytes.
OK tb@
|
|
ok beck jsing
|
|
Some bread/bwrite functions implement this themselves, while others do not.
This makes it consistent across all BIO implementations.
Addresses an issue that Guido Vranken found with his fuzzer.
ok tb@
|
|
functions.
ok beck, jsing
|
|
|
|
ok jsing
|
|
Based on BoringSSL.
|
|
ok jsing
|
|
as was done earlier in libssl. Thanks inoguchi@ for noticing
libssl had more reacharounds into this.
ok jsing@ inoguchi@
|
|
15 years.
|
|
Remove unnecessary NULL check.
ok miod@
|
|
PR #3439 via OpenSSL trunk
|
|
Remove the openssl public includes from cryptlib.h and add a small number
of includes into the source files that actually need them. While here,
also sort/group/tidy the includes.
ok beck@ miod@
|
|
|
|
ok guenther
|
|
ok miod
|
|
|
|
OPENSSL_foo wrappers. This changes:
OPENSSL_malloc->malloc
OPENSSL_free->free
OPENSSL_relloc->realloc
OPENSSL_freeFunc->free
|
|
|
|
readable. This pass is whitespace only and can readily be verified using
tr and md5.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|