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

Add advisement on applicability of security policy based on authentication state and resource semantics #8

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

Conversation

csarven
Copy link
Member

@csarven csarven commented May 31, 2024

No description provided.

…ation state and resource semantics

Co-authored-by: Virginia Balseiro <[email protected]>
@csarven csarven changed the title Add advisement on applicability of security policy based on authentic… Add advisement on applicability of security policy based on authentication state and resource semantics May 31, 2024
@elf-pavlik
Copy link
Member

@csarven can you please provide some concrete examples showing how this paragraph is applicable to the described attack?

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

csarven commented Jun 5, 2024

It had to do with the general considerations with the countermeasures in mind. The point was that the server may not want to blindly apply them especially if it is inapplicable, e.g., if user is unauthenticated, they can't perform properly authorized append/write operations any way. Applying the countermeasures on a given resource may also imply that the server has some knowledge of the resource semantics. That may be true in some cases but not all, or at least there is no particular expectation. So, best to rule out inapplicable cases so that potential features are not diminished.

Aside: the Considerations sections in this PR and the whole document needs a revisit I think. It probably should be on the same level as Attack, Prerequisites, Countermeasures, as opposed to a subsection of one of those.

@elf-pavlik
Copy link
Member

If we want this document to serve as accessible educational material for intended audiences to make educated choices, we should avoid vague and overly generalized suggestions.

Having clear, reproducible attacks and different actionable countermeasures makes it approachable to implement them. Whenever a countermeasure has possibly undesirable consequences (side effects), we should document them along with the specific countermeasure.

Could you pinpoint how the sentence you propose in this PR applies to a specific already documented attack on one of the included countermeasures? If it applies to an attack and countermeasure, which still needs to be documented, it should be included in the same PR.

@csarven
Copy link
Member Author

csarven commented Jun 6, 2024

we should avoid vague and overly generalized suggestions

e.g., precisely the effects of blindly slapping CSP: sandbox as a countermeasure.

I don't find emphasising that the reader reviews the actual applicability to be "vague". What is currently being generally recommended as a countermeasure is vague. In fact, it (this PR) aims to be more specific by stating that this applies to a specific authentication state as well as the knowledge of the resource semantics.

So, thinking further, I think it would be more appropriate to have the Prerequisite section cover this up front. Do you have an objection to that before I update the PR?

@elf-pavlik
Copy link
Member

In fact, it (this PR) aims to be more specific by stating that this applies to a specific authentication state and knowledge of the resource semantics.

The draft currently has two distinct scenarios. Could you refer to the specific steps in those scenarios and explain how your suggestion applies to them?

Comment on lines +118 to +119
Servers are encouraged to check the applicability of security policies based on the user's authentication state as well as resource semantics. Some attacks might only be applicable for authenticated requests, so functionality restrictions could unnecessarily prevent non-affected users from using certain features.

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
Servers are encouraged to check the applicability of security policies based on the user's authentication state as well as resource semantics. Some attacks might only be applicable for authenticated requests, so functionality restrictions could unnecessarily prevent non-affected users from using certain features.
Servers are encouraged to check the applicability of security policies based on the user's authentication state as well as resource semantics. Some attacks might only be applicable for authenticated requests, so functionality restrictions could unnecessarily prevent non-affected users from using certain features. Servers should therefore focus on applying these countermeasures only to authenticated requests.

@elf-pavlik
Copy link
Member

The request is never authenticated if a user navigates a browser to a resource (e.g., HTML hosted on Solid Storage). Only requests made from Javascript, which uses something like a solid-old client, can be authenticated.

This does not seem relevant to applying the CSP sandbox to HTML pages since those are always requested without authentication.

It would be really helpful if you could showcase how your suggestion applies to one of the two documented attacks.

@VirginiaBalseiro
Copy link
Member

This does not seem relevant to applying the CSP sandbox to HTML pages since those are always requested without authentication.

I am confused. That is exactly what this PR is trying to clarify.

It would be really helpful if you could showcase how your suggestion applies to one of the two documented attacks.

It does not. It applies to the countermeasures.

@elf-pavlik
Copy link
Member

This does not seem relevant to applying the CSP sandbox to HTML pages since those are always requested without authentication.

I am confused. That is exactly what this PR is trying to clarify.

Does the proposed sentence suggest applying a CSP sandbox on unauthenticated requests or only on authenticated requests?

If we can clarify how it applies to the described scenarios by Wednesday, we could bring it up during CG weekly and attempt to agree on how to move forward with this PR.

@csarven
Copy link
Member Author

csarven commented Jun 18, 2024

If there is no attack when user is unauthenticated, is there a reason to slap CSP on? That was the point. More generally the authentication state in which the user is in in conjunction with the resource semantics. So, the server probably shouldn't give an overarching "countermeasure" if the server has no clue about the either. It should know exactly why and when it is doing it..

@elf-pavlik
Copy link
Member

If there is no attack when user is unauthenticated

I assume that you don't refer to the user being authenticated with the client but to either the client or the user agent making an authenticated or unauthenticated request to the resource server (storage)

If we look at https://solid.github.io/security-considerations/#use-solid-oidc-dpop-bound-id-tokens-to-make-authenticated-requests

The request made to the HTML by navigating the browser is unauthenticated; currently, Solid only specifies making authenticated requests with a client that sets access tokens in the Authorization header.

If the CSP sandbox wasn't set in response to that unauthenticated request, it could open a possibility that the executed javascript will be able to make authenticated requests on behalf of the attacker.

So, the described attack works with a browser requesting HTML with an unauthenticated request.

@csarven
Copy link
Member Author

csarven commented Jun 18, 2024

I meant the user when I said the user. That is the the user/victim that's being referenced in prerequisites and attack. One becomes a victim because they're authenticated no? Is there an attack to speak of if the user is not authenticated?

@elf-pavlik
Copy link
Member

elf-pavlik commented Jun 18, 2024

One becomes a victim because they're authenticated no?

https://solid.github.io/security-considerations/#use-solid-oidc-dpop-bound-id-tokens-to-make-authenticated-requests

Once the victim is logged in via [SolidOS], any new session is automatically logged in to the same account.

So, there is an authenticated session between the user and SolidOS, which acts as the client.

At the same time

The attacker writes a malicious text/html file to the server. When this file is opened by the user

The request for that HTML is not made under any authenticated session.


No global binary state exists where the user is either authenticated or not. When you refer to authentication, please be specific about which parties and during which interactions that authentication plays a role.

@csarven
Copy link
Member Author

csarven commented Jun 18, 2024

The suggestion here is that a clarification is made about the attack and the countermeasure. As also mentioned elsewhere, if you want a set of particular countermeasures, clarify why or in which scenarios in particular the chosen ones among other possibilities are actually meaningful. As it stands, countermeasures are being slapped on in cases where they may not be applicable. Again, that's the clarification that's being sought for considerations to take into account. If you don't want considerations or okay to lock things down rrgardless, just say so, so we can skip some discussions. You are the editor of this document, you have sufficient material on the table to work with and the responsibility to make sense of what should be expressed or not with it. If you don't want to take the PR because it is faulty or can't be worked in, edited, or whatever, just note that down and close the PR.

@elf-pavlik
Copy link
Member

elf-pavlik commented Jun 18, 2024

I don't understand how the paragraph you suggested applies to the described scenarios. I need to understand it first before I can even agree or disagree with it.
Could you simply explain how what you suggest in this PR can be put in practice in at least one documented attacks with its countermeasures? Since currently there is only one with CSP sandbox let's clarify how does the authentication play role in the request where the response would set that CSP header.

@csarven
Copy link
Member Author

csarven commented Jun 19, 2024

  1. On editorial level, the Considerations sections needs a revisit: Fix Consideration subsection per attack section #11 . That may or may not be playing a part in confusions / miscommunications but it wouldn't hurt to improve the structure.
  2. I'm not sure what more needs to be said about this PR / clarification on consideration. As said, it is a reaction / clarification to the chosen countermeasure. The added consideration can be taken specific to the scenario or even generally for all attacks.
  3. The chosen countermeasure is saying "shut it all down". The consideration is saying "understand when it is applicable or implications".
  4. The document is literally about considerations, not "requirements" nor "best practices". Why is mentioning / clarifying the implications of any action needs to be absolutely quantified?
  5. The prerequisites attacks rely on user's authentication state - however that's understood / tracked / qualified - so, is it really incomprehensible to clarify the considerations / implications?
  6. The implications of action or no-action needs to be better clear because as it stands the document is waving "do x because of y" and not enough "take care of doing x because it fudges up z".

Let me re-emphasise the last point above (which was already brought up numerous times in other discussions). There are short-term and long-term implications of the called "security considerations".

For the short-term: the implications are that a category (or more) of use cases may be disabled or needs to be done in different ways without any overly concrete or successful alternatives. That's either not communicated enough in this document or just being chalked up as "oh well" possibly because those use cases are not of interest to them so they don't care whether Solid disables them or not. This is both bias and short-sighted, and it will be apparent soon enough (if not already obvious by now).

For the long-term: the implications will eventually render Solid utterly useless to predominant publishing practices / interactive documents. The argument that's being pitched is along the lines of "Solid will do it the right way"(TM) but all things considered, it hasn't even showed comparable applications or a coherent system on how it will actually play out. So, being a touch humble about what's actually possible and not tell everyone to stop what and how they are doing and do it this other way may go a long way.

Which is generally why we need to take care on "considerations" and not just render the system useless in the name of security. Because do you know what's super secure? Don't let the user near an input device.

@elf-pavlik
Copy link
Member

elf-pavlik commented Jun 19, 2024

@csarven please see my first comment, which I posted 3 weeks ago #8 (comment)

can you please provide some concrete examples showing how this paragraph is applicable to the described attack?

I see that you put a lot of time into the back-and-forth on this issue. I don't understand why you can't simply illustrate what you suggest with a concrete example based on the scenario already included in the document.

Since you mention use cases, this document is practically structured like a set of use cases. You could simply add an alternative flow based on an existing flow. Which would add two points, for example:

  • the potential victim navigates their browser to the HTML planted by the attacker
  • the server "check the applicability of security policies based on the user's authentication state as well as resource semantics"
  • based on presence of auth state X and resource semantic Y does Z

I see a real need for something as simple as above to avoid us taking another 3 weeks for this PR.

@@ -115,6 +115,8 @@ The attacker writes a malicious `text/html` file to the server. Depending on the

Servers are strongly encouraged to consider the countermeasures in the context of the use cases they want to enable or disable on a given storage. For instance, using `Content-Security-Policy: sandbox` will universally prohibit various functionalities for applications, including but not limited to accessing local storage, executing scripts, using forms, interacting with plugins, or including external content. This broad range of restrictions may not be desirable for various categories of applications that rely on client-side storage mechanisms, collaborative features, or dynamic content interaction.

Servers are encouraged to check the applicability of security policies based on the user's authentication state as well as resource semantics. Some attacks might only be applicable for authenticated requests, so functionality restrictions could unnecessarily prevent non-affected users from using certain features.

Choose a reason for hiding this comment

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

Can you give an example?

@csarven
Copy link
Member Author

csarven commented Jun 20, 2024

Then we don't understand each other about what constitutes a consideration or acknowledging the implications. Or at the very least what this general advisement is even saying. You're attempting to turn what's expressed as a general consideration about the implications of the countermeasure into an algorithm / "example". That wasn't the intention to begin with. So, to answer your question, that's precisely why you're not understanding what's being said because I don't think you're digesting the information given to you in a fair and balanced way.

I've raised the need to revisit the structuring the Considerations sections too, including specific considerations per attack as well as an overarching one for the document. That is part of editorial work to figure things out to improve clarity. Perhaps we need additional editors.

I'm not sure what more I can add to this discussion at this point. Feel free to dismiss the PR, merge the PR, split its content into separate statements or whatever...

@elf-pavlik
Copy link
Member

You're attempting to turn what's expressed as a general consideration about the implications of the countermeasure into an algorithm / "example". That wasn't the intention to begin with.

If something is generally applicable, it should be possible to show how it applies in at least one specific case. If it applies to various scenarios that are not yet in the document. Let's start with one concrete example and work from there.

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.

5 participants