Add support for parameterized SipHash #2216
Conversation
|
So, this PR basically assumes that PR #2128 will be submitted first. |
test/testutil.c
Outdated
| for (i = 0; i < len; i++) | ||
| fprintf(stderr, "%02x", a[i]); | ||
| } | ||
|
|
There was a problem hiding this comment.
This could really be a separate PR
There was a problem hiding this comment.
Yeah; I pulled it out of poly1305_internal_test.c
There was a problem hiding this comment.
Cleaning up all "hexdump" to use that one common routine is a separate PR, but let's not make it the situation worse with this one.
There was a problem hiding this comment.
Not necessary with BIO_hex_string, but I undid the changes to poly1305.
tmshort
force-pushed
the
tshort-siphash
branch
from
7842545
to
4db8bc3
Compare
|
This needs a rebase |
|
Yup, with the Poly1305 commit. |
tmshort
force-pushed
the
tshort-siphash
branch
from
4db8bc3
to
5e84c0c
Compare
|
Rebased. |
|
Looks good to me. |
crypto/evp/p_lib.c
Outdated
| const unsigned char *EVP_PKEY_get0_siphash(const EVP_PKEY *pkey, size_t *len) | ||
| { | ||
| ASN1_OCTET_STRING *os = NULL; | ||
| if (pkey->type != EVP_PKEY_SIPHASH) { |
crypto/include/internal/siphash.h
Outdated
| #define SIPHASH_MIN_DIGEST_SIZE 8 | ||
| #define SIPHASH_MAX_DIGEST_SIZE 16 | ||
|
|
||
| typedef struct siphash_context SIPHASH; |
There was a problem hiding this comment.
we don't have consistency here, but maybe siphash_st is the more commonly used style?
crypto/siphash/siphash_ameth.c
Outdated
| static void siphash_key_free(EVP_PKEY *pkey) | ||
| { | ||
| ASN1_OCTET_STRING *os = EVP_PKEY_get0(pkey); | ||
| if (os != NULL) { |
crypto/siphash/siphash_ameth.c
Outdated
|
|
||
| static int siphash_pkey_ctrl(EVP_PKEY *pkey, int op, long arg1, void *arg2) | ||
| { | ||
| /* nothing, (including ASN1_PKEY_CTRL_DEFAULT_MD_NID), is supported */ |
There was a problem hiding this comment.
remove, the, first, comma.
or (remove the parens)
There was a problem hiding this comment.
Three commas in a row? oh my,
crypto/siphash/siphash_pmeth.c
Outdated
|
|
||
| /* SIPHASH pkey context structure */ | ||
|
|
||
| typedef struct { |
There was a problem hiding this comment.
put a name here, siphash_pkey_ctx_st
There was a problem hiding this comment.
Uhmmmm... why is that important?
| @@ -49,7 +49,7 @@ The control command is indicated in B<cmd> and any additional arguments in | |||
| B<p1> and B<p2>. | |||
|
|
|||
| For B<cmd> = B<EVP_PKEY_CTRL_SET_MAC_KEY>, B<p1> is the length of the MAC key, | |||
| and B<p2> is MAC key. This is used by Poly1305, HMAC and CMAC. | |||
| and B<p2> is MAC key. This is used by Poly1305, SipHash, HMAC and CMAC. | |||
There was a problem hiding this comment.
all that work and this is the only doc update needed? :) the joy of standard API's.
There was a problem hiding this comment.
I don't believe the EVP_PKEY_SIPHASH (or other similar ones) have a place for documentation. But I should check.
There was a problem hiding this comment.
it was a comment on how good standard API's are. not asking for more work on this.
There was a problem hiding this comment.
Oh, I know! :) But I want to be thorough.
There was a problem hiding this comment.
Nothing referencing stuff like EVP_PKEY_HMAC, etc.
INSTALL
Outdated
| rc2, rc4, rmd160, scrypt, seed or whirlpool. The "ripemd" | ||
| algorithm is deprecated and if used is synonymous with rmd160. | ||
| rc2, rc4, rmd160, scrypt, seed, siphash or whirlpool. The | ||
| "ripemd" algorithm is deprecated and if used is synonymous |
There was a problem hiding this comment.
might be some white-space issues here.
There was a problem hiding this comment.
check space/TAB issues. GH sometimes gets confused, too.
|
There already is a no-siphash option. It's currently on by default |
The core SipHash supports either 8 or 16-byte output and a configurable number of rounds. The default behavior, as added to EVP, is to use 16-byte output and 2,4 rounds, which matches the behavior of most implementations. There is an EVP_PKEY_CTRL that can control the output size.
tmshort
force-pushed
the
tshort-siphash
branch
from
5e84c0c
to
f14cdc2
Compare
|
Rebased, comments resolved. |
|
@levitte please reconfirm and i will merge. |
The core SipHash supports either 8 or 16-byte output and a configurable number of rounds. The default behavior, as added to EVP, is to use 16-byte output and 2,4 rounds, which matches the behavior of most implementations. There is an EVP_PKEY_CTRL that can control the output size. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from #2216)
|
3f5616d on master. Thanks! |
The core SipHash supports either 8 or 16-byte output and a configurable
number of rounds.
The default behavior, as added to EVP, is to use 16-byte output and
2,4 rounds, which matches the behavior of most implementations.
Note that documentation has not been updated, as there was some doc updates
for Poly1305 that I did in PR #2128 that I want to build upon.
Checklist
Description of change