search

solid / specification

Solid Technical Reports
https://solidproject.org/TR/
MIT License
489 stars 45 forks source link

Make any deviation of SPARQL UPDATE explicit #139

Closed RubenVerborgh closed 1 year ago

kjetilk commented 4 years ago

Yes, indeed, this is relevant in the context of #85 and #125 .

acoburn commented 3 years ago

We have implemented only standard SPARQL 1.1

michielbdejong commented 3 years ago

Solid OS uses this willful violation. NSS and PSS support it. Neither CSS nor ESS do. @acoburn is right, we could achieve a very similar effect with If-Match. It wouldn't be too much work to implement that in rdflib if the consensus is that it's better to align with upstream sparql-update on this.

@timbl what should we do?

timbl commented 3 years ago

@RubenVerborgh - I agree. "I think that, if we make such deviations, they should be made explicit".

What do you suggest is the best way? We could flag it in the content type.

Maybe the ideal is to use a recognizably different syntax in the file, likeREMOVE (which fails) instead of DELETE (which never fails) and so extend sparql in a way the WG could potentially agree to be generally adopted.

That would mean for us the cost of changing our syntax,

We could then flag it like with a ; remove=true or something on the content type.So we would be extending SPARQL UPDATE and not changing it. There must be a lot of discussion of this already elsewhere.

michielbdejong commented 3 years ago

OK, so to summarize, the proposed change would be:

Right?

michielbdejong commented 3 years ago

Great!

michielbdejong commented 3 years ago

Looks like after almost two years we all agree now, awesome!!

michielbdejong commented 3 years ago

OK, sorry I wasn't aware. What are the steps?

kjetilk commented 3 years ago

Obviously, I've lost a year of context, but I have some opposition to the semaphore mechanism due to security concerns. It would require not only write permission, but also read permission, which would violate the principle of least privilege. I tried to sum these problems in https://github.com/solid/query-panel/issues/2 and proposed a solution to the SPARQL 1.2 CG: https://lists.w3.org/Archives/Public/public-sparql-12/2020Jan/0000.html

I certainly see the value of a light-weight semaphore mechanism (even though there is some opposition to that idea in the SPARQL 1.2 CG), but I believe that as it stands, it is too dangerous. We need to carefully consider the possible impact. Given time, I would certainly want to work on this.

justinwb commented 3 years ago

OK, sorry I wasn't aware. What are the steps?

The proposal would need to be documented in the form of a change submission to the solid protocol specification, and go through editorial review. This issue and any related ones would/should be referenced as part of that proposal.

michielbdejong commented 3 years ago

FWIW, NSS now accepts both application/sparql-update and application/sparql-update-single-match as the content-type. In both cases, the behaviour is "Solid-flavoured" sparql-update (as it has been for years in the case of application/sparql-update).

Solid OS (rdflib) still sends application/sparql-update and expects the pod server to behave the same way as NSS. If and when other servers support NSS-compatible behaviour with application/sparql-update-single-match, we'll update Solid OS (rdflib) to start sending the new content-type header. HTH.

acoburn commented 3 years ago

Before implementing support for application/sparql-update-single-match (or any new content type), please first show me a formal specification (even in draft form) that formalizes the semantics. A github discussion is a fine way to find initial consensus, but there is no way to build a reasonable implementation based on a discussion thread such as this.

kjetilk commented 3 years ago

Here be dragons! And not the small and cute ones you see on dragon-taming shows, the big and fire-breathing ones. :-)

Let me reiterate: By doing it this way, we have to be really sure that the spec details security considerations, and that the use of a content type is a special case that requires read access. We also need to have tests that verify compliance. A semaphore is exposing information, it MUST be a read operation, even though the operations to the resource are only write operations.

I'm very skeptical about having such special cases in general, it can be misunderstood and bugs are likely. Thus, the big, dangerous dragons.

Instead, it should be understood in terms of a projection (in SPARQL terms, and more generally in database terms). With that, it is consistently understood in implementations that all projections are exposing information and require read privs. For something as security critical as this, that, I think, is the level of rigor we should have.

michielbdejong commented 3 years ago

OK, too bad, I thought we had finally found a solution but sounds like this will take a few more rounds. I removed the -single-match from the content type again in the test suite, but the behaviour that is tested for is the Solid-flavoured one.

kjetilk commented 3 years ago

This was one of the things that made me start the query panel back in the day... I believe that format is the right one to discuss issues like these (even though this might be the single issue we have for the short term)

michielbdejong commented 3 years ago

I'll also mark the test for this as disputed again, since it seems the dispute will still go on. For now, we need to tell clients to avoid sending DELETE/INSERT since the different server implementations will behave differently if the DELETE fails. And then the same goes for https://github.com/solid/specification/issues/220

michielbdejong commented 3 years ago

OK sorry maybe the word 'dispute' is too strong. Maybe we should call it an 'undecided issue'? Let's use that word from now on.

What I try to cover with it is the situation where different implementations behave in different ways. And that's the current reality of Solid. And it's not OK.

We are in the early stages of making a decision

I sincerely hope not! I brought this undecided issue to the attention of the spec editors in June 2019. So if this is early stages, then I dare not estimate how many more years Solid will stay in limbo on this issue.

I tried one last time to lead a decision by asking Tim to take a stance, which he did in https://github.com/solid/specification/issues/139#issuecomment-780517959 and we got pretty close last week. I proposed a resolution, @RubenVerborgh adapted it, @acoburn agreed. That's as close as we've ever been.

After that, @RubenVerborgh retracted his position saying it was a 5-second proposal. Fair enough, so then spend more than 5 seconds and tell us why your proposal is not good enough, so we can improve it. Is the specific string -single-match wrong? Given this issue is about "making our deviation explicit", doesn't your proposal solve that nicely?

@kjetilk welcome back! we missed you! <3 I see your points about whether or not we should use a semaphore at all. That's a good question, and it looks like it's unanswered. Tim seems to says yes, Ruben and Aaron seem to say ok, but only if we signal it properly, and you seem to say no. I don't care either way, I just want it to either be yes (signaled in whichever way we decide on) for all servers or no for all servers.

Emphasis on for all servers! :)

kjetilk commented 3 years ago

Hi @michielbdejong ! Thanks for the welcome back!

Actually, I'm very much in favour of having a semaphore mechanism, just not this one :-)

I just think we need to be very careful not to introduce a mechanism that will either limit our ability to design a good least privilege system, or that there will be confusion around that can introduce obscure security issues. Also, I don't want to break SPARQL 1.1.

I think it is useful to think in short term and long term. In the long term, it is very feasible to introduce generic functionality into SPARQL 1.2, which is being worked on, albeit with low intensity, that can support the semaphore mechanism. I'd love to prototype that, and champion it towards the SPARQL 1.2 effort.

In the short term, there are a few options:

  1. Don't have the semaphore mechanism, instead require backends to isolate different HTTP requests from each other (as I understand ESS is doing)
  2. Don't have a semaphore mechanism and say nothing about it, just wait for a long term solution.
  3. Have a short term solution based on e.g. a distinguishing content-type

I suppose all these could be orthogonal to a long term solution. I certainly see the value of having the semaphore, as I see the ACID requirement that could easily be the alternative as a pretty hefty one. But I am also very uneasy about the content-type proposed here in a broad security context, so I just don't see a good short-term solution to this problem.

BTW, if we go with a content-type, I think we should but it in the vnd tree.

kjetilk commented 3 years ago

BTW, I just rediscovered the Github issue I made with the SPARQL 1.2 CG on this topic: https://github.com/w3c/sparql-12/issues/60 It is useful to review for context and for what might be considered appropriate for SPARQL.