search

stef / libopaque

c implementation of the OPAQUE protocol with bindings for python, php, ruby, lua, zig, java, erlang, golang, js and SASL.
GNU Lesser General Public License v3.0
69 stars 10 forks source link

Parameters should use consistent naming to avoid confusion #6

Closed creemama closed 3 years ago

creemama commented 3 years ago

In this pull request I looked at naming and byte sizes.

I also added opaque_envelope_len to avoid some boilerplate.

stef commented 3 years ago

52ec07332dd3665932375c6c629a984c21d63ac5 is a super good one. making this explicit everywhere is super important thanks!

stef commented 3 years ago

i would like to contest 7af3eb13b9363656566df9185e8ea0762320154d since it suggest something (reasonable) but not necessarily binding? dunno if that is a good word here. but what i wanna say, that the output here is a shared secret, which could be just as well plugged into AES if that makes sense, or one of the winners of the AEAD competition, or something entirely different like the rootkey of a (double)ratchet.

stef commented 3 years ago

great patch! only 1 or 2 things i'm unsure about, eager to hear your response to those otherwise there's only one thing i would like to change, there's now an AUTHORS file in the root, add yourself to it pls.

creemama commented 3 years ago

i would like to contest 7af3eb1 since it suggest something (reasonable) but not necessarily binding? dunno if that is a good word here. but what i wanna say, that the output here is a shared secret, which could be just as well plugged into AES if that makes sense, or one of the winners of the AEAD competition, or something entirely different like the rootkey of a (double)ratchet.

I'm not a cryptography expert, so I think that makes sense. I'll remove this commit.

This is just a thought. I was wondering if in opaque.h if sk should be uint8_t *sk since the size of sk is not binding. However, in opaque.c, change references to some of the 32s to a new OPAQUE_SHARED_SECRET_LEN or crypto_secretbox_KEYBYTES.

stef commented 3 years ago

This is just a thought. I was wondering if in opaque.h if sk should be uint8_t *sk since the size of sk is not binding. However, in opaque.c, change references to some of the 32s to a new OPAQUE_SHARED_SECRET_LEN or crypto_secretbox_KEYBYTES.

maybe OPAQUE_SHARED_SECRETBYTES is more in line with the conventions.

creemama commented 3 years ago

I think this branch is ready to merge with your approval.

stef commented 3 years ago

super good! thanks for this valuable cleaning up of my mess!