Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better macro guarding fix undeclared var error #7799

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

anhu
Copy link
Member

@anhu anhu commented Jul 26, 2024

No description provided.

@anhu anhu requested a review from wolfSSL-Bot July 26, 2024 19:50
@anhu anhu self-assigned this Jul 26, 2024
@anhu
Copy link
Member Author

anhu commented Jul 26, 2024

retest this please

@@ -1855,7 +1855,8 @@ int AddSessionToCache(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* addSession,
WOLFSSL_MSG("Hash session failed");
#ifdef HAVE_SESSION_TICKET
XFREE(ticBuff, NULL, DYNAMIC_TYPE_SESSION_TICK);
#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_TICKET_NONCE_MALLOC)
#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_TICKET_NONCE_MALLOC) && \
Copy link
Contributor

Choose a reason for hiding this comment

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

@anhu please share the steps to reproduce this error...

Copy link
Contributor

Choose a reason for hiding this comment

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

Odd that FIPS is different here since this is outside the boundary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reproducible with ./configure --enable-curl --enable-fips=v5

Copy link
Contributor

Choose a reason for hiding this comment

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

I see four places in this AddSessionToCache function where XFREE(preallocNonce, addSession->heap, DYNAMIC_TYPE_SESSION_TICK); is used. Only one of them is wrapped with FIPS check... Should the others also? What's the error when you reproduce? Again I am surprised this is related to FIPS and not a specific build macro since its is NOT in the wolfCrypt FIPS boundary.

Copy link
Contributor

Choose a reason for hiding this comment

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

The error:

  CC       src/libwolfssl_la-wolfio.lo
  CC       src/libwolfssl_la-keys.lo
  CC       src/libwolfssl_la-ssl.lo
  CC       src/libwolfssl_la-tls.lo
  CC       src/libwolfssl_la-tls13.lo
  CC       src/libwolfssl_la-ocsp.lo
  CC       src/libwolfssl_la-crl.lo
  CC       wolfcrypt/test/test.o
  CC       examples/benchmark/tls_bench.o
  CC       examples/client/client-client.o
In file included from src/ssl.c:197:
./src/ssl_sess.c:1859:15: error: use of undeclared identifier 'preallocNonce'
        XFREE(preallocNonce, addSession->heap, DYNAMIC_TYPE_SESSION_TICK);
              ^
./src/ssl_sess.c:1870:15: error: use of undeclared identifier 'preallocNonce'
        XFREE(preallocNonce, addSession->heap, DYNAMIC_TYPE_SESSION_TICK);
              ^
  CC       examples/echoclient/echoclient.o
  CC       examples/echoserver/echoserver.o

Not sure how it is related to FIPS either but looks like it was an intentional choice by @rizlik when adding in the --enable-nonce-malloc functionality. #5593 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is intentionally guarded from FIPS < 5,3. The reason is that the bigger nonce is used for KDF functions inside the boundary, and that those functions have a hardcoded limit for the size of input key material. See

#if defined(WOLFSSL_TICKET_NONCE_MALLOC) && \
and #5593 (comment)

@lealem47 lealem47 requested a review from dgarske July 29, 2024 22:09
@@ -1855,7 +1855,8 @@ int AddSessionToCache(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* addSession,
WOLFSSL_MSG("Hash session failed");
#ifdef HAVE_SESSION_TICKET
XFREE(ticBuff, NULL, DYNAMIC_TYPE_SESSION_TICK);
#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_TICKET_NONCE_MALLOC)
#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_TICKET_NONCE_MALLOC) && \
Copy link
Contributor

Choose a reason for hiding this comment

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

I see four places in this AddSessionToCache function where XFREE(preallocNonce, addSession->heap, DYNAMIC_TYPE_SESSION_TICK); is used. Only one of them is wrapped with FIPS check... Should the others also? What's the error when you reproduce? Again I am surprised this is related to FIPS and not a specific build macro since its is NOT in the wolfCrypt FIPS boundary.

@douzzer douzzer merged commit e9d820b into wolfSSL:master Sep 12, 2024
133 checks passed
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.

6 participants