-
Notifications
You must be signed in to change notification settings - Fork 44
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 security consideration about information exposure #228
Conversation
Co-authored-by: Justin Bingham <justin.bingham@janeirodigital.com>
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.
Responding to an “authorized” request should not return “unauthorized information”? The inverse would make sense to me, but this does not at the moment. Also the phrasing is too vague even if I’d agree with the content.
@RubenVerborgh Retake: 7ec8228 - Better? The intention of the security consideration is for servers to not expose information beyond what can be controlled by an agent through access policies. Thus far the system left a gap in which this PR attempts to close. For example, the initial text "authorized" referred to the target resource of a request while may be granted, it is possible that the response can include other information that the agent is unauthorized for because of the way server generates a representation. This is especially the case when a server can potentially expose composite or aggregate information about other resources (than the target of the request) eg. description of contained resources or links to certain auxiliary resources (eg. to an ACL document). As it appears to be case in for example: nodeSolidServer/node-solid-server#1567 |
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@@ -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> |
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 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.
- Does that mean I cannot send out
- 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.
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 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.
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.
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
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.
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.
Merged this as is for the time being. Will follow-up with improvements/corrections. For starters. the security consideration section needs to be revised into non-normative, and all normative text can move elsewhere. |
Some related issues in which this topic is touched upon: