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

Some corrections to the text #438

Merged
merged 5 commits into from
Mar 7, 2024
Merged

Some corrections to the text #438

merged 5 commits into from
Mar 7, 2024

Conversation

bytemare
Copy link
Collaborator

As I was rereading the draft, I saw a couple of mistakes and missing spaces. So I ran the text through a corrector that found some other suggestions.

Copy link
Collaborator

@chris-wood chris-wood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some nits

clients to safely store and retrieve arbitrary application data on servers
using only their password.
using only their passwords.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
using only their passwords.
using only their password.

Copy link
Collaborator Author

@bytemare bytemare Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not native in English so it's very probable that my suggestion comes from another language.

But does putting the singular to password not mean something like "the one password that all the clients have" instead of "the password of each client"?

Copy link
Collaborator

@kevinlewi kevinlewi Mar 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is best left singular, agree with Chris's suggestion here

draft-irtf-cfrg-opaque.md Outdated Show resolved Hide resolved
draft-irtf-cfrg-opaque.md Outdated Show resolved Hide resolved
bytemare and others added 2 commits January 8, 2024 19:35
Co-authored-by: Christopher Wood <caw@heapingbits.net>
Co-authored-by: Christopher Wood <caw@heapingbits.net>
@@ -1923,7 +1923,7 @@ protocols such as TLS.
The specification as written here differs from the original cryptographic design in {{JKX18}}
and the corresponding CFRG document {{I-D.krawczyk-cfrg-opaque-03}}, both of which were used
as input to the CFRG PAKE competition. This section describes these differences, including
their motivation and explanation as to why they preserve the provable security of OPAQUE based
their motivation and explanation as to why they preserve the provable security of OPAQUE-based
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change should be reverted

@@ -2233,7 +2233,7 @@ message for an unregistered client if these client enumeration attacks can
be mitigated through other application-specific means or are otherwise not
applicable for their threat model.

OPAQUE does not prevent against this type of attack during the registration flow.
OPAQUE does not prevent this type of attack during the registration flow.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better change would be "protect against"

clients to safely store and retrieve arbitrary application data on servers
using only their password.
using only their passwords.
Copy link
Collaborator

@kevinlewi kevinlewi Mar 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is best left singular, agree with Chris's suggestion here

@@ -329,7 +329,7 @@ as a "compiler" for transforming any suitable AKE protocol into a secure
aPAKE protocol. (See {{security-considerations}} for requirements of the
OPRF and AKE protocols.) This document specifies one OPAQUE instantiation
based on {{TripleDH}}. Other instantiations are possible, as discussed in
{{alternate-akes}}, but their details are out of scope for this document.
{{alternate-akes}}, but their details are out of the scope of this document.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would revert this change

Copy link
Collaborator

@kevinlewi kevinlewi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and addressed the comments I left with a commit, so I am approving now!

@kevinlewi kevinlewi merged commit 36470f8 into master Mar 7, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

None yet

3 participants