| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
| |
|
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
|
|
| |
Give example IPv6 addresses to clarify what is meant with 1, 2 or 3 zero
length elements.
tb made me look.
perverted, twisted, crippled
|
|
|
|
|
|
| |
These are not exactly useful and we previously stopped exposing them.
ok tb@
|
| |
|
|
|
|
|
|
|
| |
no overlap. Document that explicitly. Also make it more explicit that
that the caller must work with a copy of out.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
SSL_select_next_proto() is already quite broken by its design: const in,
non-const out, with the intention of pointing somewhere inside of the two
input pointers. A length returned in an unsigned char (because, you know,
the individual protocols are encoded in Pascal strings). Can't signal
uailure either. It also has an unreachable public return code.
Also, due to originally catering to NPN, this function opportunistically
selects a protocol from the second input (client) parameters, which makes
little sense for ALPN since that means the server falls back to a protocol
it doesn't (want to) support. If there's no overlap, it's the callback's
job to signal error to its caller for ALPN.
As if that wasn't enough misdesign and bugs, the one we're concerned with
here wasn't reported to us twice in ten years is that if you pass this API
a zero-length (or a sufficiently malformed client protocol list), it would
return a pointer pointing somewhere into the heap instead into one of the
two input pointers. This pointer could then be interpreted as a Pascal
string, resulting in an information disclosure of up to 255 bytes from the
heap to the peer, or a crash.
This can only happen for NPN (where it does happen in old python and node).
A long time ago jsing removed NPN support from LibreSSL, because it had
an utter garbage implementation and because it was practically unused.
First it was already replaced by the somewhat less bad ALPN, and the only
users were the always same language bindings that tend to use every feature
they shouldn't use. There were a lot of complaints due to failing test
cases in there, but in the end the decision turned out to be the right
one: the consequence is that LibreSSL isn't vulnerable to CVE-2024-5535.
Still, there is a bug here to fix. It is completely straightforward to
do so. Rewrite this mess using CBS, preserving the current behavior.
Also, we do not follow BoringSSL's renaming of the variables. It would
result in confusing code in almost all alpn callbacks I've seen in the
wild. The only exception is the accidental example of Qt.
ok jsing
|
|
|
|
|
|
|
|
|
| |
This code was only previously enabled if the minimum enabled version was
TLSv1.0 and a non-version locked method is in use. Since TLSv1.0 and
TLSv1.1 were disabled nearly a year ago, this code is no longer ever
being used.
ok tb@
|
|
|
|
| |
ok jsing
|
|
|
|
| |
suggested by jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
RSA key exchange is known to have multiple security weaknesses,
including being potentially susceptible to padding oracle and timing
attacks.
The RSA key exchange code that we inherited from OpenSSL was riddled
with timing leaks, many of which we fixed (or minimised) early on.
However, a number of issues still remained, particularly those
related to libcrypto's RSA decryption and padding checks.
Rework the RSA key exchange code such that we decrypt with
RSA_NO_PADDING and then check the padding ourselves in constant
time. In this case, the pre-master secret is of a known length,
hence the padding is also a known length based on the size of the
RSA key. This makes it easy to implement a check that is much safer
than having RSA_private_decrypt() depad for us.
Regardless, we still strongly recommend disabling RSA key exchange
and using other key exchange methods that provide perfect forward
secrecy and do not depend on client generated keys.
Thanks to Marcel Maehren, Nurullah Erinola, Robert Merget, Juraj
Somorovsky, Joerg Schwenk and Hubert Kario for raising these issues
with us at various points in time.
ok tb@
|
|
|
|
|
|
|
| |
The diff decoupling the shuffle from the table order still relied on PSK
being last because it failed to adjust the upper bound in the for loop.
ok jsing
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These constitute the bulk of the remaining global mutable state in
libcrypto. This commit moves most of them into data.rel.ro, leaving
out ERR_str_{functs,libraries,reasons} (which require a slightly
different approach) and SYS_str_reasons which is populated on startup.
The main observation is that if ERR_load_strings() is called with a 0 lib
argument, the ERR_STRING_DATA argument is not actually modified. We could
use this fact to cast away const on the caller side and be done with it.
We can make this cleaner by adding a helper ERR_load_const_strings() which
explicitly avoids the assignment to str->error overriding the error code
already set in the table.
In order for this to work, we need to sprinkle some const in err/err.c.
CMS called ERR_load_strings() with non-0 lib argument, but this didn't
actually modify the error data since it ored in the value already stored
in the table.
Annoyingly, we need to cast const away once, namely in the call to
lh_insert() in int_err_set_item(). Fixing this would require changing
the public API and is going to be tricky since it requires that the
LHASH_DOALL_FN_* types adjust.
ok jsing
|
| |
|
|
|
|
| |
ok kettenis@ deraadt@ tb@
|
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The content is effectively a u32 length prefixed field, so use
CBB_add_u32_length_prefixed(). Use BN_bn2binpad() rather than manually
padding if we need to extend and use sensible variable names so that the
code becomes more readable.
Note that since CBB can fail we now need to be able to indicate failure.
This means that BN_bn2mpi() can now return -1 when it would not have
previously (correct callers will check that BN_bn2mpi() returns a positive
length).
ok tb@
|
| |
|
|
|
|
|
|
| |
This is needed for an upcoming change in libcrypto.
ok tb@
|
| |
|
|
|
|
| |
This aligns it with do_ext_i2d()
|
|
|
|
| |
now that ext is free, we can use it like everywhere else
|
|
|
|
| |
In this code 'ext' is usually used for an X509_EXTENSION object.
|
|
|
|
|
|
|
|
| |
There's no reason for them not to be const. This is a piece of a larger
diff that I carry in several of my trees to move more things to rodata
or relro. The full diff requires a change to a public header and it's
very annoying to have to 'make includes' and recompile the entire lib
all the time when hopping from tree to tree.
|
|
|
|
| |
requested by jsing on review
|
|
|
|
| |
There are no nid variables in this file, so no need to disambiguate.
|
| |
|
|
|
|
| |
requested by jsing on review
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
If ASN1_OCTET_STRING_new() failed, ext_der would be leaked, fix this.
If i2d(foo, NULL) succeeded, the same is not guaranteed for the second
with appropriately sized buffer since i2d() may make further allocations
internally. So use the proper error check. Also transfer the ownership of
ext_der to the octet string to avoid a now possible double free.
ok jsing
|
| |
|
|
|
|
| |
ok jsing
|
| |
|
|
|
|
| |
requested by jsing on review
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This API is wrapped by nine *_get{,1}_ext_d2i() functions and they all
have the same defect: if an idx variable is passed in, multiple extensions
are handled incorrectly.
Clean up the mess that was the current implementation by replacing the
reimplementation of X509v3_get_ext_by_NID() with extra twists by actual
calls to the real thing. This way the madness is implemented explicitly
and can be explained in comments. The code still gets shorter.
In brief: always call this API with a known nid, pass crit, and a NULL idx.
If NULL is returned, crit != -1 is an error (malformed cert or allocation
failure).
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
pthread -> mutex
stdint -> uint8_t
stdio.h -> asprintf
stdlib.h -> calloc
string.h -> memcpy
ecdsa -> ECDSA_METHOD leftover, remove
ec -> EC_KEY
evp -> EVP_PKEY
pem -> PEM_read_bio_X509
x509 -> X509
90% of the diff is from tb@, I only spotted the missing string.h :)
ok tb@
|
|
|
|
| |
OK tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The combination of two bugs made this unexpectedly work as intended. To
appreciate this, let's first note that
a) check_issued(..., child, parent) checks if child was issued by parent.
b) X509_check_issued(child, parent) checks if parent was issued by child.
Now like in the real world, b) will only be true in unusual circumstances
(child is known not to be self-issued at this point). X509_check_issued()
fails by returning something different from X509_V_OK, so
return X509_check_issued(child, parent) != X509_V_OK;
will return true if child was issued by parent since then parent was indeed
not issued by child. On the other hand, if child was not issued by parent,
the verifier will notice elsewhere, e.g., in a signature check.
Fix this by reversing the order of child and parent in the above return
line and check for equality instead. This is nearly impossible to detect
in regress.
ok beck
|