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
Introduce new Secure Message API #389
Introduce new Secure Message API #389
Changes from 10 commits
ef65c98
54aefce
aaaccdb
609a6bf
02b38ea
acb767b
9ff2cf0
c21a87f
ac107b4
8306f70
7f4f939
35e3cde
c341511
0d3edf5
7075209
3549f9d
fdc4bc6
7e75bb7
dfbd1b3
ec67c84
27970d3
2f02158
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.
maybe it's time to verify the type of key to avoid misusage incorrect type of key (public/private)? : )
we can do simple check on prefix of key value for known prefixes of private keys:
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.
but better will be to extend api and add some functions that will validate keys:
validate_private_key
/validate_public_key
and for now, it will check the only prefix. In the future we can extend these validations with verifying full key's structureThere 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.
Come to think of it... I've added the following utilities for the Rust wrapper to query the key kind and validate its content:
themis/src/wrappers/themis/rust/libthemis-sys/src/wrapper.h
Lines 28 to 41 in 29ebf63
Can they be of some use? What do you think of moving these functions to the core library (and using them for checks internally as well)? I believe this API may be useful for applications to verify key content independently.
As for Secure Messages, I guess we can even go a bit further and verify that both private and public keys are of ECDSA or RSA flavor. Ideally, we'd also verify that both use the same public parameters, but that's harder to do.
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.
good checks, I think it's good to move into the core themis library
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.
agree, good checks, make sense at core lvl
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.
@Lagovas @vixentael I've moved the key validation routines to Themis core, added some tests for them, added validation to new Secure Message API, and added some early checks to ThemisPP and JsThemis which are most affected by poor error reporting. Please give it a look.