-
Notifications
You must be signed in to change notification settings - Fork 1
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
initial WebID Document considerations #9
Conversation
|
||
* Authorization system depends on a specific trust anchor in the user's WebID Document. | ||
For example, [[Solid.OIDC]] issuer designation or pubic keys/keyset. | ||
* User authorizes a malicious application to write or append to their WebID Document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The context of malicious application is unclear here. Whether it is something that adheres to Solid Protocol interacting with a Solid storage, or just generally anything potentially having the means to update the WebID Profile Document. They are separate cases, and whether or to what extent this document (Solid Security Considerations) should touch on WebID Profile Documents that are not available via the Solid Protocol needs a consideration.
index.bs
Outdated
|
||
### Countermeasures | ||
|
||
* There is no requirement to expose WebID Document via [[Solid.Protocol]] and host it in Solid Storage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs more nuance. Technically true for any WebID Profile Document but not true for Solid WebID Profiles. In other words, WebID Profile Documents served from a Solid Storage - which is a core requirement from Solid use cases - naturally use the Solid Protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid Protocol 0.11 doesn't depend on a WebID profile draft. If Solid Protocol 0.11 requires a WebID Document to be exposed via the Solid Protocol, please link it in this thread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* There is no requirement to expose WebID Document via [[Solid.Protocol]] and host it in Solid Storage. | |
* There is no requirement to expose WebID Document via [[Solid.Protocol]] nor to host it in Solid Storage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elf-pavlik , in https://github.com/solid/security-considerations/pull/9/files#r1626487426 you are now using Solid application as the prerequisite of the attack. That's obviously only meaningful in the context of the Solid Protocol and hence Solid storage since Solid applications can't be guaranteed to do anything against servers that do not conform to the Solid Protocol.
If the idea is to only detail attacks pertaining to WebID Profile Documents hosted on a Solid storage and that their trust anchors could change, then stick to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hosting a WebID Document in Solid Storage makes this attack possible. One proposed countermeasure is not to host the WebID Document in Solid Storage.
- the attack relies on the WebID Document being hosted in a Solid Storage
- one of the countermeasures relies on WebID Document not being hosted in a Solid Storage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the take away is that if we don't use Solid, the system will actually be more secure? =) Maybe we need to rethink that...
A similar attack categorically exists when the document is not hosted on a Solid storage. Whatever takes to modify the trust anchor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider comparing it to a secured house with a security safe inside. While the house already has a security system, some critical items are protected by being placed in a security safe. Does it make no sense to use the safe if you can't fit the whole house in it?
We agreed at the last meeting that what is being discussed here is an actual security issue existing in many Solid deployments. What's proposed here is only one possible solution. Please feel free to PR another solution to the vulnerability we talked about here. I'm currently implementing the proposed solution as an option for CSS; It is not required, but I hope you or someone else plans to work on implementing the alternative, which I assume you would like to add in a dedicated PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the take away is that if we don't use Solid, the system will actually be more secure? =) Maybe we need to rethink that...
Right. I think this PR is saying "if you need to host a WebID Document securely, don't do it with Solid". What I think we should be doing here is "If you need to host a WebID Document securely on a Solid storage, here's how to do it".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those two approaches are not mutually exclusive. One may be available today, and one could be made available sometime in the future. If you have an alternative countermeasure, please create a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the take away is that if we don't use Solid, the system will actually be more secure? =) Maybe we need to rethink that...
We also need to rethink removing user control over the most important document in the Solid ecosystem. @elf-pavlik mentioned how many members of the CG have their WebIDs hosted on non-Solid servers but that is really irrelevant. A CG member does not lose control over their WebID, they know how to edit it and what the edits mean. This does not apply to the general public who could benefit from a profile-editing app that informs them of what their changes mean.
index.bs
Outdated
WebID providers can support only a set of discovery features relying on the WebID document and provide | ||
highly secured interface to change it, possibly requiring two-factor authentication. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "WebID provider", can this be expressed using other terms related to, e.g., "Solid storage", "WebID Profile Document", "HTTP server", "OAuth Identity Provider" or something else?
The user was tricked into authorizing the application, it is unclear how 2FA would actually be a countermeasure besides introducing additional steps to the user. If the idea is that the user should be made aware of all or any "important" changes to the WebID Profile Document (in which the trust anchor is being injected) in the form of having them go through 2FA, this can be made more clear in the text.
This countermeasure also seems to punt the problem. It seems to essentially boil down to user being made aware of potentially unwanted changes to their WebID Profile Document. Perhaps this should jump out more in the text?
index.bs
Outdated
Users need a way to manage their social profiles, approaches like linking from the user's WebID Document to | ||
less protected vcard are available to accomplish it without opening discussed attack vectors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users need a way to manage their social profiles, approaches like linking from the user's WebID Document to | |
less protected vcard are available to accomplish it without opening discussed attack vectors. | |
Users need a way to manage their social profiles, approaches like linking from the user's WebID Document to | |
less protected vcard are available to accomplish it without opening discussed attack vectors. | |
For performance reasons, applications may only read the primary WebID Profile Document and ignore useful information which may only be available from additional profile documents. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are you basing this performance aspect on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users need a way to manage their social profiles, approaches like linking from the user's WebID Document to | |
less protected vcard are available to accomplish it without opening discussed attack vectors. | |
Users need a way to manage their social profiles. Approaches like linking from the user's WebID Document to | |
less-protected vcards are available to accomplish this without opening discussed attack vectors. | |
For performance reasons (e.g., the time required to retrieve multiple linked documents from | |
remote services), applications may choose to only read the primary WebID Profile Document and | |
ignore possibly useful information which may only be available in additional profile documents. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion doesn't have 3. ... So we have O(1) and O(2), if we have a list of n users we would have O(n) and O(2n).
I agree with @csarven 's change suggestion highlighting performance reasons for apps to read only one document. Performance is one aspect to consider, but there are other issues with this approach that should be taken into account, such as UX and DX considerations. From a UX perspective, it's a challenge to clearly communicate to users which of their profiles are editable and which are not, along with what actions they can take for each type. On the DX side, this approach needs more thought and clear guidelines and tools to help developers implement these considerations without adding significant complexity to their code. Implementors should consider the burden on the client when weighing the different options. Furthermore, this PR should explore the option of the server rejecting modifications to oidcIssuer (or any sensitive information), which is a viable approach that would relieve the burden on the client. I also agree that if the document is not modifiable by Solid protocol, this document is not applicable. |
A future dedicated PR can explore this hypothesis. This PR includes countermeasures already available today and used in at least one major Solid deployment. Sub-resource access control, as far as I know, is only available in Trinpod, and there is no open draft for it. The WebID Profile draft suggests this possibility, but I don't know of any existing implementations.
I assume that we are talking about some of the scenarios documented in https://github.com/solid/webid-profile/blob/main/notes/use-cases.md Which use cases don't fit a generic discovery mechanism, in lines of type indexes or SAI or future unified system? One more practical consideration: due to (ehm. unfortunate) 'slash semantics,' once a WebID Document is created in Solid Storage, it may be very hard to 'pull it out.' While this functionality is still underspecified, it is a safer approach not to create WebID Documents inside a Solid Storage. |
"title": "WebID 1.0", | ||
"publisher": "WebID Incubator Group" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...except that the WebID XG never published this, with or without a version declaration. It would be better to title it as what it was, "WebID 1.0 Editor's Draft"
(which must be read and understood as "Editor's Draft of WebID 1.0"
) ... and of course, the document now served from that location says "Draft Community Group Report 05 March 2014", which is similarly but differently inaccurate and contrary to the "publisher" identified here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case we can't quickly resolve it in this PR I created a dedicated issue for it #10
Co-authored-by: Ted Thibodeau Jr <[email protected]>
I would like us to agree and merge this PR during the CG plenary next week. If something being proposed would lead to non-conformance to Solid Protocol 0.11, we should focus on that. Otherwise, we should accept it as an available option and follow up with more pull requests, providing other options. If something is being planned for future Solid releases, that would make the proposed countermeasure lead to non-conformance. We can add an inline issue and track that change to the protocol while it is under consideration. |
Merging things which are known to be wrong is a bad idea. Too often, it leads to those wrong things being kept due to things like inertia and beliefs that "it wouldn't have been merged if it was wrong". There are a number of open change suggestions/requests here. They should optimally all be brought to consensus, or if consensus is deemed impossible turned into new issues for later resolution. Such issues will at least make plain that it was merged with known concerns, so future fixes will be more likely. |
I agree with @csarven and @VirginiaBalseiro that performance and DX issues should be taken into account. The diagram below outlines the major pathways and what they require. I do not support the sentence that says "There is no requirement that a WebID document ...". This may or may not be true in the future and is irrelevant. Just say that one possible counter-measure is to store the WebID document on something other than a Solid Resource Server. I also very much agree with them that having a server protect vulnerable triples while allowing write access to other portions of the profile should be mentioned. I agree with them that a Solid Specification can't apply directly to something that isn't a Solid server, however I disagree that we should say nothing on the subject. I believe that we should define a Solid WebID Profile as a document that lives on a Solid Resource server and is either directly de-referenced from the WebID URI or pointed to from the WebID Document with a solid:profileDocument (or foaf:primaryTopicOf) predicated pointing to a document on a Solid Resource Server. This is option #2 on the diagram. The current Solid WebID Profile draft spec is option #3 and is I believe, the wild west. @elf-pavlik I may have misrepresented SAI, please correct me. |
If the Solid Protocol requires hosting WebID in Solid Storage, then the suggested countermeasure would be invalid since it would lead to non-conformance. The only hard requirement for the proposed countermeasures is that they can not lead to non-conformance.
This is a distinct countermeasure; as of Solid Protocol 0.11, it is not specified, and as far as I know, no storage implementation supports it. Once we merge this PR, which defines the attack, please make a dedicated PR, and we can discuss all the details as well as any needed inline issues/notes in that dedicated PR.
Solid Storage (aka Resource Server) is just one of many Product Classes defined by the Solid Protocol or being dependent on. One external dependency is WebID and WebID Document. Solid Storage plays a role in a prerequisite of the described attack and is not used for hosting WebID Document in the proposed countermeasure. All the discovery approaches are out of scope for this PR; we should schedule STM to follow up on that. None of the approaches currently proposed and implemented require that the WebID Document be hosted in Solid Storage. If any proposal depends on it, we can even add an inline issue to that specific proposal. |
Then I guess you will not get my approval for this PR since I do not believe that issues of DX should be that last thing considered and I find it absurd to put the Solid Profile beyond the range of the Solid Protocol with no mention of how ito discover anything that is in the range of the Solid Protocol. As described in this PR it is entirely the prerogative of the IdP what goes in the WebID Document and where it points. That, to me, is contrary to the entire spirit of Solid. I believe the security warning about the WebID Document belongs in the WebID-Profile spec in a context which covers both the security concern and the discovery methods needed to work within the constraints imposed by those concerns. |
One of the reasons it is important to include discovery along with this warning is that the warning will mandate changes by server implementers. So, is NSS first going to remove the profile from the RS and then at some later stage decide what discovery methods it should provision the new non-RS profile with? That is not workable - we will end up with a variety of non-RS WebID documents with no consistency in what is in them and no way other than manual user intervention to modify them. Another aspect this PR fails to deal with is the issue of self-hosting. How will self-hosting work -? Will users need to run an IdP and an RS? |
The WebID-Profile draft is not part of Solid Protocol 0.11, and AFAIK, it is also not being proposed for the Linked Web Storage WG. However, Solid Protocol 0.11 depends on both Solid-OIDC and WebID drafts. https://solidproject.org/TR/protocol#normative-references And proposed countermeasure is conformant with Solid Protocol 0.11
I'm working on an extension for CSS that enables a configuration option which will create WebID documents outside of the storage. For people self-hosting CSS it will just require to use that configuration instead of what currently is a default config.
The countermeasure proposed in this PR doesn't mandate anything; it just informs and documents already broadly used practice. If NSS prefers another countermeasure, it can be submitted via dedicated PR. While it is not required to create that dedicated PR, I would be interested to know the implementation status of another countermeasure in NSS. |
Security Considerations for the discovery mechanism will need a few dedicated iterations; I created a first issue and will followup with an initial PR |
After listening to the discussion about this topic in today's CG meeting, here are my 2¢
I suggest to replace the countermeasure section with an issue block, explaining that the group is still seeking consensus on the proposed countermeasure(s). My feeling is that we are much closer to consensus on the first part, and that would allow us to make progress. |
index.bs
Outdated
|
||
### Countermeasures | ||
|
||
* There is no requirement to expose WebID Document via [[Solid.Protocol]] nor to host it in Solid Storage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's needed if the end user wants to use a Solid app to update their photo and contact details.
That's a pretty common thing to do.
So if the main WebID doc is not editable, then all info that a user would want to edit (like contact details and photo) should be in extended profile docs using rdfs:seeAlso
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I included it in the considerations section. I didn't mention rdfs:seeAlso
because I see it as a very poor choice. We should pick or mint a dedicated predicate for that. All those details are outside the scope of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mention
rdfs:seeAlso
because I see it as a very poor choice. We should pick or mint a dedicated predicate for that. All those details are outside the scope of this PR.
I very much agree that a dedicated predicate is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if the main WebID doc is not editable, then all info that a user would want to edit (like contact details and photo) should be in extended profile docs using
rdfs:seeAlso
And what happens if there is no rdfs:seeAlso in the WebID document? The user must manually request one? And suppose a different seeAlso is needed, does that go in the first rdfs:seeAlso and how many recursions are apps expected to go to find the seeAlsos in the seeAlsos? And if there are 10 seeAlso docs and the photo-album pointer is in the 10th - this means 10 loads instead of a maximum of one or two. There is no way for a user to prioritize public data by placing it where apps will see it first or second. Look at the diagrams I have posted above - with #2 (a dedicated predicated) there is a maximum of two loads to get to something prioritized. In #3, with only seeAlso, there is an unknown number of loads
Thank you,, I think this is the way to go. As it stands now the document recommends what may be the best security policy but is definitely the worst interoperability possibility. There are other avenues to explore that would both protect the sensitive data and allow some Solid Apps to modify the profile. For example, a server provider could name a couple of supported apps e.g. Penny or SolidOS which could modify the profile but forbid all other apps. I would approve this PR if the counter-measure section is dropped and it is a left as a future challenge for the community to come up with solutions. |
If that is truly the case then the counter-measure section should be removed since it depends on a predetermined solution to the discovery issue. You can't both refuse to discuss it and put forth a one-sided take on it. |
In what way does the countermeasure depend on data discovery, like type indexes or SAI authorization agents with agent registrations? |
Look at my diagram above. Your counter-measure (which I am not opposed to) makes choices. There are other possible choices and, even given the choice of that counter-measure, there are a number of unsolved discovery issues. If choices about discovery are out of scope, then do not favor one of those choices in your counter-measures. |
The proposed countermeasure does not depend on any data discovery mechanism. It is applicable even with no data discovery mechanism in place. On the other hand, some of the possible approaches in your diagram depend on something that is not guaranteed by the Solid Protocol. Specifically, the WebID Document being exposed via Solid Protocol. If you consider proposing that Solid Protocol add that guarantee, it could be linked as an open issue from the countermeasure proposed in this PR, which is entirely valid under Solid Protocol 0.11 I'm going to move the countermeasure into a separate PR so we can at least merge the attack. IMO, this will create the appearance that, after a decade, Solid Protocol has a fatal vulnerability with no available countermeasures, even though the countermeasure proposed in this PR is widely used and available to everyone who wants to start providing account bundles. Solid Protocol 0.11 does not specify data discovery mechanism; at least two proposals are on the table for future versions, both of which can work with the proposed countermeasure in place. |
This may be more helpful. If Solid Protocol draft was to have WebID-Profile as normative reference (dependency), and WebID-Profile draft required that WebID Document is hosted in Solid Storage. The proposed countermeasure would indeed be invalid since it wouldn't conform to that potential future version of Solid Protocol draft. As of Solid Protocol draft 0.11 (present), it does not depend on the WebID-Profile draft. Even more, the WebID-Profile draft could change that requirement before potentially becoming a dependency of the Solid Protocol draft. Even more interesting is that the WebID-Profile draft is not being proposed as input to the Linked Web Storage WG. This makes it hard for me to imagine how the final Technical Recommendation produced by the WG would depend on the WebID-Profile draft. Given all the above, I stand by my conclusion that the countermeasure proposed in this PR is fully valid under the current Solid Protocol 0.11, and I don't see a path where TR published by the WG would make it invalid. And even if it somehow did we can always adjust this draft here accordingly. I see a problem with the general lack of a clear strategy on how Solid Protocol is supposed to be composed of all the smaller specifications. I share some thoughts specifically on WebID-Profile draft in the: Again, all that is out of the scope of this PR. We don't do anything agile-like, where we make small incremental changes, refactor whenever necessary, and sometimes revert old choices based on experience gained in the process. We also don't do waterfall, where we have all the acceptance criteria defined upfront. I don't know what we do, but it doesn't seem to work. |
The last commit replaces the first documented countermeasure with an inline issue linking to this PR |
I am not OK with this being merged without addressing all the change requests. |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
partially addresses #3
preview | diff