| Age | Commit message (Collapse) | Author |
|---|
|
|
|
suggested by jsing
|
|
suggested by jsing
|
|
more readable.
Repeated complaints by jsing
|
|
sk_find + sk_value into something easier to follow and swallow.
ok inoguchi jsing
|
|
ok inoguchi jsing
|
|
|
|
various loops in addr_validate_path_internal().
|
|
what it is.
|
|
validation_err() is an ugly macro with side effects and a goto in it.
At the cost of a few lines of code we can turn this into a function
where the side effects are explicit and ret is now explicitly set in
the main body of addr_validate_path_internal().
We get to a point where it is halfway possible to reason about the
convoluted control flow in this function.
ok inoguchi jsing
|
|
the function and unindent some code.
ok inoguchi jsing
|
|
range_should_be_prefix() currently always fails. The reason for this
is that OpenSSL commit 42d7d7dd incorrectly moved a memcmp() out of
an assertion. As a consequence, the library emits and accepts
incorrectly encoded ipAddrBlock extensions since it will never detect
ranges that MUST be encoded as a prefix according to RFC 3779, 2.2.3.7.
The return -1 from this memcmp() indicates to the callers that the
range should be expressed as a range, so callers must check beforehand
that min <= max to be able to fail. Thus, remove this memcmp() and
add a check to make_addressRange(), the only caller that didn't already
ensure that min <= max.
This fixes the noisy output in regress/lib/libcrypto/x509/rfc3779.
ok inoguchi jsing
|
|
Use child and parent instead of a and b. Split unrelated checks. Use
accessors and assign to local variables to avoid ugly line wrapping.
Declare vriables up front instead of mixing declarations with
assignments from function returns.
ok inoguchi jsing
|
|
Assign to local variables to avoid ugly line wrapping.
ok inoguchi jsing
|
|
by returning 0 instead of -1 on extract_min_max() failure. Callers
would interpret -1 as success of addr_contains().
ok inoguchi jsing
|
|
Assign repeated nested expressions to local variables and avoid some
awkward line wrapping.
|
|
things like prefixlen, afi_length, etc.
suggested by jsing
|
|
suggested by jsing during review
|
|
extract_min_max() crammed all the work in two return statements
inside a switch. Make this more readable by splitting out the
extraction of the min and max as BIT STRINGs from an addressPrefix
or an addressRange and once that's done expanding them to raw
addresses.
ok inoguchi jsing
|
|
The NULL checks and the checks that aor->type is reasonable are already
performed in extract_min_max(), so it is unnecessary to repeat them
in X509v3_addr_get_range()
ok inoguchi jsing
|
|
Instead of checking everything in a single if statement, group the
checks according to their purposes.
ok inoguchi jsing
|
|
Make the callers pass in the afi so that make_addressPrefix() can check
prefixlen to be reasonable. If the afi is anything else than IPv4 or
IPv6, cap its length at the length needed for IPv6. This way we avoid
arbitrary out-of-bounds reads if the caller decides to pass in something
stupid.
ok inoguchi jsing
|
|
IPAddressRange_new() populates both its min and max members, so
they won't ever be NULL and will never need to be allocated.
ok inoguchi jsing
|
|
IPAddressOrRange_new() instantiates a choice type, so we need to
allocate one member of the union ourselves, so aor->u.addressPrefix
will always be NULL.
ok inoguchi jsing
|
|
Replace reaching into the structs with IPAddressFamily accessors
and add a few comments that explain what the code is actually doing.
ok inoguchi jsing
|
|
Introduce a helper function that allows fetching the AFI and the
optional SAFI out of an IPAddressFamily. Also add two wrappers that
only fetch and validate the AFI, where validation currently only
means that the length is between 2 and 3.
Use these accessors throughout to simplify and streamline the code.
ok inoguchi jsing
|
|
|
|
prefer this.
|
|
This is again a straightforward conversion and leads to something which
matches our usual style more.
ok jsing
|
|
Again, we're dealing with necessarily not fully validated data here,
so a check up front seems prudent.
ok jsing
|
|
This is a more or less straightforward conversion using the new
IPAddressFamily accessor API. As a result, some checks have become
a bit stricter, which is only desirable here.
ok jsing
|
|
As mentioned in a previous commit, IPAddressFamily_cmp() can't really
check for trailing garbage in addressFamily->data. Since the path
validation and hence the X.509 validator call X509v3_addr_is_canonical(),
this deals with only partially validated data.
ok jsing
|
|
Define and use MINIMUM() instead of a ternary operator and separate
the code from the declarations. Also, we can spare a line to make the
return legible instead of squeezing it into another ternary operator.
addressFamily->data contains a two-bytes AFI and an optional one-byte
SAFI. This function currently also compares any trailing garbage that
may be present. Since comparison functions can't really error, this
needs to be checked bofore it is used. Such checks will be added in
subsequent commits.
ok jsing
|
|
ok jsing
|
|
ok jsing
|
|
Declare IPAddressFamily before using it.
|
|
|
|
unknown address family types.
Pointed out by jsing during review.
|
|
One reason why this file is hard to read are endless repetitions of
checks and assignments reaching deep inside structs. This can be made
much more readable by adding a bunch of accessors. As a first step,
we deal with IPAddressFamily, where we want to check the type of the
ipAddressChoice member, check whether the inheritance element is present
or access the addressOrRanges field.
This diff already makes minimal use of these accessors to appease -Werror.
More use and additional accessors will follow in later passes.
ok inoguchi jsing
|
|
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
|
|
|
|
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
|
