Node-solid-server: Regression bug in v5: Need to check for a file's ACL when it is served as resource for a container

[megoth]

When a file is served as response to dereferencing a container, we also need to check that file's ACL settings when determining whether a user has access to that resource.

How it works on v4: as a visitor I have read access to both https://megoth.solid.community/ and https://megoth.solid.community/index.html
But on v5: as a visitor I have read access to https://megoth-test1.dev.inrupt.net/index.html, but not to https://megoth-test1.dev.inrupt.net/

[kjetilk]

So, thinking about it, I'm thinking that we should be using /.acl even if the server serves up /index.html.

The default ACL file says / is public, so could you double check that your /.acl says that?

[megoth]

Sorry, forgot to answer this.

The template for root ACL is

# Root ACL resource for the user account
@prefix acl: <http://www.w3.org/ns/auth/acl#>.

<#owner>
    a acl:Authorization;

    acl:agent <{{webId}}> ;

    # Optional owner email, to be used for account recovery:
    {{#if email}}acl:agent <mailto:{{{email}}}>;{{/if}}

    # Set the access to the root storage folder itself
    acl:accessTo </>;

    # All resources will inherit this authorization, by default
    acl:default </>;

    # The owner has all of the access modes allowed
    acl:mode
        acl:Read, acl:Write, acl:Control.

# Data is private by default; no other agents get access unless specifically
# authorized in other .acls

So the default ACL file for root says that all files are private.

[megoth]

So making the root folder publicly available is a solution for this, that is changing default_templates/new-account/.acl to:

# Root ACL resource for the user account
@prefix acl: <http://www.w3.org/ns/auth/acl#>.
@prefix foaf: <http://xmlns.com/foaf/0.1/>.

<#owner>
    a acl:Authorization;

    acl:agent <{{webId}}> ;

    # Optional owner email, to be used for account recovery:
    {{#if email}}acl:agent <mailto:{{{email}}}>;{{/if}}

    # Set the access to the root storage folder itself
    acl:accessTo </>;

    # All resources will inherit this authorization, by default
    acl:default </>;

    # The owner has all of the access modes allowed
    acl:mode
        acl:Read, acl:Write, acl:Control.

# Data is private by default; no other agents get access unless specifically
# authorized in other .acls

# The public has read permissions to root folder
<#public>
    a acl:Authorization;
    acl:agentClass foaf:Agent;
    acl:accessTo <./>;
    acl:mode acl:Read.

This does not give access all other files in the root folder, so I think this the "right way" of fixing this.

The problem with this approach (which I think is correct) is that users with outdated root ACLs will need to upgrade their root ACL if they want their homepage be publicly available. The reason this works in v4.x seems to _actually be a bug_...

We might want to create a migration script for POD providers to run when upgrading to v5. Any thoughts on this?

[csarven]

Can you elaborate on "This does not give access all other files in the root folder"? meanwhile #public with acl:Read?

Why not keep private by default and give read to the index or whatever you want explicitly?

[mikeadams1]

I am also confused here as well because, @megoth I thought you stated that the acl issue with regards to home pages being public was an err in NSSv5 but, it appears that your position now is that it was a bug in v4.x that the home pages were public and should not have been, is this correct?

[mikeadams1]

If I am mistaking the discussion here, just let me know.

[RubenVerborgh]

What is being passed to the ACL checker? / should be passed in the above case, not /index.html.

[megoth]

I am also confused here as well because, @megoth I thought you stated that the acl issue with regards to home pages being public was an err in NSSv5 but, it appears that your position now is that it was a bug in v4.x that the home pages were public and should not have been, is this correct?

My understanding of the problem evolves, so my hypothesis was something else initially. Is also why I'm stating how I understanding of the problem here, in hopes of getting feedback and maybe correct me if I'm still not understanding the problem correctly.

[megoth]

Can you elaborate on "This does not give access all other files in the root folder"? meanwhile #public with acl:Read?

Why not keep private by default and give read to the index or whatever you want explicitly?

Just to note, this ties into https://github.com/solid/web-access-control-spec/issues/36 as well. I tried changing the last part of the root-ACL to:

<#public>
    a acl:Authorization;
    acl:agentClass foaf:Agent;
    acl:accessTo <./index.html>;
    acl:mode acl:Read.

That doesn't work, I'm afraid =/

As I have understood the ACL-spec, I need to make the root-folder readable to make it accessable to the public. But the previously mentioned change would only make / readable, not other resources that resides in root (e.g. /test.txt will not be readable unless you have /test.txt.acl that says it should be).

If I had added acl:default as well, that would be different, i.e.

<#public>
    a acl:Authorization;
    acl:agentClass foaf:Agent;
    acl:accessTo <./>;
    acl:mode acl:Read.

That would be different, as it would make _all_ resources in root (and other sub-folders that doesn't have an .acl) publicly readable.

Does that make it clearer?

[megoth]

What is being passed to the ACL checker? / should be passed in the above case, not /index.html.

Yes, we're passing /; my initial approach to this issue was actually to change it to pass /index.html (or /index.ttl or whichever resource that is "actually" passed to the client) to the ACL checker, but if I understand the ACL spec correctly, that is not how we should do it.

Am I understanding that part correctly?

[RubenVerborgh]

Yes, we're passing /

Is the problem specific to just /, or do we have the same situation with /foo/ and /foo/index.html?

but if I understand the ACL spec correctly, that is not how we should do it.

Indeed.

[megoth]

Another effect of this is that you can actually make the root publicly readable, but not the default file that the server ends up serving (by root ACL stating that / is readable, but index.html.acl says that /index.html is not publicly available), e.g. you could visit https://megoth.solid.community/ but not https://megoth.solid.community/index.html.

[megoth]

Yes, we're passing /

Is the problem specific to just /, or do we have the same situation with /foo/ and /foo/index.html?

/foo/ (and /foo) and /foo/index.html is not publicly readable by default. (Neither with my proposed change to state that / is publicly readable by all.)

[RubenVerborgh]

/foo/ (and /foo) and /foo/index.html is not publicly readable by default.

Indeed, but what I mean is: if we set similar permissions on them, would they have the same problems? I guess /public/ and /public/index.html are in that case (if an index.html exists).

[RubenVerborgh]

I indeed think that https://github.com/solid/node-solid-server/blob/v5.0.0-beta.5/default-templates/new-account/index.html.acl is the problem; this should not be index.html.acl but .acl. Permissions should be set on /, not on /index.html.

[megoth]

/foo/ (and /foo) and /foo/index.html is not publicly readable by default.

Indeed, but what I mean is: if we set similar permissions on them, would they have the same problems? I guess /public/ and /public/index.html are in that case (if an index.html exists).

Yes, we could get the same problem, e.g. if /foo/index.html has specific ACL (i.e. /foo/index.html.acl) that makes it publicly readable, you would get a 200 for /foo/index.html, but you would get a 401 for /foo/ (and /foo).

[megoth]

So the fix could be to make / and /index.html publicly readable in /.acl, and remove /index.html.acl? (Since they could diverge and make it confusing.)

[RubenVerborgh]

We need two fixes:

1. new accounts indeed should not have any rules for *index* files
2. old accounts should continue working

The second thing is problematic. We do not want to start changing file contents; that sets a bad precedent. (We do change internal/external filename correspondence, but that does not touch their contents from an HTTP perspective.)

I think our only course of action here is to code the buggy behavior for the /index.html file (and only those) as an exception in NSS, possibly behind a flag. Not doing this would break homepages. I'm fine with breaking other resources though; the user can repair them when needed, and I expect few (if any) cases to exist in the wild.

[megoth]

I think our only course of action here is to code the buggy behavior for the /index.html file (and only those) as an exception in NSS, possibly behind a flag. Not doing this would break homepages. I'm fine with breaking other resources though; the user can repair them when needed, and I expect few (if any) cases to exist in the wild.

That is a pragmatical solution that I think would make most users happy. It would require us to live with a bug (as it diverges from how it _should_ be handled), but documenting it and being aware of it is an ok solution in my mind.

[RubenVerborgh]

Indeed. Perhaps not even behind a flag, just an exception.

[RubenVerborgh]

Fixed the template part in https://github.com/solid/node-solid-server/pull/1065.

[mikeadams1]

@megoth I understand what your saying, and trust that you all are making the best decision. Thank you all for your attention to this matter.

[kjetilk]

@megoth and I discussed this a bit in person, as there are many issues around this. One is that in 4.4, the root ACL isn't respected, i.e. users are granted access to root without an authorization in the ACL system. That this has been changed in the NSS5 beta series is a consequence of that we have been tightening up the security. So, I expressed my discomfort with adding code that emulates old behaviour that does not respect the ACL system, and therefore, I don't want to merge #1067 .

What we can easily do, is to grant public readability by default to root, #1065 will do that, and so I propose to merge that.

Further, I don't think that we as developers should force public read on anything where it is not expressed in the ACL. That could lead to user's information being compromised. However, we also recognize that for most users, a public root is what they expect. I think the solution to this is to provide a script that POD providers can run to add an authorization for public read to root if it is not there. POD providers should know their users better than we do, so it should be their call.

So, that's my opinion anyway. Does it sound reasonable?

[RubenVerborgh]

I haven't been able to look at #1067 in detail, but I think it is much simpler than that.

We have to distinguish between legacy and future. #1065 fixes the future indeed, no need to change things there.

However, fixing the ACL system (as we do in v5) breaks the incorrect ACLs on the 100.000+ pods already out there which were created with a broken ACL system in place.

So what I am proposing is not

we as developers should force public read on anything where it is not expressed in the ACL

What I am proposing is:

• specifically (and only) for the legacy case where an ACL file named /index.html.acl gives read permissions to /.index.html, we interpret it as read permissions to /.

This provides continuity, without many practical (instead of theoretical) security implications.

[kjetilk]

That is basically what #1067 does, but in a more general way.

Indeed, we could constrain that to just index.html, but I think there is potential for confusion when people are setting their ACLs and they are not reflected in the actual permissions. I don't think we should have that, instead, I think a better solution is for POD Providers to add an authorization by a script. Same effect, much less hacky and error prone.

So, basically, this is a choice between

1. an awful hack in the code that could add confusion and security problems
2. allowing POD providers to add an authorization to user's file
3. leave it to users to set the authorization themselves.

The options 1. and 2. have the same effect, but as a choice between evils, I think the latter is the lesser. I think 3. is a real option too, although it will certainly be surprising to many users that they have to, but it is the nature of a prototype.

[RubenVerborgh]

Indeed, we could constrain that to just index.html

What concrete problems would that bring?
It is a hack, but a really good one.

The alternatives include:

1. breaking 100,000 homepages
2. changing people's ACL files

The first is so horrible it can drive people away ("Solid is the thing that broke my homepage on an upgrade").
The second is so horrible that it gives people a reason to never trust us again ("they said control, but they changed my ACL file").

I think there is potential for confusion when people are setting their ACLs and they are not reflected in the actual permissions.

We can make it really elaborate:

• IF there is a file called /index.html.acl file
• IF it contains a read-only rule for the public on /index.html
• IF /.acl does _not_ contain a rule for the public on /
• THEN we apply the relaxation that read-only access also applies to /.

So as soon as people have modified anything to their ACLs, the above conditions will be void.

I think a better solution is for POD Providers to add an authorization by a script.

We should not change exposed document contents; they are the users' files, not ours.

[megoth]

Indeed, we could constrain that to just index.html

What concrete problems would that bring?
It is a hack, but a really good one.

I think it's a bad hack, but I think it's the best option we've got.

The alternatives include:

1. breaking 100,000 homepages
2. changing people's ACL files

The first is so horrible it can drive people away ("Solid is the thing that broke my homepage on an upgrade").
The second is so horrible that it gives people a reason to never trust us again ("they said control, but they changed my ACL file").

The first one is something we really should avoid. So some sort of action is preferable.

I'm open to the the script solution though, with the instructions being as follow:

1. Iterate through all accounts
2. If root ACL do not grant READ access to root, add rule that grants it

As a script this allows the POD providers to make the decision if they should do it.

That being said, I'm very skeptical as it violates our rule to never change the content of a POD. So this is a bad solution, but it is a pragmatical solution that works for 99.99% of the accounts...

I think a better solution is for POD Providers to add an authorization by a script.

We should not change exposed document contents; they are the users' files, not ours.

I agree with @RubenVerborgh here, but I'm really conflicted in this situation TBH...

[megoth]

Just to note, there is a third option: Create an API that allows users to reset access to a given resource. This could also be useful for issues where users for various reasons have been able to lock themselves out of control on a given resource.

This solution needs some thought though:
1) We should verify that the user is willingly sending the request, e.g. require them to verify via email, like we do for resetting passwords;
2) We cannot allow users control of special resources like serverSide.ttl, that should not be controllable by user.

It is also probably not doable to ask 100000s of users to reset the control of their root ACL...

[kjetilk]

Indeed, we could constrain that to just index.html

What concrete problems would that bring?
It is a hack, but a really good one.

I don't think so, consider that the user removes the public authorization on the root ACL, and puts some confidential material in there, not realizing that the index.html.acl overrides it, and so has confidential information exposed. It is a horrible hack, with flaming antlers, the smell of phosphorous, sulfur and dead fish. And then some. :-)

Moreover, wouldn't it create implications for the spec and therefore other implementations?
Now, it means that for foreseeable future, you would not require a rule for public on / to grant access.
Say that somebody takes their 4.x-era Pod with a /.acl that does not contain a rule for public on /, and wants to move that to a host with a different implementation, they would experience the same breakage. It seems to me that this breaks interoperability for everyone who signed up in 4.x-time. They need to have their root ACL fixed, that is the only future-proof, interoperable and secure solution. The question is how we can fix that.

The alternatives include:

1. breaking 100,000 homepages

2. changing people's ACL files

The first is so horrible it can drive people away ("Solid is the thing that broke my homepage on an upgrade").
The second is so horrible that it gives people a reason to never trust us again ("they said control, but they changed my ACL file").

But we also said prototype... At least none of those will surprise any of our next billion users, which is a greater concern, there is a very real possibility that this hack will expose confidential information even in a rewritten and secure server, because of the bad security usability it represents.

Also note that there is something browser-specific, curl gives 401 also on 4.4. It might be that the impact is lower than we think. Do we really know that it is the intention to have that public readable?

I think there is potential for confusion when people are setting their ACLs and they are not reflected in the actual permissions.

We can make it really elaborate:

* IF there is a file called `/index.html.acl` file

* IF it contains a read-only rule for the public on `/index.html`

* IF `/.acl` does _not_ contain a rule for the public on `/`

* THEN we apply the relaxation that read-only access also applies to `/`.

It wouldn't help the neither of the above. What would help is if we wrote any changes to /.acl with acl:accessTo </> to /index.html.acl too...

Complicates stuff, adds a hack that we would need to keep indefinitely, and can surprise people who assumes that /.acl and /index.html.acl are two different resources, but that's probably more marginal.

Or, could we have an explicit sunset on this hack? I.e., we do this for 5.0.0, but warn our users that they need to set read permission on their root, because this will be removed in the future?

Have some UI on the frontend, saying "You do not have an explicit public read permission on your root. For now, we allow public access anyway, but that will only be in the case in the Solid Server prototype, so if you want to continue having your homepage public-readable, set it".

That should also warn them if it is unintentional.

[mikeadams1]

"Or, could we have an explicit sunset on this hack? I.e., we do this for 5.0.0, but warn our users that they need to set read permission on their root, because this will be removed in the future?"

I think that this is the best course of action seeing as it should be the responsibility of the pod owner to be explicitly familiar with and manage there own acl.

[RubenVerborgh]

"Or, could we have an explicit sunset on this hack? I.e., we do this for 5.0.0, but warn our users that they need to set read permission on their root, because this will be removed in the future?"

Good idea. Practical problem would be lack of a place to show such a notification.

Even better perhaps is combining this with something I read earlier: making this a button "Set permission" so it is zero effort.

[mikeadams1]

@RubenVerborgh Agreed, make it so!

[kjetilk]

OK, good! I think we have consensus!

[mikeadams1]

Agreed!

[kjetilk]

BTW, @RubenVerborgh which repo should get an issue to get the deprecation warning and "Set permission" button?

[RubenVerborgh]

Depends on where we want to show it. I think NSS, should probably login redirect page.

[kjetilk]

Right, we can do that... I was also thinking about some of the other UIs, e.g. databrowser?

[RubenVerborgh]

A general mechanism for server notifications would be great for sure. I'm thinking #1057.

Databrowser, was also thinking that. But maybe too specific to couple server to this.

The Solid way would be to send something to people's inboxes 🙂

[mikeadams1]

"The Solid way would be to send something to people's inboxes 🙂"

That sounds like a great resolution, and no one can say that they did not receive notice.

[kjetilk]

OK! I think we could use the login page, as everyone is on that every now and then.

[mikeadams1]

On second thought, the login page would probably be a great place for notices.

[Quantum-Bot]

I am very happy to read here that everyone will have a choice if their index.html is visible or not via the button I suggested to control that setting in gitter.
Thank you to everyone for giving this issue attention and all the hard work everyone contributes to this project.

[megoth]

I've implemented a hack so that the index.html-algorithm works, but I have some questions reg the button-part.

There are three scenarios that we might want to consider:

1. The user wants to grant READ access to root: We need to grant public READ access to '/' in /.acl; We can present this option with the button that has been discussed
2. The user wants to make homepage private explicitly: We need to remove public READ access to index.html on `index.html.acl; Maybe we should give a button for this as well?
3. The user doesn't care, just wants to remove the message: We need to store that dismissal somewhere; Maybe have a button that dismisses the message, so that the user doesn't have to be bothered. We might store this in users' preferences, but there's really no property to describe that, so would have to create something (right?).

We could also "force" the user to make a choice by only allowing option 1 and 2. Also, I reckon this section is at the bottom of the homepage, so wouldn't be up in the users' face. (Or do we want to have if up front, to "force" them to make a choice?)

This section could show only when there's a discrepancy between READ access to '/' and '/index.html', which makes it so that once an option is chosen, the section won't show anymore (unless the user does something with the READ access on either files later, of course).

[megoth]

I see that people have suggested that the button should live in the login-page? I'm confused as to how that would work? Or am I misunderstanding something?

[megoth]

To give people an idea of what I'm thinking wrt two buttons (make public, make private):

  <section>
    <h1>Make this page explicitly public or private</h1>
    <p>We have detected that this page have a discrepancy when handling access. The details might be a bit confusing, but you can <a href="#">read more about it in the forum</a>. To keep it simple, we ask you to consider whether you want to make this page explicitly available for the public.</p>
    <form>
      <div class="form-group">
        <p>By making the page public you can view this page more as a normal webpage, that anyone can access. They still won't get access to resources that requires login, like the private folder.</p>
        <button class="btn btn-primary">Make page public</button>
      </div>
    </form>
    <form>
      <div class="form-group">
        <p>By making the page private you make sure that only you can see this page, and it requires you to login if you don't have a valid session.</p>
        <button class="btn btn-primary">Make page private</button>
      </div>
    </form>
  </section>

This would only be shown to users that have control of this POD.

The idea is to also link to a thread in the forum that explains the situation in more details.

[megoth]

"Or, could we have an explicit sunset on this hack? I.e., we do this for 5.0.0, but warn our users that they need to set read permission on their root, because this will be removed in the future?"

Good idea. Practical problem would be lack of a place to show such a notification.

Even better perhaps is combining this with something I read earlier: making this a button "Set permission" so it is zero effort.

I am wondering where we should have this button. If I put it in /index.html we need to run the update POD script that was implemented in https://github.com/solid/node-solid-server/pull/953, but as https://github.com/solid/node-solid-server/issues/1030 discuss, this might be problematic.

[kjetilk]

I think they should decide, as not deciding will cause problems when the hack is retired.

I think it is a good idea to have two buttons though, so that they can choose.

When you say it, I'm not sure what the others meant by "login"-page, because when you see that, you're not yet logged in, I suppose.

[kjetilk]

I have merged the backend aspects of this an released a new beta, and made a new ticket for the frontend buttons in #1076 , so we can close this now.