fixed minor length of data bug #1
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Steven Cooreman <[email protected]>
Signed-off-by: Steven Cooreman <[email protected]>
Signed-off-by: Steven Cooreman <[email protected]>
Signed-off-by: Steven Cooreman <[email protected]>
Signed-off-by: Steven Cooreman <[email protected]>
Inline with review comments. Signed-off-by: Steven Cooreman <[email protected]>
Pass the "certificate policies" extension to the callback supplied to mbedtls_x509_crt_parse_der_with_ext_cb() if it contains unsupported policies. This allows the callback to fully replicate the behaviour of the deprecated MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION configuration. Signed-off-by: Nicola Di Lieto <[email protected]>
as suggested in Mbed-TLS#3419 (comment) also removed two no longer necessary void casts Signed-off-by: Nicola Di Lieto <[email protected]>
CTR-DRBG and HMAC-DRBG may used the seed differently depending on its length. To avoid leaks, pass them a constant-length seed. Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
Define _POSIX_C_SOURCE to be 200112L, as a minimum for C99.
Signed-off-by: Steven Cooreman <[email protected]>
[Forward-port] Add Apache-2.0 headers to all source files
While this is a static function, so right now we know we don't need the check, things may change in the future, so better be on the safe side. Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
Signed-off-by: nia <[email protected]>
…parameters Remove Dangerous Parameter Passing
Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
…inistic Make basic-build-test.sh more deterministic
Align declaration of ./include include directory among libraries, static and shared. Signed-off-by: Ronald Cron <[email protected]>
Compile everest code only if MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED is defined in config.h Signed-off-by: Ronald Cron <[email protected]>
Add the possibility to distinguish between public and non-public include directories. Public directories are the one to use to access definitions of 3rd party code interfaces. Signed-off-by: Ronald Cron <[email protected]>
Don't define anymore globally third party include directories and compile definitions. Declare them within the scope of the crypto library target as per the third party source files. Note that targets linking to the crypto library inherit from the third party public include directories. Signed-off-by: Ronald Cron <[email protected]>
Use internal RNG in ecp_mul when none was provided
Remove the declaration of ./include and ./library as include directories for all targets. Prefer being more local and declare include directories at the target level using target_include_directories(). Note that there is no need to declare explicitely ./include as an include directory for tests as they inherit it from the "mbed librairies". Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
NetBSD 9.0 build fixes
as requested, see Mbed-TLS#3419 (comment) Signed-off-by: Nicola Di Lieto <[email protected]>
Pass "certificate policies" extension to callback
Lucky 13: just use starts/finish around calls to process()
Use all.sh and its component list in pre-push hook
Signed-off-by: nia <[email protected]>
We expect PSA_MAC_FINAL_SIZE to be exact in this implementation, so check it here. Signed-off-by: Gilles Peskine <[email protected]>
Rely on Asan to detect a potential buffer overflow, instead of doing a manual check. This makes the code simpler and Asan can detect underflows as well as overflows. Signed-off-by: Gilles Peskine <[email protected]>
Test psa_mac_sign_finish with a smaller or larger buffer. Signed-off-by: Gilles Peskine <[email protected]>
An early draft of the PSA crypto specification required multipart operations to keep working after destroying the key. This is no longer the case: instead, now, operations are guaranteed to fail. Mbed TLS does not comply yet, and still allows the operation to keep going. Stop testing Mbed TLS's non-compliant behavior. Signed-off-by: Gilles Peskine <[email protected]>
Add negative tests for psa_mac_verify_finish: too large, too small, or a changed byte. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Return a name that more clearly returns nonzero=true=good, 0=bad. We'd normally expect check_xxx to return 0=pass, nonzero=fail so check_parity was a bad name. Signed-off-by: Gilles Peskine <[email protected]>
The documentation had the boolean meaning of the return value inverted. Signed-off-by: Gilles Peskine <[email protected]>
…tive-tests PSA: add negative MAC tests
…ithout_time Always revoke certificate on CRL
…-cleanups-202008 Minor fixes in PSA code and tests
MBEDTLS_HAVE_TIME is documented as: "System has time.h and time()." If that is not defined, do not attempt to include time.h. A particular problem is platform-time.h, which should only be included if MBEDTLS_HAVE_TIME is defined, which makes everything messier. Maybe it should be refactored to have the check inside the header. Signed-off-by: Daniel Axtens <[email protected]>
Signed-off-by: Daniel Axtens <[email protected]>
Signed-off-by: Raoul Strackx <[email protected]> [dja: add some more fixes] Signed-off-by: Daniel Axtens <[email protected]>
To be able to test utility programs for an absence of time.h, we need a baremetal config that is not crypto only. Add one. Signed-off-by: Daniel Axtens <[email protected]>
PKCS7 signing format is used by OpenPOWER Key Management, which is using mbedtls as its crypto library. This patch adds the limited support of pkcs7 parser and verification to the mbedtls. The limitations are: * Only signed data is supported. * CRLs are not currently handled. * Single signer is supported. Signed-off-by: Daniel Axtens <[email protected]> Signed-off-by: Nayna Jain <[email protected]>
Some libraries (for eg. openssl) support writing PKCS7 signature in the full PKCS7 format or just the signed data. The original code supports only parsing full PKCS7 format, which resulted error in parsing signatures generated by tools writing only signed data. This patch adds the support to parse either full PKCS7 format or only its content type. If the buffer is only the content type, the OID is added by parsing code based on the successful parsing of the content. The return types are modified now to return either the content type or a negative error. Signed-off-by: Nayna Jain <[email protected]>
There was copy-paste error of wrong comments in PKCS7 oid section. This patch corrects them. Signed-off-by: Nayna Jain <[email protected]>
…tive Not important either way. Signed-off-by: Daniel Axtens <[email protected]>
Signed-off-by: Daniel Axtens <[email protected]>
Signed-off-by: Daniel Axtens <[email protected]>
RFC s 9.1:
o signerInfos is a collection of per-signer
information. There may be any number of elements in the
collection, including zero.
Signed-off-by: Daniel Axtens <[email protected]>
Signed-off-by: Daniel Axtens <[email protected]>
daxtens
pushed a commit
that referenced
this pull request
Nov 18, 2020
The previous code used comparison operators >= and == that are quite likely to
be compiled to branches by some compilers on some architectures (with some
optimisation levels).
For example, take the following function:
void old_update( size_t data_len, size_t *padlen )
{
*padlen *= ( data_len >= *padlen + 1 );
}
With Clang 3.8, let's compile it for the Arm v6-M architecture:
% clang --target=arm-none-eabi -march=armv6-m -Os foo.c -S -o - |
sed -n '/^old_update:$/,/\.size/p'
old_update:
.fnstart
@ BB#0:
.save {r4, lr}
push {r4, lr}
ldr r2, [r1]
adds r4, r2, #1
movs r3, #0
cmp r4, r0
bls .LBB0_2
@ BB#1:
mov r2, r3
.LBB0_2:
str r2, [r1]
pop {r4, pc}
.Lfunc_end0:
.size old_update, .Lfunc_end0-old_update
We can see an unbalanced secret-dependant branch, resulting in a total
execution time depends on the value of the secret (here padlen) in a
straightforward way.
The new version, based on bit operations, doesn't have this issue:
new_update:
.fnstart
@ BB#0:
ldr r2, [r1]
subs r0, r0, #1
subs r0, r0, r2
asrs r0, r0, Mbed-TLS#31
bics r2, r0
str r2, [r1]
bx lr
.Lfunc_end1:
.size new_update, .Lfunc_end1-new_update
(As a bonus, it's smaller and uses less stack.)
While there's no formal guarantee that the version based on bit operations in
C won't be translated using branches by the compiler, experiments tend to show
that's the case [1], and it is commonly accepted knowledge in the practical
crypto community that if we want to sick to C, bit operations are the safest
bet [2].
[1] https://github.com/mpg/ct/blob/master/results
[2] https://github.com/veorq/cryptocoding
Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
daxtens
force-pushed
the
dja-pkcs7-for-slof
branch
3 times, most recently
from
ab076e8
to
16babb5
Compare
daxtens
force-pushed
the
dja-pkcs7-for-slof
branch
from
16babb5
to
23126e9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey Daniel,
A bug came up when printing the length of pkcs7->signer->serial when the length of serial is 0. Since it is an unsigned integer, the length - 1 is a large number, leading to a seg fault. By casting to a signed integer, the conditional in the for loop will never pass leading to correct execution and a proper parsing failure. This type of bug could be something that comes up in the code more than once. This particular issue has come to my attention through afl fuzzing. I will let you know if anything else comes up.
Hope this helps