This was intended to detect aborted operations, but now that case is handled
by the caller freeing the restart context.
Also, as the internal sub-context is managed by the callee, no need for the
caller to free/reset the restart context between successful calls.
Following discussion in the team, it was deemed preferable for the restart
context to be explicitly managed by the caller.
This commits in the first in a series moving in that directly: it starts by
only changing the public API, while still internally using the old design.
Future commits in that series will change to the new design internally.
The test function was simplified as it no longer makes sense to test for some
memory management errors since that responsibility shifted to the caller.
In case of argument change, freeing everything is not the most efficient
(wastes one free()+calloc()) but makes the code simpler, which is probably
more important here
We'll need to store MPIs and other things that allocate memory in this
context, so we need a place to free it. We can't rely on doing it before
returning from ecp_mul() as we might return MBEDTLS_ERR_ECP_IN_PROGRESS (thus
preserving the context) and never be called again (for example, TLS handshake
aborted for another reason). So, ecp_group_free() looks like a good place to
do this, if the restart context is part of struct ecp_group.
This means it's not possible to use the same ecp_group structure in different
threads concurrently, but:
- that's already the case (and documented) for other reasons
- this feature is precisely intended for environments that lack threading
An alternative option would be for the caller to have to allocate/free the
restart context and pass it explicitly, but this means creating new functions
that take a context argument, and putting a burden on the user.
Our current behaviour is a bit inconsistent here:
- when the bad signature is made by a trusted CA, we stop here and don't
include the trusted CA in the chain (don't call vrfy on it)
- otherwise, we just add NOT_TRUSTED to the flags but keep building the chain
and call vrfy on the upper certs
This ensures that the callback can actually clear that flag, and that it is
seen by the callback at the right level. This flag is not set at the same
place than others, and this difference will get bigger in the upcoming
refactor, so let's ensure we don't break anything here.
When a trusted CA is rolling its root keys, it could happen that for some
users the list of trusted roots contains two versions of the same CA with the
same name but different keys. Currently this is supported but wasn't tested.
Note: the intermediate file test-ca-alt.csr is commited on purpose, as not
commiting intermediate files causes make to regenerate files that we don't
want it to touch.
As we accept EE certs that are explicitly trusted (in the list of trusted
roots) and usually look for parent by subject, and in the future we might want
to avoid checking the self-signature on trusted certs, there could a risk that we
incorrectly accept a cert that looks like a trusted root except it doesn't
have the same key. This test ensures this will never happen.
The tests cover chains of length 0, 1 and 2, with one error, located at any of
the available levels in the chain. This exercises all three call sites of
f_vrfy (two in verify_top, one in verify_child). Chains of greater length
would not cover any new code path or behaviour that I can see.
So far there was no test ensuring that the flags passed to the vrfy callback
are correct (ie the flags for the current certificate, not including those of
the parent).
Actual tests case making use of that test function will be added in the next
commit.
With cmake, CFLAGS has to be set when invoking cmake, not make (which totally
ignores the value of CFLAGS when it runs and only keeps the one from cmake).
Also, in that case the flags were either redundant (-Werror etc) or wrong
(-std=c99 -pedantic) as some parts of the library will not build with
-pedantic (see the other -pedantic tests, which are correct, for what needs to
be disabled).
This is step 1 of a plan to get rid once and for all of missing depends_on in
the X509 test suite (step 2 will be RSA/ECDSA, and step 0 was curves.pl).
We have code to skip them but didn't have explicit tests ensuring they are
(the corresponding branch was never taken).
While at it, remove extra copy of the chain in server10*.crt, which was
duplicated for no reason.
This shows inconsistencies in how flags are handled when callback fails:
- sometimes the flags set by the callback are transmitted, sometimes not
- when the cert if not trusted, sometimes BADCERT_NOT_TRUSTED is set,
sometimes not
This adds coverage for 9 lines and 9 branches. Now all lines related to
callback failure are covered.
Now all checks related to profile are covered in:
- verify_with_profile()
- verify_child()
- verify_top()
(that's 10 lines that were previously not covered)
Leaving aside profile enforcement in CRLs for now, as the focus is on
preparing to refactor cert verification.
Previously flags was left to whatever value it had before. It's cleaner to
make sure it has a definite value, and all bits set looks like the safest way
for when it went very wrong.
The tests only work for a specific number for MBEDTLS_X509_MAX_INTERMEDIATE_CA
so the check has been changed to confirm the default value, and to show an error
otherwise.
The test for MBEDTLS_NO_UDBL_DIVISION wasn't preserving it's own config.h
for the next test.
Also added comments to ARM Compiler 6 tests to better explain them.
