Skip to content

Allow limited ecc_verify operations when compiled with NO_ASN#3

Closed
danielinux wants to merge 2 commits intodgarske:spcortexfrom
danielinux:sp-cortexm-ecc-verify-no-asn
Closed

Allow limited ecc_verify operations when compiled with NO_ASN#3
danielinux wants to merge 2 commits intodgarske:spcortexfrom
danielinux:sp-cortexm-ecc-verify-no-asn

Conversation

@danielinux
Copy link

HAVE_ECC_VERIFY can be now compiled in combination with NO_ASN to allow limited access to the DSA verification interface

Copy link
Owner

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

/* Make sure ASN is enabled for ECC sign/verify */
#if (defined(HAVE_ECC_SIGN) || defined(HAVE_ECC_VERIFY)) && defined(NO_ASN)
#if (defined(HAVE_ECC_SIGN) && defined(NO_ASN))
#error ASN must be enabled for ECC sign/verify
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have NO_ASN wrappers below around the sign_hash and verify_hash I am not sure this section is even required.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the section entirely

#endif

#ifdef NO_ASN
# define ECC_SECP256R1_OID 526
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its probably better to expose these in asn_public.h even with NO_ASN is defined. I suspect we'll need other OID's.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the whole enum Ecc_Sum to asn_public.h

}

#if defined(WOLFSSL_VALIDATE_ECC_KEYGEN) || defined(HAVE_ECC_SIGN)
#if defined(WOLFSSL_VALIDATE_ECC_KEYGEN) || defined(HAVE_ECC_SIGN) || defined(HAVE_ECC_VERIFY)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure Sean has this updated in his SP scripts. https://github.com/wolfSSL/scripts/pull/24

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mailed Sean about it 👍

@danielinux
Copy link
Author

I know this PR is still in the wrong branch, but if the approach looks good now, I'll prepare a similar one for master

Copy link
Owner

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielinux : Looks great! Go ahead and submit in wolfSSL master. The changes in sp_cortexm depend on wolfSSL#2028 , so you might want to branch off that or wait until that is merged into master.

@dgarske dgarske closed this Jan 18, 2019
rizlik added a commit that referenced this pull request Jan 19, 2022
otherwise if profile_str_len is > strlen(gSrtpProfiles[i].name) we end up
comparing memory past gSrtpProfiles[i].name. -fsanitize=address catches this:

```
==100159==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f40d8d533b2 at pc 0x7f40d8eb014f bp 0x7f40d50fe240 sp 0x7f40d50fd9e8
READ of size 21 at 0x7f40d8d533b2 thread T107
    #0 0x7f40d8eb014e in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long) /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:860
    #1 0x7f40d8eb06e6 in __interceptor_memcmp /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:892
    #2 0x7f40d8eb06e6 in __interceptor_memcmp /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:887
    #3 0x7f40d8c2e830 in DtlsSrtpFindProfile src/ssl.c:1310
    #4 0x7f40d8c2e9ed in DtlsSrtpSelProfiles src/ssl.c:1347
    #5 0x7f40d8c2eada in wolfSSL_CTX_set_tlsext_use_srtp src/ssl.c:1359
    #6 0x563bf381b4c5 in server_test examples/server/server.c:2278
    #7 0x7f40d88f0258 in start_thread (/usr/lib/libpthread.so.0+0x9258)
    wolfSSL#8 0x7f40d88195e2 in __GI___clone (/usr/lib/libc.so.6+0xfe5e2)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants