Hi Angela, Thank you for the clarification. To summarize, I think consensus has been reached.
1. Move forward with the patch in OAK-6575 discussed in this thread. 2. Develop a thread model so that this and future changes can be evaluated as part of the release process, ideally before the next stable release. If anyone feels consensus has not been reached, please continue this thread. Best Regards Ian On 15 September 2017 at 07:52, Angela Schreiber <anch...@adobe.com.invalid> wrote: > Hi Ian > > On 13/09/17 23:34, "Ian Boston" <i...@tfd.co.uk> wrote: > > >Hi Angela, > > > >On 13 September 2017 at 06:50, Angela Schreiber > ><anch...@adobe.com.invalid> > >wrote: > > > >> Hi Ian > >> > >> The new proposal looks a lot better to me. > >> > >> The only concern from a security perspective I could come up with is the > >> one we expressed already with the very first proposal (see initial word > >>of > >> caution mail sent by Francesco): applications built on top of Oak can up > >> to now be sure that all access to the repository content is subject to > >>the > >> same permission evaluation as configured with the repository setup. This > >> is no longer guaranteed when we offer the ability to plug application > >>code > >> that may or may not by-pass the evaluation by allowing it to directly > >> access binaries. > >> > > > >I don't think this patch bypasses Oak security, and since the API can only > >be implemented by Oak itself. I am sure any future patch would be subject > >to the same scrutiny. If it can be implemented outside Oak, then Oak has > >already been breached, something I can see no evidence of. > > > >In this case, the signed url is only issued after Oak security has granted > >access to the binary, and only returned over the JCR API to the JCR > >Session > >that made the call, in the same way that an InputStream allows the bytes > >of > >the binary to be read by that session. The URL only allows read access. > > > >What the session does with that data, is outside the control of Oak. > >Unlike the byte[] from the that has no protection, the signed URL is > >protected. It may only be used unmodified for the purpose it was intended > >by Oak and only for a well defined period of time. In that respect, > >arguably, its is more secure than the byte[] or InputStream. > > I am not worried about the security of the approach and maybe I am even > mistaken if I think this changes our threat model. But even if it actually > changes our threat model wouldn't mean that your proposal isn't secure. > Those are two quite distinct things. > > That's why i would love us to finally reserve some time to create a threat > model. It will help us with this discussion and with similar requests in > the future. and even the threat model itself can evolve over time... but > it allows us to review features from a different angle. > > > > >> > >> While I know that this is actually the goal of the whole exercise, we > >>have > >> to be aware that this also is a change in our Oak security model. As > >>such > >> this may look like a security breach and I have been told by my > >>colleagues > >> at Adobe that the 'single-way-to-access' is a relevant security question > >> with a lot of potential customers. > >> > >> That doesn't mean that I am opposed to the patch in it's current form > >>as I > >> see the benefits from an Oak pov, I just want to highlight that we are > >> going to make a fundamental change and we should treat and document it > >> with the necessary care... maybe we should take this opportunity to > >> finally create a threat model for Oak? Doing so at this stage would > >>allow > >> us to visualise the proposed change to all parties involved. > >> > >> wdyt? > >> > > > >Agreed. > >Having a fully developed threat model which clarified all the risks for > >every aspect of Oak would, imho, be much better than not defining the > >risks > >that exist. Even the most secure application has risks, best exposed in a > >threat model, however brief. > > > >Unfortunately Oak now exists in a world which is distributed, where > >applications need to embrace the network. This is a fundamental change, > >which Oak has to embrace. An Oak Threat model that recognises this will be > >a great step forwards. > > > >On the other hand, if you are saying that the Oak Threat model has to be > >developed and agreed, before this patch can be added, then I am concerned > >that will take too long. Doing justice to an Oak Treat model will require > >resource. > > No... that's not what I am saying :-) > What I would strongly advocate though is that we start working on the > threat model in parallel and make sure we have it completed before we > create any stable release (1.8.x, 1.6.x, etc) that includes your patch. > This will allow the whole Oak team to take a final look at it from a > design of view. Since this has been subject to a lot of disputes in the > past and actually got rejected then, I think having a threat model will be > very valuable. > > Kind regards > Angela > > > > >Best `Regards > >Ian > > > > > >> > >> Kind regards > >> Angela > >> > >> > >> On 07/09/17 16:39, "Ian Boston" <i...@tfd.co.uk> wrote: > >> > >> >On 7 September 2017 at 14:41, Francesco Mari <mari.france...@gmail.com > > > >> >wrote: > >> > > >> >> On Thu, Sep 7, 2017 at 11:05 AM, Ian Boston <i...@tfd.co.uk> wrote: > >> >> > On 7 September 2017 at 07:22, Ian Boston <i...@tfd.co.uk> wrote: > >> >> > > >> >> >> Hi, > >> >> >> > >> >> >> On 6 September 2017 at 22:43, Michael Dürig <mdue...@apache.org> > >> >>wrote: > >> >> >> > >> >> >>> > >> >> >>> > >> >> >>> On 06.09.17 23:08, Michael Dürig wrote: > >> >> >>> > >> >> >>>> > >> >> >>>> Hi, > >> >> >>>> > >> >> >>>> On 05.09.17 14:09, Ian Boston wrote: > >> >> >>>> > >> >> >>>>> Repeating the comment to on OAK-6575 here for further > >>discussion. > >> >>2 > >> >> new > >> >> >>>>> Patches exploring both options. > >> >> >>>>> > >> >> >>>> > >> >> >>>> I would actually prefer the original patch ( > >> >> >>>> https://github.com/ieb/jackrabbit-oak/compare/trunk...ieb:O > >> >> >>>> AK-6575?expand=1) in most parts. However I have concerns > >>regarding > >> >>the > >> >> >>>> generality of the new OakConversionService API as mentioned in > >>my > >> >> previous > >> >> >>>> mail. I would be more comfortable if this could be restricted to > >> >> something > >> >> >>>> that resembles more like a "URIProvider", which given a blob > >> >>returns > >> >> an URI. > >> >> >>>> > >> >> >>>> On the implementation side, why do we need to introduce the > >> >>adaptable > >> >> >>>> machinery? Couldn't we re-use the Whiteboard and OSGiWhiteBoard > >> >> mechanisms > >> >> >>>> instead? I think these could be used to track URIProvider > >>instances > >> >> >>>> registered by the various blob stores. > >> >> >>>> > >> >> >>>> > >> >> >>> See https://github.com/mduerig/jackrabbit-oak/commit/2709c097b01 > >> >> >>> a006784b7011135efcbbe3ce1ba88 for a *really* quickly hacked > >>together > >> >> and > >> >> >>> entirely untested POC. But it should get the idea across though. > >> >> >> > >> >> >> > >> >> >> > >> >> >> Thank you. > >> >> >> That makes sense. > >> >> >> I think it only needs the java/org/apache/jackrabbit/ > >> >> >> oak/blob/cloud/aws/s3/CloudFrontS3SignedUrlAdapterFactory.java > and > >> >>the > >> >> >> API to be inside Oak, everything else can be in Sling. > >> >> >> I'll update my patch and do a 2 options for Sling. > >> >> >> > >> >> > > >> >> > > >> >> > > >> >> > > >> >> > https://github.com/ieb/jackrabbit-oak/compare/trunk.. > >> >> .ieb:OAK-6575-3?expand=1 > >> >> > > >> >> > and > >> >> > > >> >> > > >> > >>>>https://github.com/apache/sling/compare/trunk...ieb:OAK- > 6575-3?expand=1 > >> >> > > >> >> > wdyt ? > >> >> > >> >> I like this a lot. It keeps Oak's side simple and cleanly integrates > >> >> Oak's lower-level services in Sling. > >> >> > >> > > >> > > >> >Good news. > >> >I think we should hold off committing the patch until Monday or > >>Tuesday to > >> >give those who may be offline this week a chance to comment. In > >>particular > >> >I have not seen a comment from Angela who I would expect to have a > >>view as > >> >this is acl/security related. That is assuming she is back online next > >> >week. > >> > > >> >Best Regards > >> >Ian > >> > > >> > > >> >> > >> >> > Obviously the second patch needs to be discussed with Sling dev, > >>but > >> >>is > >> >> > should not be too contentious. > >> >> > > >> >> > Best Regards > >> >> > Ian > >> >> > > >> >> > > >> >> > > >> >> >> > >> >> >> I think that should address others concerns since it drops all > >>signs > >> >>of > >> >> >> any generic object to object conversion from Oak (Francesco), and > >> >> doesn't > >> >> >> require wide scale fragile changes with implied requirements being > >> >> placed > >> >> >> on how intermediate classes are connected and behave (mine). > >> >> >> > >> >> >> Best Regards > >> >> >> Ian > >> >> >> > >> >> >> > >> >> >>> > >> >> >>> Michael > >> >> >>> > >> >> >> > >> >> >> > >> >> > >> > >> > >
