Add streaming support for PKCS7_VerifySignedData.

[TakayukiMatsuo]

Description

Extend PKCS7_VerifySignedData streaming to support PKCS7 bundle which has multiple part octet.

Fixes zd#16606

Testing

Run an application and data provided in zd#16606.

Checklist

• [x] added tests
• [ ] updated/added doxygen
• [ ] updated appropriate READMEs
• [ ] Updated manual and documentation

[TakayukiMatsuo]

Hi　@JacobBarthelmeh ,

In this PR, I made a small change to wc_PKCS7_AddDataToStream. This change resets pksc7->stream>idx to 0 after reading the input data that satisfies the argument expected. Without this modification, If pksc7->stream>idx is not reset to 0, the input data will be skipped in the next wc_PKCS7_AddDataToStream call.

I thought this was a bug, so made a change in this PR. However, this change caused errors in existing streaming test cases. Am wondering if this change is wrong, or if I'm calling wc_PKCS7_AddDataToStream incorrectly. I would be happy to hear your opinions on the changes I have made.

Thanks, Tak

[dgarske]

@TakayukiMatsuo , I see PR review feedback that is not yet fixed. Can you provide an update on the status of this PR? Are you planning to push further fixes? Thanks, David Garske

[JacobBarthelmeh]

FYI for the current failing PRB master it looks to be a build when PKCS7 streaming mode is disabled (defining the macro NO_PKCS7_STREAM):

wolfcrypt/src/pkcs7.c:5273:18: error: ‘PKCS7’ has no member named ‘stream’
 5273 |         if (pkcs7->stream->content != NULL) {

      |                  ^~

In file included from ./wolfssl/wolfcrypt/pkcs7.h:29,

                 from wolfcrypt/src/pkcs7.c:31:

wolfcrypt/src/pkcs7.c:5274:24: error: ‘PKCS7’ has no member named ‘stream’
 5274 |             XFREE(pkcs7->stream->content, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
      |                        ^~
./wolfssl/wolfcrypt/types.h:573:63: note: in definition of macro ‘XFREE’

Not as sure on the krb5 actions test, it would need farther investigation.

[TakayukiMatsuo]

@douzzer , @JacobBarthelmeh and @dgarske , thank you for your help. It's been a while, but I will adjust my time to complete this PR.

[bandi13]

retest this please

[cconlon]

@TakayukiMatsuo Is this ready for re-review?

Thanks, Chris

[TakayukiMatsuo]

@cconlon , yes, it is ready.

[cconlon]

Thanks @TakayukiMatsuo, I'm assigning over to @JacobBarthelmeh for re-review. @JacobBarthelmeh, feel free to have @douzzer re-review his comments if you would like.

[TakayukiMatsuo]

Hi Jacob,

Probably need to modify the code fragment you use a bit to be able to handle the return code from wc_PKCS7_VerifySignedData, like this:

 for (z = 0; z < derSz;) {
        int sz = (z + chunkSz > derSz)? derSz - z : chunkSz;
        rc = wc_PKCS7_VerifySignedData(&pkcs7, derBuf + z, sz);
        if (rc < 0) {
            if (rc == WC_PKCS7_WANT_READ_E) {
                z += sz;
                continue;
            }
            printf("Error z:%d %d: %s\n", z, rc, wc_GetErrorString(rc));
            break;
        }
        else {
            break;
        }
    }

With this code, could get expected result with the test bundle file. Tak

[JacobBarthelmeh]

Hi Jacob,

Probably need to modify the code fragment you use a bit to be able to handle the return code from wc_PKCS7_VerifySignedData, like this:

 for (z = 0; z < derSz;) {
        int sz = (z + chunkSz > derSz)? derSz - z : chunkSz;
        rc = wc_PKCS7_VerifySignedData(&pkcs7, derBuf + z, sz);
        if (rc < 0) {
            if (rc == WC_PKCS7_WANT_READ_E) {
                z += sz;
                continue;
            }
            printf("Error z:%d %d: %s\n", z, rc, wc_GetErrorString(rc));
            break;
        }
        else {
            break;
        }
    }

With this code, could get expected result with the test bundle file. Tak

Tak, adding an early bail on error case does succeed. This likely means that the last bytes of the bundle are not being parsed though and that it is reaching the (-272) error in this case early. With indef it should only be 0's but I think we should have it confirm that they are. Adding derBuf[derSz - 2] = derBuf[derSz - 2] + 1; before verifying the bundle seems to still return success.

[bandi13]

retest this please

[TakayukiMatsuo]

Hi Jacob, added trailing zero's check.

[JacobBarthelmeh]

Hi @TakayukiMatsuo , Please double check the case of altering the last bytes in the bundle before verification.

[TakayukiMatsuo]

Hi @JacobBarthelmeh , Confirmed ASN_PARSE_E(-140) is returned when the last byte of the bundle is altered by adding derBuf[derSz - 2] = derBuf[derSz - 2] + 1; to the sample code.