Skip to content

PKCS7 streaming with encode/sign#7184

Merged
douzzer merged 7 commits intowolfSSL:masterfrom
JacobBarthelmeh:pkcs7-enc
Feb 2, 2024
Merged

PKCS7 streaming with encode/sign#7184
douzzer merged 7 commits intowolfSSL:masterfrom
JacobBarthelmeh:pkcs7-enc

Conversation

@JacobBarthelmeh
Copy link
Copy Markdown
Contributor

ZD16812

@JacobBarthelmeh JacobBarthelmeh self-assigned this Jan 29, 2024
@JacobBarthelmeh
Copy link
Copy Markdown
Contributor Author

Examples added here wolfSSL/wolfssl-examples#422

@JacobBarthelmeh JacobBarthelmeh marked this pull request as ready for review February 1, 2024 21:29
@JacobBarthelmeh
Copy link
Copy Markdown
Contributor Author

retest this please Jenkins

Copy link
Copy Markdown
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

Looks good. This is a nice addition!

I left a couple inline comments, and there's also this:

-Wconversion warnings (associated configure options at end, to let you reproduce):

[allcryptonly-Wconversion-intelasm-build] [14 of 30] [5b060453a4]
    configure...   real 0m11.097s  user 0m3.682s  sys 0m8.661s
    build...wolfcrypt/src/asn.c: In function ‘StreamOctetString’:
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3495)         if ((sz + i) > inSz) {
wolfcrypt/src/asn.c:3495:17: error: conversion to ‘word32’ {aka ‘unsigned int’} from ‘int’ may change the sign of the result [-Werror=sign-conversion]
 3495 |         if ((sz + i) > inSz) {
      |                 ^
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3496)             sz = inSz - i;
wolfcrypt/src/asn.c:3496:18: error: conversion to ‘int’ from ‘word32’ {aka ‘unsigned int’} may change the sign of the result [-Werror=sign-conversion]
 3496 |             sz = inSz - i;
      |                  ^~~~
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3499)         ret = SetOctetString(sz, tmp);
wolfcrypt/src/asn.c:3499:30: error: conversion to ‘word32’ {aka ‘unsigned int’} from ‘int’ may change the sign of the result [-Werror=sign-conversion]
 3499 |         ret = SetOctetString(sz, tmp);
      |                              ^~
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3499)         ret = SetOctetString(sz, tmp);
wolfcrypt/src/asn.c:3499:15: error: conversion to ‘int’ from ‘word32’ {aka ‘unsigned int’} may change the sign of the result [-Werror=sign-conversion]
 3499 |         ret = SetOctetString(sz, tmp);
      |               ^~~~~~~~~~~~~~
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3501)             outIdx += ret;
wolfcrypt/src/asn.c:3501:20: error: conversion to ‘word32’ {aka ‘unsigned int’} from ‘int’ may change the sign of the result [-Werror=sign-conversion]
 3501 |             outIdx += ret;
      |                    ^~
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3505)             if (ret + sz + i + outIdx > *outSz) {
wolfcrypt/src/asn.c:3505:26: error: conversion to ‘word32’ {aka ‘unsigned int’} from ‘int’ may change the sign of the result [-Werror=sign-conversion]
 3505 |             if (ret + sz + i + outIdx > *outSz) {
      |                          ^
In file included from ./wolfssl/wolfcrypt/error-crypt.h:34,
                 from wolfcrypt/src/asn.c:105:
85a8c06062 (<douzzer@wolfssl.com> 2021-10-18 23:45:02 -0500 681)             #define XMEMCPY(d,s,l)    memcpy((d),(s),(l))
./wolfssl/wolfcrypt/types.h:681:54: error: conversion to ‘size_t’ {aka ‘long unsigned int’} from ‘int’ may change the sign of the result [-Werror=sign-conversion]
  681 |             #define XMEMCPY(d,s,l)    memcpy((d),(s),(l))
      |                                                      ^~~
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3508)             XMEMCPY(tmp + ret, in + i, sz);
wolfcrypt/src/asn.c:3508:13: note: in expansion of macro ‘XMEMCPY’
 3508 |             XMEMCPY(tmp + ret, in + i, sz);
      |             ^~~~~~~
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3511)         outIdx += sz;
wolfcrypt/src/asn.c:3511:16: error: conversion to ‘word32’ {aka ‘unsigned int’} from ‘int’ may change the sign of the result [-Werror=sign-conversion]
 3511 |         outIdx += sz;
      |                ^~
5b8f3c32d2 (<jacob@wolfssl.com> 2024-01-29 10:45:55 -0700 3512)         i      += sz;
wolfcrypt/src/asn.c:3512:16: error: conversion to ‘word32’ {aka ‘unsigned int’} from ‘int’ may change the sign of the result [-Werror=sign-conversion]
 3512 |         i      += sz;
      |                ^~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:6873: wolfcrypt/src/src_libwolfssl_la-asn.lo] Error 1
make[1]: *** [Makefile:8036: all-recursive] Error 1
make: *** [Makefile:4754: all] Error 2
   real 0m3.010s  user 0m21.057s  sys 0m3.805s
    allcryptonly-Wconversion-intelasm-build fail_build
    failed config: '--enable-intelasm' '--enable-sp-math-all' '--disable-inline' '--enable-cryptonly' '--enable-all-crypto' '--disable-pwdbased' '--disable-enckeys' '--disable-scrypt' '--disable-camellia' '--disable-md2' '--disable-pkcs7' '--disable-curve25519' '--disable-ed25519' '--disable-ed25519-stream' '--disable-curve448' '--disable-ed448' '--disable-ed448-stream' '--disable-examples' '--disable-benchmark' '--disable-crypttests' 'CC=gcc' 'CPPFLAGS=-Wconversion'

Notice --disable-pkcs7 is in the option list. @dgarske and I never got pkcs7 cleaned up for -Wconversion, so these warnings are just around the new code in asn.c, which isn't gated on HAVE_PKCS7.

Comment thread wolfcrypt/src/asn.c Outdated
Comment thread wolfcrypt/src/asn.c Outdated
Copy link
Copy Markdown
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

LGTM. tested with super-quick-check.

@douzzer douzzer merged commit 7823acb into wolfSSL:master Feb 2, 2024
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.

3 participants