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

Import export sign retrieve #119

Merged
merged 10 commits into from
Sep 1, 2022
Merged

Conversation

indomitableSwan
Copy link
Contributor

Contains new protocols for importing, exporting, signing, and retrieving that captures remote secret generation and signing. Also contains variants for local storage, signing, and retrieval.

Closes #114: importing a key changes (to allow for remote-only storage).
Closes #109: obtaining a signature from key server (where key server signs)
Closes #118: fixing some context issues in generation and retrieve.

  • Makes progress on Add ECDSA signing functionality #107 (but does not complete bc issues haven't yet been created to implement changes made to spec). i.e., protocols for local and remote signing added to spec.
  • Clarifies some issues around client-side storage (Define requirements for client-side storage #39) and as a related issue, fixes the "local generation of a secret" workflow to be consistent with the new local signing functionality.
  • Contains some minor cleanup and additional implementation guidance throughout for consistency and redundancy.
  • Fixes some inconsistencies in generation of secrets that were missed in the original "remote generation of secrets" draft.
  • Fixes miscellaneous typos and inconsistencies.

Copy link
Contributor

@marsella marsella left a comment

Choose a reason for hiding this comment

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

I have a few clarifying questions but overall this looks quite good to me.

An implementation note: This PR requires somewhat more interaction with the arbitrary data / context stored with keys and used in encryption. We will likely want to update the representation of that data in the code to be able to more efficiently do things like, "check if the associated data says client generated"

system-functionalities.md Outdated Show resolved Hide resolved
system-functionalities.md Show resolved Hide resolved
dev-notes.md Show resolved Hide resolved
@indomitableSwan
Copy link
Contributor Author

I have a few clarifying questions but overall this looks quite good to me.

An implementation note: This PR requires somewhat more interaction with the arbitrary data / context stored with keys and used in encryption. We will likely want to update the representation of that data in the code to be able to more efficiently do things like, "check if the associated data says client generated"

Yes, agreed with that last note on implementation. I think there is still confusion as to the point of the specification and what the pseudocode in the spec is meant to capture. I'm not trying to capture the implementation design work here, I'm trying to capture enough detail so that a developer can get a working implementation. More to the point, I don't have the bandwidth to write the spec at the level of detail where an implementor doesn't have to think through design at all, and I'm not sure that's even desirable anyway.

There are some places I suspect the spec could benefit from including more precise encoding requirements and descriptions (e.g. when there are security implications if the implementation does not include the missing details properly, e.g., see #101, and #120).

My feeling is that if we want the spec to capture that level of detail (and I'm not sure we do, in fact, want that), then the implementor has to be the responsible party for updating the specification in tandem with the corresponding code.

Concretely, what do you suggest here?

Copy link
Contributor

@marsella marsella left a comment

Choose a reason for hiding this comment

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

Yes, agreed with that last note on implementation. I think there is still confusion as to the point of the specification and what the pseudocode in the spec is meant to capture. I'm not trying to capture the implementation design work here, I'm trying to capture enough detail so that a developer can get a working implementation. More to the point, I don't have the bandwidth to write the spec at the level of detail where an implementor doesn't have to think through design at all, and I'm not sure that's even desirable anyway.

There are some places I suspect the spec could benefit from including more precise encoding requirements and descriptions (e.g. when there are security implications if the implementation does not include the missing details properly, e.g., see #101, and #120).

My feeling is that if we want the spec to capture that level of detail (and I'm not sure we do, in fact, want that), then the implementor has to be the responsible party for updating the specification in tandem with the corresponding code.

Concretely, what do you suggest here?

Oh, I agree that we shouldn't try to capture implementation details like AD representation here -- I mostly wanted to write it down so I'd remember it later.

As you say, I think encoding is the most useful thing to include -- but that is often most useful for other people who are generating compatible implementations, and that's not really in our medium-term plan, and definitely not for this PoC. I would appreciate guidance on when to add details to the spec, or I can just check in with you if I find myself implementing something that seems complex and un-specified.

@indomitableSwan
Copy link
Contributor Author

Yes, agreed with that last note on implementation. I think there is still confusion as to the point of the specification and what the pseudocode in the spec is meant to capture. I'm not trying to capture the implementation design work here, I'm trying to capture enough detail so that a developer can get a working implementation. More to the point, I don't have the bandwidth to write the spec at the level of detail where an implementor doesn't have to think through design at all, and I'm not sure that's even desirable anyway.
There are some places I suspect the spec could benefit from including more precise encoding requirements and descriptions (e.g. when there are security implications if the implementation does not include the missing details properly, e.g., see #101, and #120).
My feeling is that if we want the spec to capture that level of detail (and I'm not sure we do, in fact, want that), then the implementor has to be the responsible party for updating the specification in tandem with the corresponding code.
Concretely, what do you suggest here?

Oh, I agree that we shouldn't try to capture implementation details like AD representation here -- I mostly wanted to write it down so I'd remember it later.

As you say, I think encoding is the most useful thing to include -- but that is often most useful for other people who are generating compatible implementations, and that's not really in our medium-term plan, and definitely not for this PoC. I would appreciate guidance on when to add details to the spec, or I can just check in with you if I find myself implementing something that seems complex and un-specified.

Right, sounds like we're in agreement. Adding notes on encoding where that relates to security might be advisable; otherwise we're not attempting to create an interoperable spec.

@indomitableSwan indomitableSwan merged commit 73fa933 into main Sep 1, 2022
@marsella marsella deleted the import-export-sign-retrieve branch September 21, 2022 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants