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 draft #2

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

initial draft #2

wants to merge 4 commits into from

Conversation

elf-pavlik
Copy link
Member

I will setup rendering and publishing as separate PR #1

This PR supersedes solid/specification#625 and includes changes by @TallTed

extracted from solid/specification#598


preview

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Copy link

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Incremental tweaks...

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Copy link

@Otto-AA Otto-AA left a comment

Choose a reason for hiding this comment

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

Thanks for writing this up and making this repository!
I've added some small inline reviews.

In general, I think we should clarify that these attacks or only some examples of all the possible attacks that arise because of serving user-created applications on a Solid server. All the "Example issues" from the original issue (solid/specification#514) also apply (and likely some/many more, that I didn't think of). For instance, ESS fixed the Service-Worker issue in NRPT-2023-001, but this issue is not covered by the examples.

I think the specific examples can help to understand the general issue, however one should not think that they are exhaustive and cover all possible attacks.

I am not sure how we should address this here. Maybe, we could state that these are only some of the example attacks and not exhaustive for this security issue?

index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
Co-authored-by: A_A <21040751+Otto-AA@users.noreply.github.com>
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Copy link
Member

@csarven csarven left a comment

Choose a reason for hiding this comment

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

Probably important to understand or clarify underlying assumptions before building a whole set of potentially misleading problems on top of it.

index.bs Outdated
Comment on lines 46 to 48
An attacker could be an agent with a WebID or an application. They need append/write access to the user's server
to store a malicious HTML file. They might have this access because the user allowed them to write a blog post,
or because they have their own storage in a different path on the same domain.
Copy link
Member

Choose a reason for hiding this comment

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

How to reproduce this security issue:

  1. Give untrusted agent Append/Write access to storage.
  2. Goto 1.

Copy link

@Otto-AA Otto-AA Apr 9, 2024

Choose a reason for hiding this comment

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

  1. Give untrusted agent Append/Write access to storage.

From how I understand, you imply that the problem is already this step of giving Append/Write access.

If yes, I'd disagree with this. If you meant something else, can you clarify it?

Append/Write access seems like a simple requirement for an attacker, for various reasons.

Firstly, if I give someone access to append pictures to my /cats container, I would not expect this to be a security issue. In fact, if the attacker could gain full control over the pod just from Append access to a container, this would render access control rather useless. The Solid protocol / implementations should ensure that access control holds up, even with malicious agents.

Secondly, any CSS instance that is publicly open for registration would give an attacker the opportunity to host malicious html files on the same origin (solidweb.me, etc.). For ESS it's the same (at least for storage.inrupt.com). For NSS, other storages are only on the same site, not origin.

And thirdly, even if I only give agents access that I completely trust: They can also get hacked. If that happens, the impact of this should be as contained as possible. So we should only give them access to what they really need, and it should be hard for them to gain more access.

EDIT: in security terms, getting more access based from some initial access is called a "privilege escalation". Serving html/... files allows for privilege escalation if the server does not prevent it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you please clarify what you mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

@csarven, if I interpret your haiku correctly, you seem to suggest that Solid access control could be boiled down to a simple allow list of WebIDs, which would give every WebID listed full access to the storage.

index.bs Outdated
Comment on lines 46 to 48
An attacker could be an agent with a WebID or an application. They need append/write access to the user's server
to store a malicious HTML file. They might have this access because the user allowed them to write a blog post,
or because they have their own storage in a different path on the same domain.
Copy link
Member

@csarven csarven Apr 9, 2024

Choose a reason for hiding this comment

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

<script>alert()</script> in the literal of a concrete RDF syntax falls within the same realm of security concerns.

Actual security concern: Sanitise content before injecting into DOM or have a shape that validates the payload (e.g., to block or clear src, data, inline scripts, and so on.) so that the consumer doesn't inadvertently run stuff.

That's of course to prevent..

the agent that was somehow given append or write access permissions - read: trusted - to a storage who then turned out to be an "attacker" or unwillingly added malicious code won't cause chaos. /s

Copy link

Choose a reason for hiding this comment

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

<script>alert()</script> in the literal of a concrete RDF syntax falls within the same realm of security concerns.

I think we could separate RDF application security issues from HTML/SVG browser related security issues.

I definitely agree, that malicious RDF can be an issue and we need to be careful when interacting with this data. I've also written an article about this last year (which in retrospectively still makes important points, but maybe it is from a hardcore security perspective): https://github.com/Otto-AA/solid-security-basics.
This also goes beyond html injection: For instance, Penny blindly trusted the container data when making a recursive deletion, such that you could have tricked it to delete any other storage as well, not only child resources. So also for business logic in applications, it is important not to blindly trust RDF data.

However, I think we could keep this security issue separate from serving html files. I would see sanitization of RDF as a task for the application that uses and renders it (currently for web apps mostly). They fetch it and, if they want to render it, they should sanitize it.

For html files we cannot rely on apps to do this sanitization. If the HTML file is served publicly, the browser will render it and execute its malicious content. A way we could do this "sanitization" in browsers would be to tell it not to render the html, with CSP headers etc. Alternatively, maybe the server could return a html-rendering app that manually fetches the html and renders a sanitized version. However, in both of these cases, it would be the servers responsibility.

I think it would make sense to separate it with this distinction: security issues where apps are responsible and security issues where servers are responsible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would you like to add a recommendation on sanitizing any RDF Literal before using it in the DOM?
This could be a separate PR

Comment on lines +10 to +12
"authors": [
"Timea Turdean"
],
Copy link
Member

@csarven csarven Apr 10, 2024

Choose a reason for hiding this comment

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

Considering listing individuals in https://solidos.solidcommunity.net/Contacts/Group/SolidOS_Team.ttl or refer to the "SolidOS Team", e.g.:

Suggested change
"authors": [
"Timea Turdean"
],
"authors": [
"SolidOS Team"
],

@csarven
Copy link
Member

csarven commented Apr 10, 2024

I'd like to respond to some of the comments and request to do so before publishing the initial draft. (Looking into it today.. this week.)

I would also like to request a Special Topic Meeting on this PR because there are some fundamentals that I believe to be important to clarify and be generally on the same page - it is not just "make improvements later". Also important to clarify what's fundamentally different or new about the suggestions in this document that's really specific to Solid. And, I'd like @timbl @Otto-AA and others interested to be present. Ping @VirginiaBalseiro et al.

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Proposed
Development

Successfully merging this pull request may close these issues.

None yet

5 participants