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.

Sign up for GitHub

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

Jump to bottom

Add security consideration about information exposure #228

Merged
merged 6 commits into from
Apr 6, 2021
Merged

Add security consideration about information exposure #228

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6435af2
Add security consideration about information exposure
csarven Feb 5, 2021
e790798
Update protocol.html
csarven Feb 5, 2021
c5718f2
Paraphrase unauthorized exposure (by @d-a-v-i-- )
csarven Feb 5, 2021
391bbd8
Minor
csarven Feb 5, 2021
7ec8228
Do not expose information than what's necessary
csarven Feb 7, 2021
e037679
Update protocol.html
csarven Feb 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions protocol.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,8 @@ <h3 property="schema:name">Security Considerations</h3>
<p>Solid data pods <a href="#cors-server">disable all cross-origin protections</a> in browsers because resource access is governed explicitly by <a href="#web-access-control">Web Access Control</a>. As such, data pods MUST NOT rely on browser-based cross-origin protection mechanisms for determining the authentication status or representation of a resource. In particular, they MUST ignore HTTP cookies from untrusted origins. Additional security measures MAY be taken to prevent metadata in error responses from leaking. For instance, a malicious app could probe multiple servers to check whether the response status code is <code>401</code> or <code>403</code>, or could try to access an error page from an intranet server within the user agent’s private network to extract company names or other data. To mitigate this, when a request from an untrusted <code>Origin</code> arrives, the data pod MAY set the status code of error responses to <code>404</code> and/or anonymize or censor their contents.</p>

<p>Data pods SHOULD use TLS connections to protect the contents of requests and responses from eavesdropping and modification by third parties. Unsecured TCP connections without TLS MAY be used in testing environments or when the data pod is behind a reverse proxy that terminates a secure connection.</p>

<p>Servers MUST NOT expose information beyond the minimum amount necessary to enable a feature. For example, when a <code>GET</code> method request targets a container, the server MUST NOT include information beyond containment statements about the contained resources in the response. Examples of what is not allowed without proper authorization include size, type, creator, label, and last modification time.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is non-enforceable as currently written.

  • What is "information beyond the minimum amount necessary"?
    • Does that mean I cannot send out X-Powered-By headers?
      • I know this is not the intention, but there's nothing that tells me either way.
  • The example is enforceable; but I think this should then be the requirement, or part of the set of requirements.
  • What does "without proper authorization" mean?
    • Is it per resource?

Another concern is that checking this quickly becomes expensive.
A valid way to satisfy this requirement seems to not output these things ever, at all. But that will break applications.

Copy link
Member Author

Choose a reason for hiding this comment

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

The purpose of the first statement is to set an understanding about server's behaviour ie. to preserve privacy by default. It is a consideration for that reason, and not something intended to be enforceable in and itself. Examples that follow will fill in what's exactly required / enforceable. We need to add these general or specific considerations as they come up.

This paves the way to writing up the "Security and Privacy Review" section (as per #147 ).

What is "information beyond the minimum amount necessary"?

https://www.w3.org/TR/security-privacy-questionnaire/#minimum-data

Does that mean I cannot send out X-Powered-By headers?

If server considers X-Powered-By unnecessary (or risky) for unauthorized agents, it shouldn't expose it.

The example is enforceable; but I think this should then be the requirement, or part of the set of requirements.

It would be a requirement ( https://solidproject.org/TR/protocol#conformance ). I'm open to moving text around later on - and something like the example could go under Containment or Representations or somewhere else... but, for the time being, it is useful to have these type of considerations under Security (or Privacy) where it can all be checked. After all, it is completely possible to move Considerations to other parts of the spec, but that's not necessarily best way of going at it.

What does "without proper authorization" mean?

Privileges being acquired within established parameters, so "proper" eg. would be if you are authorized to read the contained resources via what the protocol offers, and did not obtain it by other means.

Is it per resource?

In that example, yes (eg. read on container, but may not have read on some of the contained resources.)

Another concern is that checking this quickly becomes expensive.

Sure, it depends on the system and whether it wants to expose information to unauthorized users. The proposal here is based off the foundations or principles. It doesn't render the system useless. The point is that if a system wants to make that information available, it should do so based on the constraints. We can throw away the generally-agreed principle(s) or make cases for why some information doesn't fall under this category - that's also fine.

A valid way to satisfy this requirement seems to not output these things ever, at all.

Right, that'd be one way. But consider the issue #227 raising the use cases for the minimal/useful information for applications to know. It is orthogonal to the consideration here. Even if for example 227 resulted in requiring certain information to be part of the container representation or needing querying or paging or something, the Consideration is still useful.

But that will break applications.

To be precise, that will "break" existing applications relying on implementation specific feature. We need to first address whether that feature is wanted and then how it will work: issue 227 etc.

Happy to revise the text but it would still be indefinitely open to improvements. I suggest that if the general proposal is agreed, we can edit through as we go. More information will come in so we'll also need to look at it as a whole.

Copy link

Choose a reason for hiding this comment

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

Improper authorization usually means a control failure or bypass. In other words you are meant to be prompted for authorization but maybe you found a way to avoid being prompted, such as a bug in consistency or a weakness. Sometimes it can mean implied, although more often because of auditability it's a formal one. CWE language is "incorrectly performs an authorization check" https://cwe.mitre.org/data/definitions/285.html

Copy link
Contributor

Choose a reason for hiding this comment

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

if the general proposal is agreed

I'm not happy with neither the general proposal nor the specific example, but I don't think I will ever be. I'll just lift my reject and let others work it out.

</div>
</section>

Expand Down