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

External security / crypto review #768

Closed
mnot opened this issue Jun 16, 2021 · 11 comments
Closed

External security / crypto review #768

mnot opened this issue Jun 16, 2021 · 11 comments
Assignees
Labels
cr-comment Candidate Recommendation comment cr-comment-resolved-explicit Candidate Recommendation comment resolved with explicit agreement pending close Issue will be closed shortly if no objections

Comments

@mnot
Copy link
Member

mnot commented Jun 16, 2021

Seeing as this spec claims certain security and cryptographic properties, it should undergo external review.

If it has had such review, could someone please point to it?

If not, I'd recommend starting with the IETF Security Area for the former (they can do external reviews on request), and the IRTF CFRG for the latter.

@msporny msporny added the cr-comment Candidate Recommendation comment label Jun 27, 2021
@msporny
Copy link
Member

msporny commented Jun 27, 2021

Hey @mnot, apologies for being delayed in getting to this, I missed it in my inbox.

The group has gone through the mandatory W3C horizontal review process including the W3C Technical Architecture Group (TAG), the W3C Privacy Interest Group (PING), a11y, i18n, done the mandatory Security and Privacy assessment, etc. The issues covering those reviews are here:

https://github.com/w3c/did-core/issues?q=is%3Aissue+is%3Aclosed+review+label%3A%22horizontal+review%22

The write-up of the Security and Privacy assessment is here (but this was an internal self-assessment with an external set of reviewers from the W3C TAG):

https://docs.google.com/document/d/1c36r86oI7q4qv0YKVsu0zlNiMwWTqCfWWReAkg3gd2A/edit#

We also have a number of security and privacy experts in the group that have been involved for a long time, but we both know that doesn't meet the bar for a proper external review; I'm just mentioning because it does come into play (we're not just flying by the seat of our pants... but could always use more eyes on the problem).

We did explore the notion of engaging the IETF Security Area and CFRG, but the general conclusion in the group was that we were not normatively defining security key formats or security protocols in this WG (the WG's scope was tightly restricted to just the data model for expressing those sorts of things). We are defining a data model that could use key formats (we do have IETF JWKs that are expressible in the data model) and could be used in a security protocol... but again, doing those things are outside of our charter scope. So the feeling was that there was nothing normative for the IETF Security Area or the CFRG to have an opinion on... but the next version of this WG (the one that starts working on protocols using this data model) is definitely where we'd need that sort of input. All that said, more review is a good thing.

We have a call this coming Tuesday and I'll request that we put it on the Agenda.

Would you suggest we start with emails to secdir? https://mailarchive.ietf.org/arch/browse/secdir/ and CFRG? https://mailarchive.ietf.org/arch/browse/cfrg/

@iherman
Copy link
Member

iherman commented Jun 30, 2021

The issue was discussed in a meeting on 2021-06-29

  • no resolutions were taken
View the transcript

5.1. External security / crypto review

See github issue #768.

Manu Sporny: the big one is from mark from the http wg, asking for an external security review...
… Mark co-chairs http wg at IETF... he is asking us why we have not asked IETF or CFRG to do a security review
… we also asked TAG
… folks at IETF are asking why there is no sec review
… suggestion is to do it, even though it will annoy folks...
… still they may have comments
… I asked mark if he expects us to engage with CFRG, and the security area

Drummond Reed: I was going to mention the ones that we are waiting on michael for, we have waited that out
… how long will a security review take?

Manu Sporny: 1 to N months to complete

Daniel Burnett: we have done what we are required to do by process
… while sec reviews are good, its not appropriate for it to be a gating factor

Manu Sporny: agree, it should not be a gating factor for getting to REC, its a politics and optics issue
… its important to work with CFRG to address the concerns folks are raising.
… the responses from CRFG have not been great.... we will go into a maintenance mode, we would like to start engaging on these items...
… when the WG is recharted / reconvened we can implement feedback gathered.

Drummond Reed: I like that

Daniel Burnett: part of the process has been implementations, its important to convey that this is not an opportunity to restart our process
… we are continuing our existing process, and look forward to addition reviews and feedback.

Manu Sporny: I will take an action to draft some language, and can put it to the group to review

Daniel Burnett: reminder that we have a higher bar regarding consensus than IETF, we are way beyond "rough consensus and running code"
… folks have known about this for some time
… this will come up as we proceed to the next phase

Manu Sporny: I have the ball on a draft

@mnot
Copy link
Member Author

mnot commented Jul 1, 2021

For me, the most important thing here is to build confidence in the security properties of the solution. I'm sure the folks who have been involved are competent and expert, but that's not sufficient to create the trust necessary for critical infrastructure.

I mentioned the Security directorate and CFRG because they were obvious (to me) places to start; by all means engage with them if you're comfortable doing that; they'll probably have much better ideas that I do about this.

Another thing you could do is working within the academic world to get their eyes on it, much like TLS did for 1.3 with the TRON Workshop.

Just throwing it out there for consideration; if this becomes as important as it promises, building this kind of confidence is key.

@mnot
Copy link
Member Author

mnot commented Jul 1, 2021

Oh, and regarding:

we were not normatively defining security key formats or security protocols in this WG (the WG's scope was tightly restricted to just the data model for expressing those sorts of things). We are defining a data model that could use key formats (we do have IETF JWKs that are expressible in the data model) and could be used in a security protocol... but again, doing those things are outside of our charter scope.

This may very well be; I'm neither a security expert nor deeply knowledgeable about the architecture here. It may be that the secdir tells you to come back at a different point.

@msporny
Copy link
Member

msporny commented Jul 1, 2021

@mnot wrote:

Just throwing it out there for consideration; if this becomes as important as it promises, building this kind of confidence is key.

Yes, strong agreement from me (and now the WG). As you may have seen above, the DID WG agrees with the notion of engaging secdir and CFRG. I have an action to write up an email for approval by the WG to send to secdir and CFRG. The transcription of the WG meeting doesn't quite capture what was said, so here are the highlights:

  • The DID WG has done a lot of horizontal review already, which includes external security and privacy review. We also have 32 implementations of the specification for many of the core "security features" (that is data model properties that are capable of expressing cryptographic key material). All that to say, we've had a lot of eyes on this stuff over the last 5+ years. From a W3C Process perspective, we've ticked all the boxes (but we are not satisfied with just doing that, for the reasons you mention).
  • The DID WG's Charter expires in September 2021 (3 months from now); so that doesn't give us a ton of time to change course at this point.
  • The DID WG will almost certainly get re-chartered as a maintenance group, and will almost certainly get spun up again in a year to do the next push (whatever that may be, probably DID Resolution, some broadly useful DID Methods, or further normative definition of the cryptographic data formats and protocols).

So, with that context, we will reach out to secdir and CFRG to note that we want a review of the specification, but it doesn't have to be done before the WG completes, and the WG is not putting that as a requirement for transition to REC. We do, however, want a deeper engagement with secdir and CFRG for this WG and multiple other security-related WGs (Verifiable Credentials v2, Linked Data Signatures, RDF Dataset Canonicalization, etc.) that are going to be spinning up over the next year or two. It's a broader liaison relationship that was already in the works (/cc @iherman), but this gives us yet another reason to set that up and keep secdir/CFRG engaged with the work going on at W3C in multiple WGs.

All this to say, we're taking action on your request, but possibly not in the way that you had anticipated. Since we're in the Candidate Recommendation phase, and given our timeline, this is the best we think we can do at this stage. Since your comment is also a CR comment, we will need confirmation from you on whether you feel that this is a reasonable way to address your concern and that you feel that if we follow through with the above that we can mark this issue as resolved (once the final emails hit the IETF mailing lists referenced above).

In summary, here's what we're going to do:

  1. We're going to contact secdir and CFRG to request a review of the DID Core specification on a timeframe that works for them. We will hopefully get a review back before we have to proceed to REC.
  2. We are going to request a broader/longer engagement from secdir and CFRG on the technologies in the ecosystem, some of these technologies are being pushed forward in other WGs (RDC WG, DID WG, VC WG, etc.).

If we do those things, would that address your immediate concern raised in this issue?

@mnot
Copy link
Member Author

mnot commented Jul 2, 2021

Sounds good to me. It might be worth just initially talking to the secdir; reviewing non-IETF documents isn't typical for them, but IIRC they've done it before, and they'll be able to advise about whether to pull CFRG in.

@selfissued
Copy link
Contributor

@msporny 's statements notwithstanding, while the specification isn't normatively defining new key formats, it is normatively referencing new and non-standard key formats. In particular, it references this individual draft not in a working group, which defines or references multiple new and non-standard key formats:

Note that there's a PR #772 under active discussion to add a warning that MultiBase is not a standard and is subject to change.

I personally believe that referencing non-standard and evolving key formats is a mistake - possibly a security mistake - but I recognize that some in the working group feel differently.

I strongly believe that this specification would benefit from outside security review.

@msporny
Copy link
Member

msporny commented Jul 3, 2021

@selfissued wrote:

it is normatively referencing new and non-standard key formats.

No, it's not normatively referencing new key formats -- it is *non-normatively* referencing new key formats while those key formats are allowed the time to become standardized. The specification does that because it's documenting reality -- those new key formats are used in the majority of implementations that have been submitted to the test suite. This approach (to document reality) was (and still is) the consensus position of the WG.

... and yes, we should get outside security review on all of this.

@msporny msporny self-assigned this Jul 3, 2021
@msporny
Copy link
Member

msporny commented Jul 11, 2021

@mnot wrote:

Sounds good to me. It might be worth just initially talking to the secdir; reviewing non-IETF documents isn't typical for them, but IIRC they've done it before, and they'll be able to advise about whether to pull CFRG in.

Letter to IETF secdir has been prepared and is being reviewed by the DID WG: https://docs.google.com/document/d/1ZiSOhXJ3agevZ8uHX_N6zEi_mbso7jWxMHzsWYs6E28/edit

@msporny
Copy link
Member

msporny commented Jul 17, 2021

The message has been sent to IETF secdir:

https://mailarchive.ietf.org/arch/msg/secdir/ijoxNV3tjqYw1WOKI7T5O0JYts0/

I'm marking this issue as pending close given that we're transitioning to Proposed Recommendation this coming week and it will most likely take many weeks to months to achieve the end goal of what we have kicked off.

Folks should feel free to disagree with closing this issue if there is another solution that was expected and that can be achieved before the WG's charter expires in September 2021.

This issue will be closed in 7 days if there are no objections.

@msporny msporny added pending close Issue will be closed shortly if no objections cr-comment-resolved-explicit Candidate Recommendation comment resolved with explicit agreement labels Jul 17, 2021
@msporny
Copy link
Member

msporny commented Jul 24, 2021

No objections received to closing during 7 day wait period. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cr-comment Candidate Recommendation comment cr-comment-resolved-explicit Candidate Recommendation comment resolved with explicit agreement pending close Issue will be closed shortly if no objections
Projects
None yet
Development

No branches or pull requests

4 participants