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

initial WebID Document considerations #9

Merged
merged 3 commits into from
Jun 19, 2024
Merged

Conversation

elf-pavlik
Copy link
Member

partially addresses #3


preview | diff

@elf-pavlik elf-pavlik self-assigned this Jun 4, 2024

* 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.
Copy link
Member

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.
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 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.

Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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".

Copy link
Member Author

@elf-pavlik elf-pavlik Jun 7, 2024

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.

Copy link
Member

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
Comment on lines 151 to 152
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.
Copy link
Member

@csarven csarven Jun 4, 2024

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
Comment on lines 155 to 156
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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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.

Copy link
Member Author

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

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).

@VirginiaBalseiro
Copy link
Member

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.

index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@elf-pavlik
Copy link
Member Author

elf-pavlik commented Jun 4, 2024

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.

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 understand the comparison to containment triples in containers. Still, I would also notice that container descriptions are one of the solid permathreads, and there is a considerable degree of divergence between existing implementations. I would be cautious with using one controversial feature as a precedence for another feature.

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.

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?
If everything boils down to client-to-client discovery, I believe we should find a way to delegate it with a single predicate from the WebID Document, preferably set in the account bootstrapping stage. Otherwise, we can discuss the issues based on concrete real-world use cases.


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.

Comment on lines +37 to +38
"title": "WebID 1.0",
"publisher": "WebID Incubator Group"
Copy link
Contributor

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.

Copy link
Member Author

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

index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@elf-pavlik
Copy link
Member Author

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.

@TallTed
Copy link
Contributor

TallTed commented Jun 7, 2024

[@elf-pavlik] I would like us to agree and merge this PR during the CG plenary next week.

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.

@jeff-zucker
Copy link
Member

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.
webid-use-case

@elf-pavlik I may have misrepresented SAI, please correct me.

@elf-pavlik
Copy link
Member Author

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.

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.

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.

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.

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.

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.

@jeff-zucker
Copy link
Member

All the discovery approaches are out of scope for this PR;

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.

@jeff-zucker
Copy link
Member

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?

@elf-pavlik
Copy link
Member Author

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.

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

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?

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.

One of the reasons it is important to include discovery along with this warning is that the warning will mandate changes by server implementers.

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.

@elf-pavlik
Copy link
Member Author

Security Considerations for the discovery mechanism will need a few dedicated iterations; I created a first issue and will followup with an initial PR

@pchampin
Copy link

After listening to the discussion about this topic in today's CG meeting, here are my 2¢

  • there seem to be more or less consensus on the described exploit prereqs and attack, but
  • there is no consensus on the proposed countermeasure.

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.
Then we can discuss on how to replace the issue block with actual proposals, and focus on this side of the discussion.

index.bs Outdated

### Countermeasures

* There is no requirement to expose WebID Document via [[Solid.Protocol]] nor to host it in Solid Storage.

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

Copy link
Member Author

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.

Copy link
Member

@jeff-zucker jeff-zucker Jun 13, 2024

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.

Copy link
Member

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

@jeff-zucker
Copy link
Member

After listening to the discussion about this topic in today's CG meeting, here are my 2¢

  • there seem to be more or less consensus on the described exploit prereqs and attack, but
  • there is no consensus on the proposed countermeasure.

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. Then we can discuss on how to replace the issue block with actual proposals, and focus on this side of the discussion.

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.

@jeff-zucker
Copy link
Member

jeff-zucker commented Jun 13, 2024

All the discovery approaches are out of scope for this PR;

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.

@elf-pavlik
Copy link
Member Author

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?

@jeff-zucker
Copy link
Member

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.

@elf-pavlik
Copy link
Member Author

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.

@elf-pavlik
Copy link
Member Author

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.

@elf-pavlik
Copy link
Member Author

The last commit replaces the first documented countermeasure with an inline issue linking to this PR

@elf-pavlik elf-pavlik merged commit d2dc6d6 into solid:main Jun 19, 2024
@TallTed
Copy link
Contributor

TallTed commented Jun 19, 2024

I am not OK with this being merged without addressing all the change requests.

elf-pavlik added a commit that referenced this pull request Jun 19, 2024
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@elf-pavlik
Copy link
Member Author

Sorry @TallTed I added your suggestions in a separate commit 2de5442

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.

7 participants