Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
addition of v6 and PQC #35
addition of v6 and PQC #35
Changes from all commits
433d304
744fe3e
e3b72cc
1e1757c
012fd6a
c9b1cf5
64ba6cd
8be734b
eb8ba12
2ab6c04
4d0fc64
19bde74
ad0e455
cfa5b9b
f17d80b
bd8ab73
e42827a
89e91a3
c804bad
737c71f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it under the define.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand: move what exactly where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@falko-strenzke As
PGP_SEIPDV2_SALT_LEN
is used only with ENABLE_CRYPTO_REFRESH, it should be under the#ifdef ENABLE_CRYPTO_REFRESH
, so code withoutENABLE_CRYPTO_REFRESH
doesn't have unused defines.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in upcoming commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some note that it is for experimental usage only and could be removed or replaced by other function at any time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be done via
rnp_op_encrypt_set_flags()
. However, adding some note that this is experimental and could be replaced or removed would also work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a note for now. If you prefer the other option I can do that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put these new defines under the PQC ifdef.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed to add
#include "config.h"
here s.t. the defines are set. Is that ok for this file?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TJ-91 Sorry, somehow mislooked your comment. Now I realize that this will not work -
config.h
is not installed to/usr/include/
, so attempt to compile code which usesrnp.h
will fail, unless I mislooked something. Wondering what would be the correct solution...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check here or somewhere else for Botan version 3.1.0 (SPHINCS support) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not handled in the line that follows?
We can update the find_package command to 3.1.0 (instead of 3.0.0), would that suffice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TJ-91 Yeah, that would work (if added only for PQC, allowing any vesion of Botan for non-PQC).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it at checking for 3.0.0 in the find_package() command. If PQC is enabled, the features are checked:
resolve_feature_state(ENABLE_PQC "KMAC;DILITHIUM;KYBER;SPHINCS_PLUS_WITH_SHA2;SPHINCS_PLUS_WITH_SHAKE")
. This means 3.0.0 is fine as long as PQC is disabled. If PQC is enabled, 3.0.0 will fail due to features.