Commit graph

5535 commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard
24be79588d Group related code together 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
4b2336d7f6 Move some more code to new function 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
22be635d13 Re-order some more code 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
ec5606ad0c Extract code to separate function
ecp_mul_comb() is already 110 lines long and we're going to add complexity
with the early-return+restart code, so let's try to make it simpler first.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
7306dff01f Group related code together
This will be split to a new function next.
2017-08-09 11:44:53 +02:00
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