Disable Stateful Signatures in the build by default #1676

ashman-p · 2024-01-23T07:14:17Z

This commit will disable XMSS and LMS from the build by default. When enabled key and signature generation is also disabled by default. They can be enabled by defining the following variables on the config command.

-DOQS_ENABLE_SIG_STFL_KEY_SIG_GEN=ON
-DOQS_ENABLE_SIG_STFL_XMSS = ON
-DOQS_ENABLE_SIG_STFL_LMS = ON

Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider and OQS-OpenSSH will also need to be ready for review and merge by the time this is merged.)

When enabled, key and signature generation is disabled by default. Only signature verification is allowed. Key and signature generation can be enabled by defining OQS_ENABLE_SIG_STFL_KEY_SIG_GEN

baentsch

Thanks for adding this separator (and turning it off by default)! May I ask whether it would be possible to completely replace OQS_SIG_STFL with OQS_SIG if "OQS_ALLOW_SFTL_KEY_AND_SIG_GEN" is not set?

SWilson4

Thanks for putting this together, Norm. Just a few typo/formatting fixes from me and one suggestion regarding test coverage.

.CMake/alg_support.cmake

CONFIGURE.md

SWilson4 · 2024-01-23T14:13:21Z

tests/kat_sig_stfl.c

@@ -297,7 +298,44 @@ OQS_STATUS sig_stfl_kat(const char *method_name, const char *katfile) {

 	ret = OQS_SUCCESS;
 	goto cleanup;
+#else
+	/*
+	 * Signature generation is disabled so only signature verification can be tested.


I know that we don't really have any "unhappy path" tests in liboqs, but I think they would be useful here as a sanity check to make sure that calling keygen / sign does, in fact, result in an error when those operations are disabled in the config.

Thanks. Will do this on another test since the KAT test compares hashes of the expected output to determine success status of the test.

Incorporated some of this in test_sig_stfl.c.

Co-authored-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>

ashman-p · 2024-01-24T19:34:34Z

Thanks for adding this separator (and turning it off by default)! May I ask whether it would be possible to completely replace OQS_SIG_STFL with OQS_SIG if "OQS_ALLOW_SFTL_KEY_AND_SIG_GEN" is not set?

Hi, could you give an example or describe more about how that would look? Thanks.

Fixed compile error, unused function. Added a negative test when stateful signature is disabled.

baentsch · 2024-01-25T07:04:24Z

Thanks for adding this separator (and turning it off by default)! May I ask whether it would be possible to completely replace OQS_SIG_STFL with OQS_SIG if "OQS_ALLOW_SFTL_KEY_AND_SIG_GEN" is not set?

Hi, could you give an example or describe more about how that would look? Thanks.

Sure, my question is whether it would be possible (for the rest of the code to continue working) when completely wrapping the current definition for OQS_SIG_STFL in "sig_stfl.h" in an #ifdef OQS_ALLOW_SFTL_KEY_AND_SIG_GEN and

#define OQS_SIG_STFL OQS_SIG

in the #else (#ifndef OQS_ALLOW_SFTL_KEY_AND_SIG_GEN) branch.

baentsch · 2024-01-26T17:03:26Z

@ashman-p I've started fleshing out my proposal above in the branch "mb-stfl-no-sigkeygen", so see e.g.

liboqs/src/sig_stfl/sig_stfl.h

Lines 181 to 183 in 1499916

    
           #ifndef OQS_ALLOW_SFTL_KEY_AND_SIG_GEN 
        
           #define OQS_SIG_STFL OQS_SIG 
        
           #else

...

Question: What code generator did you use to create the many variants? It's tedious to manually #ifdef the now apparently unneeded replicate references to "oid", "sigs_remaining", "sigs_total"... I started this in the branch to see whether compilation succeeds -- it does: Those structs are apparently only needed in the disabled (under unset OQS_ALLOW_SFTL_KEY_AND_SIG_GEN) keygen and sign routines... So now I wonder whether there's a master template that can be adapted and used to generate the many xmss.c files(?).

If this were successful, XMSS/LMS algs might indeed be used (for verification) just like any other OQS_SIG & no downstream changes appear to be necessary to use this code right out of the box (only if configured without sig/gen).

baentsch · 2024-01-28T16:31:30Z

I hate commenting my own comment but I have to:

So now I wonder whether there's a master template that can be adapted and used to generate the many xmss.c files

After understanding the code a bit better I'd now see a much simpler solution, doing away with a possible generator and dozens of files and compile jobs, namely #defines for the ...new etc functions. What is the reason this has not been done, @ashman-p ? This would make the code much easier to maintain, adapt and faster to build, no? Example:

// macro to en/disable OQS_SIG_STFL-only structs used only in sig&gen case:
#ifdef OQS_ALLOW_SFTL_KEY_AND_SIG_GEN
#define XMSS_SIGGEN(designator) \
        sig->oid = OQS_SIG_STFL_alg_xmss##designator##_oid; \
        sig->sigs_remaining = OQS_SIG_STFL_alg_xmss##designator##_sigs_remaining;\
        sig->sigs_total = OQS_SIG_STFL_alg_xmss##designator##_sigs_total;
#else
#define XMSS_SIGGEN(designator)
#endif

// generator for all alg-specific functions:
#define XMSS_ALG(mt, designator) \
OQS_SIG_STFL *OQS_SIG_STFL_alg_xmss##designator##_new(void) { \
\
        OQS_SIG_STFL *sig = (OQS_SIG_STFL *)malloc(sizeof(OQS_SIG_STFL)); \
        if (sig == NULL) { \
                return NULL; \
        } \
        memset(sig, 0, sizeof(OQS_SIG_STFL)); \
\
        XMSS_SIGGEN(designator) \
        sig->method_name = OQS_SIG_STFL_alg_xmss##designator; \
        sig->alg_version = "https://datatracker.ietf.org/doc/html/rfc8391"; \
        sig->euf_cma = true; \
\
        sig->length_public_key = OQS_SIG_STFL_alg_xmss##designator##_length_pk; \
        sig->length_secret_key = OQS_SIG_STFL_alg_xmss##designator##_length_sk; \
        sig->length_signature = OQS_SIG_STFL_alg_xmss##designator##_length_signature; \
\
        sig->keypair = OQS_SIG_STFL_alg_xmss##designator##_keypair;\
        sig->sign = OQS_SIG_STFL_alg_xmss##designator##_sign;\
        sig->verify = OQS_SIG_STFL_alg_xmss##designator##_verify;\
\
        return sig;\
} \
\
OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_XMSS##designator##_new(void) {\
        return OQS_SECRET_KEY_XMSS_new(OQS_SIG_STFL_alg_xmss##designator##_length_sk);\
}\
\
OQS_API OQS_STATUS OQS_SIG_STFL_alg_xmss##designator##_keypair(XMSS_UNUSED_ATT uint8_t *public_key, XMSS_UNUSED_ATT OQS_SIG_STFL_SECRET_KEY *secret_key) {\
\
        if (public_key == NULL || secret_key == NULL || secret_key->secret_key_data == NULL) {\
                return OQS_ERROR;\
        }\
\
        if (xmss##mt##_keypair(public_key, secret_key->secret_key_data, OQS_SIG_STFL_alg_xmss##designator##_oid)) {\
                return OQS_ERROR;\
        }\
\
        return OQS_SUCCESS;\
}\
\
OQS_API OQS_STATUS OQS_SIG_STFL_alg_xmss##designator##_sign(uint8_t *signature, size_t *signature_len, const uint8_t *message, size_t message_len, OQS_SIG_STFL_SECRET_KEY *secret_key) {\
        return OQS_SIG_STFL_alg_xmss##mt##_sign(signature, signature_len, message, message_len, secret_key);\
}\
\
OQS_API OQS_STATUS OQS_SIG_STFL_alg_xmss##designator##_verify(const uint8_t *message, size_t message_len, const uint8_t *signature, size_t signature_len, const uint8_t *public_key) {\
        return OQS_SIG_STFL_alg_xmss##mt##_verify(message, message_len, signature, signature_len, public_key);\
}\
\
OQS_API OQS_STATUS OQS_SIG_STFL_alg_xmss##designator##_sigs_remaining(unsigned long long *remain, const OQS_SIG_STFL_SECRET_KEY *secret_key) {\
        return OQS_SIG_STFL_alg_xmss##mt##_sigs_remaining(remain, secret_key);\
}\
\
OQS_API OQS_STATUS OQS_SIG_STFL_alg_xmss##designator##_sigs_total(unsigned long long *total, const OQS_SIG_STFL_SECRET_KEY *secret_key) {\
        return OQS_SIG_STFL_alg_xmss##mt##_sigs_total(total, secret_key);\
}

// each of these lines replaces a complete file/compile target
// possibly wrapped with #ifdef OQS_ENABLE_SIG_STFL_xmss_....
XMSS_ALG(, _sha256_h16)
XMSS_ALG(mt, mt_sha256_h60_12)
....

Please let me know whether you'd want to adopt this -- or whether I'm overlooking a problem with this. If the former, let me know whether I should prepare a complete PR or you'd rather do it yourself.

ashman-p · 2024-01-29T04:49:32Z

Yes, I agree that this macro would speed things up in general.
In your proposed changes. We would also not set vector functions for 'keygen' and 'sign' since i think those signatures are different between OQS_SIG and OQS_SIG_STFL. The oid values are also not used during a verify.

I am in the process of making an update to address one of Spencer's comments and also fix a build issue.
Please let me complete that task and then revisit yours?

baentsch · 2024-01-29T05:20:38Z

We would also not set vector functions for 'keygen' and 'sign' since i think those signatures are different between OQS_SIG and OQS_SIG_STFL. The oid values are also not used during a verify.

OK, that could be facilitated easily by moving those two assignments to the proposed "XMSS_SIGGEN" macro.

Please let me complete that task and then revisit yours?

Sure. Just wanted to ask whether my thinking in general is OK for you. Thanks for the feedback.

Allow some key generation tests to run as negative tests when key and sig gen is off

ashman-p · 2024-01-29T21:44:34Z

@baentsch, Let me study this a little, get feedback from @ducnguyen-sb and run some tests to try it.
I believe it will work but, having two APIs like this feels a little strange to me. That is, using the 'stateless' API to verify a 'stateful' signature. I will also look at converting the files to use a macro where applicable.

baentsch · 2024-01-30T07:57:14Z

I believe it will work but, having two APIs like this feels a little strange to me. That is, using the 'stateless' API to verify a 'stateful' signature.

Yes, that may sound "unconventional" -- but the question seems to be: Is it logically right? And technically doable, i.e., is signature state sufficiently encoded in the signatures to permit verification? If the latter, I don't really see a mismatch: OQS_SIG then (continues to) support state-free signature operations (on all algorithms supporting that) while OQS_SIG_STFL adds functionality for stateful operations (on those algs needing/supporting this).

I will also look at converting the files to use a macro where applicable.

That would be great: I got stuck a bit trying that in the pre-existing macro machinery loading "external/xmss.c" that you probably understand much better than I do.

dstebila · 2024-01-30T13:26:01Z

I believe it will work but, having two APIs like this feels a little strange to me. That is, using the 'stateless' API to verify a 'stateful' signature.

Yes, that may sound "unconventional" -- but the question seems to be: Is it logically right? And technically doable, i.e., is signature state sufficiently encoded in the signatures to permit verification? If the latter, I don't really see a mismatch: OQS_SIG then (continues to) support state-free signature operations (on all algorithms supporting that) while OQS_SIG_STFL adds functionality for stateful operations (on those algs needing/supporting this).

It is an interesting idea... are we using the prefix OQS_SIG_ to denote the family of algorithm or a class of functionality?

In the context of key generation, both OQS_KEM_..._keygen and OQS_SIG_..._keygen have the same function signature (output a public key and secret key pair) and both are intended for the same purpose (key generation), but we prefixed them separately rather than have a generic OQS_..._keygen.

Granted, SIG and SIG_STFL are "closer" conceptually than KEM and SIG.

I don't have a conclusion at this point, but you made me think.

ashman-p · 2024-01-30T14:38:48Z

I would like to treat this proposal as a separate task. If it is OK, let me merge this branch to 'stateful-sigs' with the stated goal of disabling the feature by default and only enable stateful verification.

I will continue the investigation of using OQS_SIG interface as well as converting some macro generated functions.

baentsch · 2024-01-30T14:55:48Z

Granted, SIG and SIG_STFL are "closer" conceptually than KEM and SIG.

Kinda... but I'm not sure I even fully concur with that: Granted, SIG and SIG_STFL both contain one function with pretty much identical signature and functionality (verify), but two with different signatures (keygen and sign) and wildly different usage logic (though conceptually keygen and sig). Taking the former as a case in point: Couldn't one argue the keygen in OQS_KEM is closer logically to the one in OQS_SIG than the one in OQS_SIG_STFL? This argument can be seen "in action" in oqsprovider: The code making use of the two is very similar. The code to activate keygen of OQS_SIG_STFL in turn however would need a completely new logic and support structure (as would SIG_STFL:sign) -- both moot points.

I don't have a conclusion at this point, but you made me think.

Thanks. I in turn am also re-thinking my original proposal to activate this OQS_SIG "verify-only" API if "OQS_ALLOW_SFTL_KEY_AND_SIG_GEN" is not set: I now begin to wonder whether this should always be used: This would also make it clear to every user which of the SIG_STFL operations are "different" (not saying dangerous).

SWilson4

OK with me to merge this and continue to work on the other points in separate PR(s). Thanks, Norm.

ducnguyen-sb · 2024-01-30T18:54:12Z

Hi @ashman-p , sorry for the late reply. Please let me know if I can help in the next PR.

* Disable stateful signature as default. When enabled, key and signature generation is disabled by default. Only signature verification is allowed. Key and signature generation can be enabled by defining OQS_ENABLE_SIG_STFL_KEY_SIG_GEN * Fixed format * Address unused variables * Update .CMake/alg_support.cmake Co-authored-by: Spencer Wilson <spencer.wilson@uwaterloo.ca> * Update CONFIGURE.md Co-authored-by: Spencer Wilson <spencer.wilson@uwaterloo.ca> * Update example_sig_stfl.c Fixed compile error, unused function. Added a negative test when stateful signature is disabled. * Fix build error. Allow some key generation tests to run as negative tests when key and sig gen is off * Fix format * Fix build error * Fix build error --------- Co-authored-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>

commit e356ebf Na lms (#1486) commit 55094c3 LMS H5_W1 (#1513) commit 4d773d7 Convert to use OQS_SIG_STFL_SECRET_KEY struct (#1525) commit 245aede LMS updated to use new SK API (#1533) commit a85a9aa Stateful sigs secret key storage callback (#1553) commit 3934949 Na statful sig lock (#1559) commit 3db6b44 Secret Key Query (#1572) commit 2446c64 Na stateful sigs lms var (#1574) commit 8df2539 Stateful sigs XMSS updates (#1590) commit a7b2987 SHA2 Increment with arbitrary length (non-block sizes) (#1614) commit 2dd9e07 Na lms kat multi level (#1620) commit 982b440 Fix Build Errors (#1635) commit ddae644 Various fixes commit cc50ef0 Fix warning commit cf03392 Update README.md commit 9325713 Update README.md commit a52b217 Update README.md commit d442ac9 Update README.md commit 72ab478 Update README.md commit 5967f12 Update src/CMakeLists.txt commit fc6d512 Update documentation and license text. (#1663) commit e7a83c7 Disable Stateful Signatures in the build by default (#1676) commit 6c81bae Na stateful macro (#1687) Signed-off-by: Norman Ashley <nashley@cisco.com>

ashman-p added 3 commits January 23, 2024 01:29

Disable stateful signature as default.

4dbc9a6

When enabled, key and signature generation is disabled by default. Only signature verification is allowed. Key and signature generation can be enabled by defining OQS_ENABLE_SIG_STFL_KEY_SIG_GEN

Fixed format

e25962b

Address unused variables

7d20dc0

ashman-p requested a review from dstebila as a code owner January 23, 2024 07:14

ashman-p self-assigned this Jan 23, 2024

ashman-p requested review from baentsch, SWilson4 and ducnguyen-sb January 23, 2024 07:14

baentsch reviewed Jan 23, 2024

View reviewed changes

SWilson4 reviewed Jan 23, 2024

View reviewed changes

baentsch mentioned this pull request Jan 23, 2024

Supporting Stateful Signatures open-quantum-safe/oqs-provider#331

Open

ashman-p and others added 2 commits January 24, 2024 14:30

Update .CMake/alg_support.cmake

9c89da5

Co-authored-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>

Update CONFIGURE.md

deff896

Co-authored-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>

Update example_sig_stfl.c

7e23187

Fixed compile error, unused function. Added a negative test when stateful signature is disabled.

ashman-p added 4 commits January 29, 2024 01:53

Fix build error.

34d35cd

Allow some key generation tests to run as negative tests when key and sig gen is off

Fix format

b982601

Fix build error

89d41d6

Fix build error

79e9457

ashman-p requested review from SWilson4 and baentsch January 29, 2024 21:46

SWilson4 approved these changes Jan 30, 2024

View reviewed changes

ashman-p merged commit ada6637 into stateful-sigs Jan 30, 2024
52 checks passed

ashman-p deleted the na-stateful-default-off branch February 8, 2024 19:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable Stateful Signatures in the build by default #1676

Disable Stateful Signatures in the build by default #1676

ashman-p commented Jan 23, 2024

baentsch left a comment

SWilson4 left a comment

SWilson4 Jan 23, 2024

ashman-p Jan 25, 2024

ashman-p Jan 29, 2024

ashman-p commented Jan 24, 2024

baentsch commented Jan 25, 2024

baentsch commented Jan 26, 2024

baentsch commented Jan 28, 2024

ashman-p commented Jan 29, 2024

baentsch commented Jan 29, 2024

ashman-p commented Jan 29, 2024

baentsch commented Jan 30, 2024

dstebila commented Jan 30, 2024

ashman-p commented Jan 30, 2024

baentsch commented Jan 30, 2024

SWilson4 left a comment

ducnguyen-sb commented Jan 30, 2024

Disable Stateful Signatures in the build by default #1676

Disable Stateful Signatures in the build by default #1676

Conversation

ashman-p commented Jan 23, 2024

baentsch left a comment

Choose a reason for hiding this comment

SWilson4 left a comment

Choose a reason for hiding this comment

SWilson4 Jan 23, 2024

Choose a reason for hiding this comment

ashman-p Jan 25, 2024

Choose a reason for hiding this comment

ashman-p Jan 29, 2024

Choose a reason for hiding this comment

ashman-p commented Jan 24, 2024

baentsch commented Jan 25, 2024

baentsch commented Jan 26, 2024

baentsch commented Jan 28, 2024

ashman-p commented Jan 29, 2024

baentsch commented Jan 29, 2024

ashman-p commented Jan 29, 2024

baentsch commented Jan 30, 2024

dstebila commented Jan 30, 2024

ashman-p commented Jan 30, 2024

baentsch commented Jan 30, 2024

SWilson4 left a comment

Choose a reason for hiding this comment

ducnguyen-sb commented Jan 30, 2024