Adds encoding options for subjectPublicKey and privateKey #123

bhess · 2023-03-17T01:23:37Z

Integrates encoding subjectPublicKey and privateKey according the following IETF drafts:

https://datatracker.ietf.org/doc/draft-uni-qsckeys-dilithium/00/
https://datatracker.ietf.org/doc/draft-uni-qsckeys-falcon/00/
https://datatracker.ietf.org/doc/draft-uni-qsckeys-kyber/00/
https://datatracker.ietf.org/doc/draft-uni-qsckeys-sphincsplus/00/

Integrates encoding library from https://github.com/Quantum-Safe-Collaboration/qsc-key-rfc/tree/main/qsc-key-encoder.
The code is used as an external project with cmake ExternalProject_Add. Optionally, oqs-provider can be built without the dependency by using -DUSE_ENCODING_LIB=OFF. The library provides API to encode/decode according to the draft specification.
The encodings are specified by setting environment variables. Example: export OQS_ENCODING_DILITHIUM2=draft-uni-qsckeys-dilithium-00/sk-pk. If no env variable are set, the default is 'no' encoding (the previous oqs-provider behavior).
Supports the -DNOPUBKEY_IN_PRIVKEY build option (enabling private keys without public keys for interop testing #83).
Note: some PQC algorithms (Kyber, SPHINCS+) already contain the public key in their 'raw' private key. With -DNOPUBKEY_IN_PRIVKEY=OFF, this usually leads to an unnecessary duplication of the public key. The encodings according to the IETF drafts avoid this duplication even if -DNOPUBKEY_IN_PRIVKEY=OFF.
Adds CI test runs (with / without encodings set, with / without NOPUBKEY_IN_PRIVKEY)

Will add config options to Wiki if merged.

Fixes #89

scripts/runtests_encodings.sh

baentsch

Missing some documentation: Which env vars activate what (what are permissible env var values)?

baentsch · 2023-03-18T12:07:24Z

Will add config options to Wiki if merged.

Wiki documentation does not help people who just use a release distribution; also it becomes stale, so please document (at least the above) within github repo.

Also, the oqs-provider version information does not contain a hint as to what encoding is actually used in the built binary. Please add sth meaningful e.g. here:

oqs-provider/oqsprov/oqsprov.c

Line 422 in a7e3464

    
           #define OQS_PROVIDER_BUILD_INFO_STR "OQS Provider v." OQS_PROVIDER_VERSION_STR OQS_PROVIDER_COMMIT " based on liboqs v." OQS_VERSION_TEXT

.

* Update fragments * Add test runs to CI (with/without encodings, with/without NOPUBKEY_IN_PRIVKEY) * Encodings settable via env variables

- Allow to pass options to runtests_encodings.sh - Add qsc_encoder version strings

bhess · 2023-03-20T10:28:13Z

Missing some documentation: Which env vars activate what (what are permissible env var values)?

Added a section to README.md in 58a2c11.

Also, the oqs-provider version information does not contain a hint as to what encoding is actually used in the built binary. Please add sth meaningful e.g. here:

Also added in 58a2c11.

baentsch

Thanks for these updates, @bhess! Makes things clearer. Please also add a (single line) entry to https://github.com/open-quantum-safe/oqs-provider/blob/main/RELEASE.md regarding this feature. Also, might it be better to list all parameters in https://github.com/open-quantum-safe/oqs-provider/blob/main/ALGORITHMS.md rather than making the README even longer? When considering listing this in ALGORITHMS.md, the immediate question comes to mind whether the OIDs are/should be different based on which encoding is active: Would make sense, no?
Also good would be a link to the IETF drafts (instead of only writing "IETF draft"): That way people may look up possible (new) versions (or discover that this code became outdated :) In general, how do you envision the permitted values to change with new RFC draft rounds? Do the constants always have to change, even if just the draft date changed (creating a new version but no new code)?
Lastly, it may be sensible to add a new file "CONFIGURE.md" given we have ever more build-time configuration options... If you wouldn't want to do this in this PR, NP, I'll follow up afterwards.

bhess · 2023-03-20T14:40:19Z

Thanks for these updates, @bhess! Makes things clearer. Please also add a (single line) entry to https://github.com/open-quantum-safe/oqs-provider/blob/main/RELEASE.md regarding this feature. Also, might it be better to list all parameters in https://github.com/open-quantum-safe/oqs-provider/blob/main/ALGORITHMS.md rather than making the README even longer? Also good would be a link to the IETF drafts (instead of only writing "IETF draft"): That way people may look up possible (new) versions (or discover that this code became outdated :)

Thanks @baentsch. I now moved the env variable docs to ALGORITHMS.md along with links to the IETF drafts and just left the compile options part in README.md (A CONFIGURE.md for these also make sense to me after this PR).

In general, how do you envision the permitted values to change with new RFC draft rounds? Do the constants always have to change, even if just the draft date changed (creating a new version but no new code)? Lastly, it may be sensible to add a new file "CONFIGURE.md" given we have ever more build-time configuration options... If you wouldn't want to do this in this PR, NP, I'll follow up afterwards.

A new round will come with a string identifying the new version (e.g. draft-00 -> draft-01). If the encodings don't change, -00 and -01 will just lead to the same encoding. I think for ease of interoperability it's still useful to support a new release string.

When considering listing this in ALGORITHMS.md, the immediate question comes to mind whether the OIDs are/should be different based on which encoding is active: Would make sense, no?

Very good point I very much agree. The drafts unfortunately don't specify the OIDs as this wasn't desired yet by IETF. That said, the encoder library in our case don't encode OIDs as only the subjectPublicKey and privateKey are processed. For oqs-provider it is possible to just use the OID env variables to use additional OIDs. In my view, an OID convention like the following would make sense, where <OID> are the current OIDs used by oqs:

<OID>.<1 ... n> -> Explicitly tells that this object is encoded according to specifications <1 ... n>
<OID> -> Doesn't tell anything about which encoding is used, but just that this object relates to the algorithm defined by the OID. This is more or less the current state without standardized OIDs. It assumes that implementations 'have already agreed' on an encoding, or are able to inspect the structures to determine which is the encoding used.

oqsprov/oqsprov.c

oqsprov/oqs_prov.h

baentsch

Thanks for these changes and feedback, @bhess. Will follow up with a clean-up PR for config options. Just one change request remaining (reducing size of base version string).

- Ifdefs for code only used when USE_ENCODING_LIB is set - Add fragment for algorithms.md

baentsch · 2023-03-21T05:53:30Z

README.md

@@ -178,6 +178,12 @@ excludes all algorithms of the "Sphincs" family.
 *Note*: By default, interoperability testing with oqs-openssl111 is no longer
 performed by default but can be manually enabled in the script `scripts/runtests.sh`.

+### Key Encoding
+
+By setting `-DUSE_ENCODING_LIB=<ON/OFF>` at compile-time, oqs-provider can be compiled with with an an external encoding library `qsc-key-encoder`. Configuring the encodings is done via environment as described in [ALGORITHMS.md](ALGORITHMS.md).


Note to self: Document default value (ON).

baentsch · 2023-03-21T06:02:51Z

.circleci/config.yml

+      - run:
+          name: Build OQS-OpenSSL provider (only STD algs) with NOPUBKEY_IN_PRIVKEY
+          command: |
+             rm -rf _build && mkdir _build && cd _build && cmake -GNinja -DNOPUBKEY_IN_PRIVKEY=ON -DOPENSSL_ROOT_DIR=$(pwd)/../.local -DCMAKE_PREFIX_PATH=$(pwd)/../.local .. && ninja


Note to self: No CI test for USE_ENCODING_LIB=OFF -> Add one if we want to keep that (plain) option.

baentsch

Thanks again for the contribution!

…um-safe#123) * Add encoding library - Encodings settable via env variables - Add key encoding options to README.md - Add qsc_encoder version strings Signed-off-by: Felipe Ventura <felipe.ventura@entrust.com>

bhess force-pushed the bhe-encoding branch 2 times, most recently from f9ca14e to c63bd4c Compare March 17, 2023 19:33

bhess marked this pull request as ready for review March 17, 2023 19:48

bhess requested a review from baentsch as a code owner March 17, 2023 19:48

baentsch reviewed Mar 18, 2023

View reviewed changes

scripts/runtests_encodings.sh Outdated Show resolved Hide resolved

baentsch requested changes Mar 18, 2023

View reviewed changes

bhess added 2 commits March 20, 2023 11:16

* Add encoding library

28e5e78

* Update fragments * Add test runs to CI (with/without encodings, with/without NOPUBKEY_IN_PRIVKEY) * Encodings settable via env variables

- Add key encoding options to README.md

58a2c11

- Allow to pass options to runtests_encodings.sh - Add qsc_encoder version strings

bhess force-pushed the bhe-encoding branch from bae981f to 58a2c11 Compare March 20, 2023 10:16

baentsch requested changes Mar 20, 2023

View reviewed changes

Add env vars to ALGORITHMS.md, add entry to RELEASE.md

a328b73

baentsch reviewed Mar 20, 2023

View reviewed changes

oqsprov/oqsprov.c Show resolved Hide resolved

baentsch reviewed Mar 20, 2023

View reviewed changes

oqsprov/oqs_prov.h Outdated Show resolved Hide resolved

baentsch requested changes Mar 20, 2023

View reviewed changes

- Remove redundant version string (and -nopub)

323cddc

- Ifdefs for code only used when USE_ENCODING_LIB is set - Add fragment for algorithms.md

baentsch reviewed Mar 21, 2023

View reviewed changes

baentsch approved these changes Mar 21, 2023

View reviewed changes

baentsch merged commit 2e0d17a into main Mar 22, 2023

baentsch mentioned this pull request Mar 22, 2023

nginx update open-quantum-safe/oqs-demos#191

Merged

baentsch deleted the bhe-encoding branch March 22, 2023 13:57

WillChilds-Klein mentioned this pull request Jun 19, 2023

Convert oqs-kem-info.md code points to hex #188

Merged

Muzosh mentioned this pull request Jun 22, 2023

Implement KEM en-/decoders #194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds encoding options for subjectPublicKey and privateKey #123

Adds encoding options for subjectPublicKey and privateKey #123

bhess commented Mar 17, 2023 •

edited

Loading

baentsch left a comment

baentsch commented Mar 18, 2023 •

edited

Loading

bhess commented Mar 20, 2023

baentsch left a comment

bhess commented Mar 20, 2023 •

edited

Loading

baentsch left a comment

baentsch Mar 21, 2023

baentsch Mar 21, 2023

baentsch left a comment

Adds encoding options for subjectPublicKey and privateKey #123

Adds encoding options for subjectPublicKey and privateKey #123

Conversation

bhess commented Mar 17, 2023 • edited Loading

baentsch left a comment

Choose a reason for hiding this comment

baentsch commented Mar 18, 2023 • edited Loading

bhess commented Mar 20, 2023

baentsch left a comment

Choose a reason for hiding this comment

bhess commented Mar 20, 2023 • edited Loading

baentsch left a comment

Choose a reason for hiding this comment

baentsch Mar 21, 2023

Choose a reason for hiding this comment

baentsch Mar 21, 2023

Choose a reason for hiding this comment

baentsch left a comment

Choose a reason for hiding this comment

bhess commented Mar 17, 2023 •

edited

Loading

baentsch commented Mar 18, 2023 •

edited

Loading

bhess commented Mar 20, 2023 •

edited

Loading