| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
RFC 3779 section 2.1.2 does a decent job of explaining how IP addresses
are encoded in. What's stored amounts to a prefix with all trailing zero
octets omitted. If there are trailing zero bits in the last non-zero octet,
bs->flags & 7 indicates how many. addr_expand() expands this to an address
of length 4 or 16 depending on whether we deal with IPv4 or IPv6.
Since an address can be the lower or the upper bound of a prefix or
address range, expansion needs to be able to zero-fill or one-fill the
unused bits/octets. No other expansion is ever used, so simplify the
meaning of fill accordingly. There's no need to special case the case
that there are no unused bits, the masking/filling is a noop.
ok jsing
|
|
|
|
| |
in make_IPAddressFamily()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The IPAddrBlocks type, which represents the IPAddrBlocks extension,
should have exactly one IPAddressFamily per AFI+SAFI combination to
be delegated. make_IPAddressFamily() first builds up a search key
from the afi and safi arguments and then looks for an existing
IPAddressFamily with that key in the IPAddrBlocks that was passed
in. It returns that if it finds it or allocates and adds a new one.
This diff preserves the current behavior that the afi and *safi
arguments are truncated to 2 and 1 bytes, respectively. This may
change in the future.
ok inoguchi jsing
|
|
|
|
|
|
|
| |
The ASN.1 template for IPAddressFamily doesn't mark either of its two
members as optional, so they are allocated by IPAddressFamily_new().
ok inoguchi jsing
|
|
|
|
|
|
|
|
|
|
|
|
| |
Per RFC 3779 2.2.3.3, the addressFamily field contains the 2-byte AFI
and an optional 1-byte SAFI. Nothing else. The optional SAFI is nowhere
exposed in the API. It is used expliclty only for pretty printing. There
are implicit uses in a few places, notably for sorting/comparing where
trailing garbage would be erroneously taken into account.
Erroring in this situation will let us avoid this in upcoming revisions.
ok inoguchi jsing
|
|
|
|
|
|
|
|
|
|
|
| |
The manual byte bashing is performed more safely using this API
which would have avoided the out-of-bounds read that this API had
until a few years back.
The API is somewhat strange in that it uses the reserved AFI 0 as an
in-band error but it doesn't care about the reserved AFI 65535.
ok inoguchi jsing
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
as is done for most other X.509 v3 extension methods.
discussed with jsing
|
|
|
|
|
|
|
|
|
|
|
| |
The define implies that we have the RFC 3779 API and corresponding
symbols publicly exposed. We don't do that since there are still
concerns about its suitability and security. oss-fuzz has code
depending on this define and this broke its build as tracked down
by jsing. This commit gets us oss-fuzz builds back while keeping
job happy since the extension pretty printing will continue to work.
ok jsing
|
| |
|
|
|
|
| |
way too long.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
The first asserts ensure that things checked in the callers hold true.
Turn them into error checks and set the error on the X509_STORE_CTX
if it's present. Checking sk_value(..., i) with i < sk_num(...) isn't
useful, particularly if that check is done via an assert. Turn one
remaining assert into a NULL check. Finally, simplify the sk_num()
checks in the callers.
ok jsing
|
|
|
|
|
|
|
|
|
| |
The first assert ensures that a stack that was just sorted in a stronger
sense is sorted in a weak sense and the second assert ensures that
the result of the canonization procedure is canonical. All callers check
for error, so these asserts don't do anything useful.
ok jsing
|
|
|
|
|
|
| |
All callers ensure that aor != NULL, so this isn't necessary.
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
| |
The first assert ensure that a stack that was just sorted in a stronger
sense is sorted in a weak sense and the second assert ensures that
the result of the canonization procedure is canonical. All callers check
for error, so these asserts don't do anything useful.
ok jsing
|
|
|
|
|
|
| |
All callers ensure that aor != NULL, so this isn't necessary.
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This is reachable from x509_verify(), but all asserts are previously
checked in the caller. Turn them into error checks and make sure
the error is set on the X509_STORE_CTX if present. Change some
stack == NULL || sk_num(stack) == 0 checks into sk_num(stack) <= 0
which is equivalent but simpler.
ok jsing
|
|
|
|
|
|
|
| |
All internal callers check the return value and future external
callers will be happy not to hit an assert from the library.
ok jsing
|
|
|
|
|
|
|
|
|
| |
This can read a value in an arbitrary base from a string that is
supposed to be followed by whitespace or a colon, so it cannot be
switched to strtonum(). The current checks don't allow a read past
the end, but let's use the standard idiom instead.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Switch an insufficiently checked strtoul() to strtonum(). This can
be used to trigger a read of a user-controlled size from the stack.
$ openssl req -new -addext 'sbgp-ipAddrBlock = IPv4:192.0.2.0/12341234'
Segmentation fault (core dumped)
The bogus prefix length 12341234 is fed into X509v3_addr_add_prefix() and
used to read (prefixlen + 7) / 8 bytes from the stack variable 'min[16]'
that ends up as 'data' in the memmove in ASN1_STRING_set().
The full fix will add length checks to X509v3_addr_add_prefix() and
make_addressPrefix() and will be dealt with later. The entire
X509v3_{addr,asid}_* API will need a thorough review before it can be
exposed.
This code is only enabled in -current and can only be reached from
openssl.cnf files that contain sbgp-ipAddrBlock or from the openssl(1)
command line.
ok jsing
|
|
|
|
| |
in OpenSSL commit d2e9e320.
|
|
|
|
|
|
| |
evp.h will be moved to evp_locl.h in an upcoming bump.
ok inoguchi
|
|
|
|
|
|
|
| |
These functions previously used the old ASN1_{d2i,i2d}_{bio,fp}()
interfaces.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
|
| |
If EVP_PKEY_copy_parameters(3) fails - among other reasons, this
may happen when out of memory - the pkey argument and/or the chain
argument will not contain all the desired parameters after returning.
Consequently, report the failure to the caller rather than silently
ignoring it.
OK tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
by using X509_get0_pubkey(3) instead of X509_get_pubkey(3);
no functional change.
OK tb@
This is similar to the relevant part of the follwoing commit
from the OpenSSL 1.1.1 branch, which is still under a free licence,
but without the bug that commit introduced into this function in OpenSSL:
commit c01ff880d47392b82cce2f93ac4a9bb8c68f8cc7
Author: Dr. Stephen Henson <steve@openssl.org>
Date: Mon Dec 14 13:13:32 2015 +0000
|
|
|
|
|
|
|
|
| |
This way, CT extensions in certs will be parsed by the new CT code
when they are encountered. This gets rid of a lot of gibberish when
looking at a cert with 'openssl x509 -text -noout -in server.pem'
ok beck jsing
|
|
|
|
|
|
| |
certificte chain. This would happen when the verification callback was
in use, instructing the verifier to continue unconditionally. This could
lead to incorrect decisions being made in software.
|
|
|
|
|
|
| |
say: return_type *function_name(args);
not: return_type* function_name (args);
OK tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
make sure it fully re-initializes the object rather than leaving
behind a stale pointer and a stale type in the object.
The old behaviour was dangerous because X509_OBJECT_get_type(3)
would then return the stale type to the user and one of
X509_OBJECT_get0_X509(3) or X509_OBJECT_get0_X509_CRL(3) would
then return the stale pointer to the user, provoking a use-after-free
bug in the application program. Having these functions return
X509_LU_NONE and NULL is better because those are the documented
return values for these functions when the object is empty.
OK tb@
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
verification, accepting CRLs that ought to be rejected, if an unusual
combination of verification flags was specified.
If time verification was explicitly requested with
X509_V_FLAG_USE_CHECK_TIME, it was skipped on CRLs if
X509_V_FLAG_NO_CHECK_TIME was also set, even though the former is
documented to override the latter both in the OpenSSL and in the
LibreSSL X509_VERIFY_PARAM_set_flags(3) manual page.
The same bug in x509_check_cert_time() was already fixed by beck@
in rev. 1.57 on 2017/01/20.
This syncs the beginning of the function check_crl_time() with the
OpenSSL 1.1.1 branch, which is still under a free license.
OK beck@
This teaches that having too many flags and options is bad because they
breed bugs, and even more so if they are poorly designed to override
each other in surprising ways.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
and various style improvements from the OpenSSL 1.1.1 branch,
which is still under a free license.
- No need to #include <openssl/lhash.h>.
- BUF_MEM_free(3) and sk_pop_free(3) can handle NULL.
- sk_value(3) can handle -1.
- Test pointers with "== NULL" rather than with "!".
- Use the safer "p = malloc(sizeof(*p))" idiom.
- return is not a function.
- Delete very wrong commented out code.
Including parts of the these commits from the 2015 to 2018 time range:
25aaa98a b4faea50 90945fa3 f32b0abe 26a7d938 7fcdbd83 208056b2 5b37fef0
Requested by and OK tb@.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
and/or CRLs in the PEM input file (for example, if the file
is empty), provide an error message in addition to returning 0.
This merges another part of this OpenSSL commit,
which is still under a free license:
commit c0452248ea1a59a41023a4765ef7d9825e80a62b
Author: Rich Salz <rsalz@openssl.org>
Date: Thu Apr 20 15:33:42 2017 -0400
I did *not* add the similar message types X509_R_NO_CERTIFICATE_FOUND
and X509_R_NO_CRL_FOUND because both code inspection and testing
have shown that the code generating them is unreachable.
OK tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
which is still under a free license. No functional change.
- No need to #include <openssl/lhash.h> here.
- return is not a function.
- Do not use the pointless macro BIO_s_file_internal().
- No need to check for NULL before X509_CRL_free(3).
This includes parts of the following OpenSSL commits from
the 2015 to 2017 timeframe: 222561fe, 9982cbbb, f32b0abe, 26a7d938
OK tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
under a free license:
1. If the three X509_load_*(3) functions are called with a NULL
file argument, do not return 1 to the caller because the return
value 1 means "i loaded one certificate or CRL into the store".
2. When calling PEM load functions, do not ask the user for a
password in an interactive manner.
This includes parts of the following commits:
commit c0452248ea1a59a41023a4765ef7d9825e80a62b
Author: Rich Salz <rsalz@openssl.org>
Date: Thu Apr 20 15:33:42 2017 -0400
Message: [...] Remove NULL checks and allow a segv to occur. [...]
commit db854bb14a7010712cfc02861731399b1b587474
Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Mon Aug 7 18:02:53 2017 +0200
Message: Avoid surpising password dialog in X509 file lookup.
OK tb@
|
|
|
|
| |
ok gnezdo jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Get rid of the last X509_OBJECT_free_contents() call by moving the object
from the stack to the heap. I deliberately kept the obj variable to keep
obj and pobj separate. Rename the out parameter from issuer to out_issuer
to ensure that we only assign it when we have acquired a reference that we
can return. Add a new X509 *issuer. In the first part of the function,
acquire an extra reference before check_issuer/check_time.
In the second part of the function, acquire a reference inside the lock to
avoid a race. Deal with ret only in one place.
ok jsing
|
|
|
|
|
|
| |
x509_check_cert_time(). Matches a change made in OpenSSL 70dd3c65.
ok jsing
|
|
|
|
|
|
|
|
| |
Split the retrieval of the certs in the store's cache that match the
desired subject into a separate function. This greatly simplifies
locking, error handling and the flow of the function.
with/ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These functions are quite messy. On top of the tricky logic querying the
cache, then refreshing the cache (unconditionally or not), then querying
again, then extracting a list of certs/crls and bumping their refcounts,
things are intermixed with locking and needlessly early allocations that
then need to be cleaned up again.
Use X509_STORE_CTX_get_obj_by_subject() to avoid using an object on the
stack and defer allocation of the returned stack of certs to later.
Flatten the logic a bit and prepare for further refactoring.
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add a X509_STORE_add_object() function that adds an X509 object to the
store and takes care of locking and cleaning up. This way we can set up
an X509_OBJECT for both the cert and CRL case and hand over to the new
function.
There is one intentional change of behavior: if there is an attempt to
add an object which is already present in the store, succeed instead of
throwing an error. This makes sense and is also the OpenSSL behavior.
As pointed out by jsing, this is a partial fix for the long standing
GH issue #100 on libtls where connections would fail if the store
contains duplicate certificates.
Also: remove the internal X509_OBJECT_dec_ref_count(), which is no
longer used.
ok jsing
|
|
|
|
|
|
| |
simplify the flow of X509_add_lookup().
ok jsing
|