configure.ac: Add in --enable-rpk option #7379

mrdeep1 · 2024-03-30T14:08:10Z

Description

By default RPK (RFC7250) support is not enabled, but is enabled when
--enable-rpk, --enable-all or --enable-dist is used.

Makes use of the HAVE_RPK compile time option.

Note: should not be merged until after #7375 is merged.

Testing

Checking that the CFLAGS -DHAVE_RPK option is created when --enable-rpk, --enable-all or --enable-dist options are used, but not set otherwise.

Checklist

added tests
updated/added doxygen
updated appropriate READMEs
Updated manual and documentation

wolfSSL-Bot · 2024-03-30T14:09:03Z

Can one of the admins verify this patch?

dgarske · 2024-04-01T15:27:30Z

Thank you @mrdeep1 . Okay to test. I see this is related to #7375. I do not see a contributor agreement on file yet. I think you are working with Kareem on this.

mrdeep1 · 2024-04-01T16:09:21Z

Please see https://wolfssl.zendesk.com/hc/requests/17707 for a copy of my Contributor Agreement.

dgarske · 2024-04-01T16:23:52Z

Thanks @mrdeep1 we will work on getting your contributor agreement processed. I'm assigning this ticket to @anhu for now.

anhu · 2024-04-01T16:46:23Z

Hello @mrdeep1, I noticed a clang tidy error with your change so added a fix. That said, I'm not sure my fix was valid. I will have other members of our team have a look at my work as a double check.

anhu · 2024-04-01T16:59:17Z

By the way, I can confirm that the Contributor Agreement was accepted.

anhu · 2024-04-01T19:00:53Z

@mrdeep1 ,
I hope you don't mind that I directly pushed commits to you PR. All tests are now passing.
@wolfSSL-Bot , Contributor agreement has been accepted and this PR is now ready for a second look and merge if satisfactory.

Warm regards, Anthony

mrdeep1 · 2024-04-01T19:35:46Z

@anhu No issues with the additional changes that you have made in the clang tidy up found in other code given that this PR now enables HAVE_RPK by default. Thanks for sorting that out.

mrdeep1 · 2024-04-01T19:37:51Z

I don't know if you need the commits squashed into a single commit.

dgarske · 2024-04-01T15:28:06Z

configure.ac

@@ -8211,6 +8212,13 @@ AC_ARG_ENABLE([dual-alg-certs],

 AS_IF([ test "$ENABLED_DUAL_ALG_CERTS" != "no" && test "$ENABLED_EXPERIMENTAL" != "yes" ],[ AC_MSG_ERROR([dual-alg-certs requires --enable-experimental.]) ])

+# Adds functionality to support RPK
+AC_ARG_ENABLE([rpk],
+    [AS_HELP_STRING([--enable-rpk],[Enable support for RPK (default: disabled)])],


Please expand description for RPK to "Raw Public Key (RPC) RFC7250"

dgarske · 2024-04-01T19:39:33Z

src/tls.c

        ret = (int)(OPAQUE8_LEN + cnt * OPAQUE8_LEN);
    }
    else if (msgType == server_hello || msgType == encrypted_extensions) {
        /* sever side */
-        cnt = ssl->options.rpkState.sending_ClientCertTypeCnt;/* must be one */


I'm not sure removing this information about the ssl->options.rpkState.sending_ClientCertTypeCnt must be one is good. Can you at least leave a comment in there?

Added comment, as well as sanity check that the Cert type actually is RPK as per RFC7250.
Code changes pushed.

douzzer

@mrdeep1 We just merged another PR (#7395) that addresses many of the same defects, and even one of the same comment spelling errors, due to duplicated/uncoordinated efforts. Can you rebase your PR on latest master and resolve the conflicts? Thanks!

mrdeep1 · 2024-04-05T20:27:48Z

@douzzer Rebased, merge errors fixed and code pushed. Various commits squashed. Tidied further the code @anhu changed for clang tidy issues.

mrdeep1 · 2024-04-20T14:36:34Z

@douzzer @anhu I have removed the clang tidy updates as they have all been fixed in #7395 so the only change now is in configure.ac. Code pushed.

dgarske · 2024-04-23T19:23:49Z

Okay to test

mrdeep1 · 2024-04-23T20:10:40Z

Do I need a login to Jenkins to look at the PRB-master-job failure?

dgarske · 2024-04-23T20:17:51Z

Sorry we will have to post the results here:

[all-c89-clang-tidy] [1 of 1] [e8426ea6f3]
    autogen.sh e8426ea6f3...   real 0m18.836s  user 0m16.959s  sys 0m1.042s
    configure...   real 0m44.523s  user 0m29.887s  sys 0m17.768s
    build.../var/lib/jenkins/workspace/PRB-multi-test-script/wolfssl/tests/api.c:68808:9: warning: mixing declarations and code is a C99 extension [clang-diagnostic-declaration-after-statement]
int isServer = 0;
^

Looks like this change is part of the original PR and is a mixed declaration. Would you mind applying this patch to the PR?

% git diff
diff --git a/tests/api.c b/tests/api.c
index ad07619f7..b07b232fb 100644
--- a/tests/api.c
+++ b/tests/api.c
@@ -68186,6 +68186,9 @@ static int test_tls13_rpk_handshake(void)
     int typeCnt_c;
     int typeCnt_s;
     int tp;
+#if defined(WOLFSSL_ALWAYS_VERIFY_CB)
+    int isServer;
+#endif

     (void)err;
     (void)typeCnt_c;
@@ -68804,8 +68807,9 @@ static int test_tls13_rpk_handshake(void)
     ExpectIntEQ(wolfSSL_set_server_cert_type(ssl_s, certType_s, typeCnt_s),
                                                         WOLFSSL_SUCCESS);

+
     /* set certificate verify callback to both client and server */
-    int isServer = 0;
+    isServer = 0;
     wolfSSL_SetCertCbCtx(ssl_c, &isServer);
     wolfSSL_set_verify(ssl_c, SSL_VERIFY_PEER, MyRpkVerifyCb);

Thanks,
David Garske, wolfSSL

By default RPK (RFC7250) support is not enabled, but is enabled when --enable-rpk, --enable-all or --enable-dist is used. Makes use of the HAVE_RPK compile time option. Fix clang issue reported in tests/api.c during test suites

dgarske · 2024-04-23T20:40:40Z

Okay to test

mrdeep1 · 2024-04-24T09:55:44Z

Need to understand the new issue thrown up by PRB-master-job failure

dgarske · 2024-04-24T14:53:11Z

It was a FIPS test failure and test results were already removed from history. Jenkins retest this please.

Changes made as requested in review.

configure.ac: Add in --enable-rpk option

dgarske assigned wolfSSL-Bot and kareem-wolfssl Apr 1, 2024

dgarske assigned anhu Apr 1, 2024

anhu force-pushed the enable-rpk branch from 39b6a1a to c3fcca8 Compare April 1, 2024 16:50

anhu requested a review from wolfSSL-Bot April 1, 2024 16:58

anhu unassigned anhu and kareem-wolfssl Apr 1, 2024

dgarske requested changes Apr 1, 2024

View reviewed changes

dgarske assigned anhu Apr 1, 2024

douzzer previously requested changes Apr 5, 2024

View reviewed changes

mrdeep1 force-pushed the enable-rpk branch from c2e06bf to 4f035c8 Compare April 5, 2024 20:22

mrdeep1 requested a review from dgarske April 5, 2024 20:26

dgarske requested review from douzzer and anhu April 16, 2024 20:07

dgarske assigned douzzer Apr 16, 2024

mrdeep1 force-pushed the enable-rpk branch from 4f035c8 to e20b0cd Compare April 20, 2024 14:32

dgarske assigned mrdeep1 and dgarske and unassigned anhu and douzzer Apr 23, 2024

configure.ac: Add in --enable-rpk option

4a1df83

By default RPK (RFC7250) support is not enabled, but is enabled when --enable-rpk, --enable-all or --enable-dist is used. Makes use of the HAVE_RPK compile time option. Fix clang issue reported in tests/api.c during test suites

mrdeep1 force-pushed the enable-rpk branch from e20b0cd to 4a1df83 Compare April 23, 2024 20:32

dgarske approved these changes Apr 23, 2024

View reviewed changes

mrdeep1 requested a review from dgarske April 24, 2024 13:33

dgarske approved these changes Apr 30, 2024

View reviewed changes

dgarske assigned SparkiDev and unassigned dgarske and mrdeep1 Apr 30, 2024

SparkiDev approved these changes Apr 30, 2024

View reviewed changes

SparkiDev requested review from douzzer and removed request for anhu and douzzer April 30, 2024 22:42

SparkiDev merged commit 72d4996 into wolfSSL:master Apr 30, 2024
114 checks passed

SparkiDev removed the request for review from wolfSSL-Bot April 30, 2024 22:44

mrdeep1 deleted the enable-rpk branch May 1, 2024 09:02

jefferyq2 pushed a commit to jefferyq2/wolfssl that referenced this pull request Jun 9, 2024

Merge pull request wolfSSL#7379 from mrdeep1/enable-rpk

3c25683

configure.ac: Add in --enable-rpk option

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

configure.ac: Add in --enable-rpk option #7379

configure.ac: Add in --enable-rpk option #7379

mrdeep1 commented Mar 30, 2024

wolfSSL-Bot commented Mar 30, 2024

dgarske commented Apr 1, 2024

mrdeep1 commented Apr 1, 2024

dgarske commented Apr 1, 2024

anhu commented Apr 1, 2024 •

edited

Loading

anhu commented Apr 1, 2024

anhu commented Apr 1, 2024

mrdeep1 commented Apr 1, 2024

mrdeep1 commented Apr 1, 2024

dgarske Apr 1, 2024

mrdeep1 Apr 1, 2024

dgarske Apr 1, 2024

mrdeep1 Apr 1, 2024

douzzer left a comment

mrdeep1 commented Apr 5, 2024

mrdeep1 commented Apr 20, 2024

dgarske commented Apr 23, 2024

mrdeep1 commented Apr 23, 2024

dgarske commented Apr 23, 2024

dgarske commented Apr 23, 2024

mrdeep1 commented Apr 24, 2024

dgarske commented Apr 24, 2024

configure.ac: Add in --enable-rpk option #7379

configure.ac: Add in --enable-rpk option #7379

Conversation

mrdeep1 commented Mar 30, 2024

Description

Testing

Checklist

wolfSSL-Bot commented Mar 30, 2024

dgarske commented Apr 1, 2024

mrdeep1 commented Apr 1, 2024

dgarske commented Apr 1, 2024

anhu commented Apr 1, 2024 • edited Loading

anhu commented Apr 1, 2024

anhu commented Apr 1, 2024

mrdeep1 commented Apr 1, 2024

mrdeep1 commented Apr 1, 2024

dgarske Apr 1, 2024

Choose a reason for hiding this comment

mrdeep1 Apr 1, 2024

Choose a reason for hiding this comment

dgarske Apr 1, 2024

Choose a reason for hiding this comment

mrdeep1 Apr 1, 2024

Choose a reason for hiding this comment

douzzer left a comment

Choose a reason for hiding this comment

mrdeep1 commented Apr 5, 2024

mrdeep1 commented Apr 20, 2024

dgarske commented Apr 23, 2024

mrdeep1 commented Apr 23, 2024

dgarske commented Apr 23, 2024

dgarske commented Apr 23, 2024

mrdeep1 commented Apr 24, 2024

dgarske commented Apr 24, 2024

anhu commented Apr 1, 2024 •

edited

Loading