Skip to content
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

Address Stephen's nits #144

Closed
chris-wood opened this issue Aug 15, 2020 · 1 comment
Closed

Address Stephen's nits #144

chris-wood opened this issue Aug 15, 2020 · 1 comment

Comments

@chris-wood
Copy link
Collaborator

From his feedback on the list:

  1. p4: PGP and fiveG don't depend on HPKE, whereas msl and esni do. Maybe worth saying that.
  2. p5: I2OSP and OS2IP aren't expanded here - be no harm to do so
  3. p6: Open as the opposite of Seal, doesn't seem like the best choice - there are too many functions called open() in too many contexts. Unseal would be better IMO.
  4. p5: DeriveKeyPair was added after discussion in github (and maybe on the MLS list) but I don't recall any disussion of that on the CFRG list at all. Was there any? Adding this seems fine to me if MLS wants it and the definition is also almost fine (see above) so this is just a process nit.
  5. p5 & elsewhere: "fixed-length" is used in various places where it's not quite true - the various lengths are fixed only after you pick a ciphersuite - so people's code has to support different sizes (if they support >1 suite)

My take on each of these is:

  1. We can clarify this.
  2. These seem fairly common in folded form and are quite a mouthful when expanded, so I'm inclined to keep these as is.
  3. Open and seal seem fairly common, at least in APIs I'm familiar with (BoringSSL/OpenSSL, libsodium, NaCL, etc), so I'm inclined to keep this as is.
  4. I disagree about this being a process nit. The document is still an active RG document, so everything is subject to change. I'm not sure why discussion would be needed before or after it lands in the document?
  5. Similar to other stacks I'm familiar with, I assume memory would statically allocated for each of these fixed-length values (on the stack) based on the maximum size supported. The specific ciphersuite/length would then use however much of that memory is needed. So this seems fine to keep, though perhaps clarifying might help? I'm not sure what that would look like right now, but I'll give it some thought.
@blipp
Copy link
Contributor

blipp commented Aug 24, 2020

  1. I agree.
  2. Maybe a middle ground would be to specify the parameters of the two functions? I2OSP(int, len) and OS2IP(str). At least for I2OSP this is usefull to recall the order of the parameters.
  3. I haven't seen Open and Seal before, but my experience with crypto libraries is limited anyway, so I abstain.
  4. I am not sure how the process is supposed to be. I haven't seen many detailed discussions about decisions like this one on the full CFRG list since I am subscribed, so it seems not unusual to have these discussions off-list?
  5. I do not have enough experience to comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants