Commit graph

5480 commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard 510d5caece Add early return test + fake implementation 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard 054433c493 Add mbedtls_ecp_set_max_ops()
The plan is to count basic operations as follows:
- call to ecp_add_mixed()   -> 11
- call to ecp_double_jac()  -> 8
- call to mpi_mul_mpi()     -> 1
- call to mpi_inv_mod()     -> 120
- everything else           -> not counted

The counts for ecp_add_mixed() and ecp_double_jac() are based on the actual
number of calls to mpi_mul_mpi() they they make.

The count for mpi_inv_mod() is based on timing measurements on K64F and
LPC1768 boards, and are consistent with the usual very rough estimate of one
inversion = 100 multiplications. It could be useful to repeat that measurement
on a Cortex-M0 board as those have smaller divider and multipliers, so the
result could be a bit different but should be the same order of magnitude.

The documented limitation of 120 basic ops is due to the calls to mpi_inv_mod()
which are currently not interruptible nor planned to be so far.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard 5e3c62fd1d Add MBEDTLS_ERR_ECP_IN_PROGRESS 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard c3a3bc7636 Add config flag MBEDTLS_ECP_EARLY_RETURN 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard 225b37a543 Fix typos in comment 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard a54f6cc874 Unify name of default profile in X.509 tests 2017-08-09 10:41:42 +02:00
Manuel Pégourié-Gonnard 24611f9383 Remove redundant variable
path_cnt was always chain_len - 1 in the loop body
2017-08-09 10:28:07 +02:00
Manuel Pégourié-Gonnard 7ff243a87c Add missing dependency in test-certs Makefile 2017-08-08 18:54:13 +02:00
Manuel Pégourié-Gonnard 562df401d3 Improve some comments, fix some typos+whitespace 2017-08-08 18:17:53 +02:00
Manuel Pégourié-Gonnard ea2dc14c0c Fix some whitespace 2017-08-08 11:10:37 +02:00
Manuel Pégourié-Gonnard a7c4c8a46c Make some perl scripts usable with git bisect run
For that they need to return between 0 and 124 on error, while die returns
255, causing bisect-run to abort.
2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard 66a36b03c6 Update comments 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard 505c3953c7 Make the ver_chain length explicit 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard a707e1d1ef Extract code to separate function for readablity 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard ce6e52ff42 Make verify_chain() iterative 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard f86f491f25 Rm unneeded function arguments & update comments 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard c547d1ab1f Start using an explicit stack for callback info
This is the first step towards making verify_chain() iterative. While from a
readability point of view the current recursive version is fine, one of the
goals of this refactoring is to prepare for restartable ECC integration, which
will need the explicit stack anyway.
2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard a468eb1764 verify_name(): factor duplicated code to function 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard 1300e99eb1 Extract name checking to separate function
Just copy-paste and unindent
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 6368612a8f Move code to separate function for readability 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 27e94797aa Simplify handling of locally trusted EE certs
Though this might require one more walk of the list in some cases,
this avoid having a check for that deep inside check_parent().
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard bdc5440232 Update comments 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard cb39610093 Finally merge the remains of top() into child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 63642776b1 Let verify_top() handle only the parent
It felt wrong for it to call the vrfy callback on two certs.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 6e786747fb Move top()'s checks on child to child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 784aee3366 Move other special case from top() to child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard b9983be73a Move one special case from verify_top() to child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 66fac75f8b Merge duplicated checks between child() and top() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 58dcd2d9b2 Get rid of unused variables/arguments 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 8f8c282de9 Merge near-duplicated (grand)parent finding code
Besides avoiding near-duplication, this avoids having three generations of
certificate (child, parent, grandparent) in one function, with all the
off-by-one opportunities that come with it.

This also allows to simplify the signature of verify_child(), which will be
done in next commit.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard f82a4d5aba Factor duplicated code into function 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 32fdc60c7b Unnest code in verify_top()
We now know that trust_ca != NULL till the end of the function
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 6038cb6909 Remove duplicate parent-searching in verify_top() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 2f09d59456 Add badkey-skipping to find_parent()
This is the last step towards removing the now-duplicated parent-searching
code in verify_top()
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 3e329b8e8d Add badtime-skipping feature to new function
This is from the morally 5th (and soon obsolete) invocation of this function
in verify_top().

Doing this badtime-skipping when we search for a parent in the provided chain
is a change of behaviour, but it's backwards-compatible: it can only cause us
to accept valid chains that we used to reject before. Eg if the peer has a
chain with two version of an intermediate certificate with different validity
periods, the first non valid and the second valid - such cases are probably
rare or users would have complained already, but it doesn't hurt to handle it
properly as it allows for more uniform code.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 9c6118c498 Factor one more occurrence of code into function
This may look like a behaviour change because one check has been added to the
function that was previously done in only one of the 3 call sites. However it
is not, because:
- for the 2 call sites in verify(), the test always succeeds as path_cnt is 0.
- for the call site in verify_child(), the same test was done later anyway in
  verify_top()
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 2f1c33dc33 Factor repeated code into function
There are 3 instance that were replaced, but 2 instances of variants of this
function exist and will be handled next (the extra parameter that isn't used
so far is in preparation for that):
- one in verify_child() where path_cnt constraint is handled too
- one in verify_top() where there is extra logic to skip parents that are
  expired or future, but only if there are better parents to be found
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 17f4a6a609 Take shortcut for directly trusted EE cert
This is a slight change of behaviour in that the previous condition was:
- same subject
- signature matches
while the new condition is:
- exact same certificate

However the documentation for mbedtls_x509_crt_verify() (note on trust_ca)
mentions the new condition, so code that respected the documentation will keep
working.

In addition, this is a bit faster as it doesn't check the self-signature
(which never needs to be checked for certs in the trusted list).
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard c61e5c9304 Don't search twice for a non-existing parent 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard b8acfd2ba8 Fix calls to check_parent()
When we're looking for a parent, in trusted CAs, 'top' should be 1.

This only impacted which call site for verify_top() was chosen, and the error
was then fixed inside verify_top() by iterating over CAs again, this time
correctly setting 'top' to 1.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 35407c7764 Add comments on chain verification cases
This is the beginning of a series of commits refactoring the chain
building/verification functions in order to:
- make it simpler to understand and work with
- prepare integration of restartable ECC
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 9bc860c3ad Add test for callback and bad signatures
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
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard a656825aef Add test for bad name and callback
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.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard 2d825d42bb Add test for same CA with different keys
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.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard c10afdb322 Add test for CA forgery attempt
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.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard d092277683 Add test for profile on trusted EE cert 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard bc313017a5 Add tests for flags passed to f_vrfy
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.
2017-08-08 11:06:49 +02:00
Manuel Pégourié-Gonnard ffa42efa1c Add ability to test flags value in vrfy callback
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.
2017-08-08 11:06:49 +02:00
Manuel Pégourié-Gonnard 329e78c7fa Improve handling of md errors in X.509
md() already checks for md_info == NULL. Also, in the future it might also
return other errors (eg hardware errors if acceleration is used), so it make
more sense to check its return value than to check for NULL ourselves and then
assume no other error can occur.

Also, currently, md_info == NULL can never happen except if the MD and OID modules
get out of sync, or if the user messes with members of the x509_crt structure
directly.

This commit does not change the current behaviour, which is to treat MD errors
the same way as a bad signature or no trusted root.
2017-08-08 11:06:49 +02:00
Manuel Pégourié-Gonnard a4a206e834 Clarify documentation for directly-trusted certs
The fact that self-signed end-entity certs can be explicitly trusted by
putting them in the CA list even if they don't have the CA bit was not
documented though it's intentional, and tested by "Certificate verification #73
(selfsigned trusted without CA bit)" in test_suite_x509parse.data

It is unclear to me whether the restriction that explicitly trusted end-entity
certs must be self-signed is a good one. However, it seems intentional as it is
tested in tests #42 and #43, so I'm not touching it for now.
2017-08-08 11:06:49 +02:00