[jira] [Commented] (SLING-11115) Allow path exemptions for referrer filter

2022-02-08 Thread Lars Krapf (Jira)


[ 
https://issues.apache.org/jira/browse/SLING-5?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17488710#comment-17488710
 ] 

Lars Krapf commented on SLING-5:


[~angela]:
bq. what do you have in mind when you refer to 'complete paths'?

I suggest to match the [resource 
path|https://sling.apache.org/apidocs/sling7/org/apache/sling/api/request/RequestPathInfo.html#getResourcePath--]
 portion of the path info. 


> Allow path exemptions for referrer filter 
> --
>
> Key: SLING-5
> URL: https://issues.apache.org/jira/browse/SLING-5
> Project: Sling
>  Issue Type: Improvement
>  Components: Sling Security
>    Reporter: Lars Krapf
>Assignee: Angela Schreiber
>Priority: Major
> Fix For: Security 1.1.24
>
>
> The referrer filter should have a configuration option to exclude one or 
> several paths from the check. 
> For context:
> It seems that the RedHat SSO IDP sends "Referrer-Policy: no-referrer" by 
> default (to adress some [security 
> concerns|https://tools.ietf.org/id/draft-ietf-oauth-security-topics-14.html#rfc.section.4.2.4]).
>  This breaks the SAML POST binding in conjunction with the Sling referrer 
> filter. Currently the only option to make it work is to allow empty referrers 
> in general, however this weakens the CSRF protection. 
> Allowing to disable the filter for individual paths would allow to solve this 
> use-case with minimal additional risk. 



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (SLING-11115) Allow path exemptions for referrer filter

2022-02-03 Thread Lars Krapf (Jira)


[ 
https://issues.apache.org/jira/browse/SLING-5?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17486456#comment-17486456
 ] 

Lars Krapf commented on SLING-5:


[~cziegeler]: 
bq. How should such an exclude list look like? Complete paths or path prefixes?

My gut feeling is to only allow exact matches. It's faster and should prevent 
people from accidently opening up entire subtrees. 
I wouldn't expect the number of exempted paths to grow too big eventually. In 
AEM, the token based CSRF protection only allows exact matches either, and so 
far no one complained :)

> Allow path exemptions for referrer filter 
> --
>
> Key: SLING-5
> URL: https://issues.apache.org/jira/browse/SLING-5
> Project: Sling
>  Issue Type: Improvement
>  Components: Sling Security
>    Reporter: Lars Krapf
>Priority: Major
>
> The referrer filter should have a configuration option to exclude one or 
> several paths from the check. 
> For context:
> It seems that the RedHat SSO IDP sends "Referrer-Policy: no-referrer" by 
> default (to adress some [security 
> concerns|https://tools.ietf.org/id/draft-ietf-oauth-security-topics-14.html#rfc.section.4.2.4]).
>  This breaks the SAML POST binding in conjunction with the Sling referrer 
> filter. Currently the only option to make it work is to allow empty referrers 
> in general, however this weakens the CSRF protection. 
> Allowing to disable the filter for individual paths would allow to solve this 
> use-case with minimal additional risk. 



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Created] (SLING-11115) Allow path exemptions for referrer filter

2022-02-03 Thread Lars Krapf (Jira)
Lars Krapf created SLING-5:
--

 Summary: Allow path exemptions for referrer filter 
 Key: SLING-5
 URL: https://issues.apache.org/jira/browse/SLING-5
 Project: Sling
  Issue Type: Improvement
  Components: Sling Security
Reporter: Lars Krapf


The referrer filter should have a configuration option to exclude one or 
several paths from the check. 

For context:
It seems that the RedHat SSO IDP sends "Referrer-Policy: no-referrer" by 
default (to adress some [security 
concerns|https://tools.ietf.org/id/draft-ietf-oauth-security-topics-14.html#rfc.section.4.2.4]).
 This breaks the SAML POST binding in conjunction with the Sling referrer 
filter. Currently the only option to make it work is to allow empty referrers 
in general, however this weakens the CSRF protection. 
Allowing to disable the filter for individual paths would allow to solve this 
use-case with minimal additional risk. 




--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (SLING-10225) Files with ".." In Name Throw 400 Exception

2021-03-18 Thread Lars Krapf (Jira)


[ 
https://issues.apache.org/jira/browse/SLING-10225?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17304160#comment-17304160
 ] 

Lars Krapf commented on SLING-10225:


Hello [~rombert]

I agree that the fix for SLING-9741, i.e {{path.contains("...")}} is incomplete 
(at least for JCR), since {{.}} is a valid node name character. Some ambiguity 
most likely cannot be avoided. 
Actually, I would *not* invalidate the path in the first place, but simply try 
to be a bit more consistent with the normalization. I'm not sure if anything 
other than {{/../}} or {{/..;foo/}} should resolve to the parent. From a strict 
security POV however there is no right or wrong IMO, it's just different 
interpretations.  

[~dklco]:
bq. Or would this just make more sense to handle in the reverse proxy rather 
than building this into the Sling Engine?

I think it's a combination of both - the decomposition from SLING-9741 is 
rather confusing (note, that additionally this is dependent on whether {{a.js}} 
exists or not, so it's impossible for a static proxy to determine the correct 
resolution). But the problem of mismatching interpretations of ".." is [not 
unique to 
Sling|https://www.acunetix.com/blog/articles/a-fresh-look-on-reverse-proxy-related-attacks/]
 - I think the best we can do is to be consistent and well documented.

> Files with ".." In Name Throw 400 Exception
> ---
>
> Key: SLING-10225
> URL: https://issues.apache.org/jira/browse/SLING-10225
> Project: Sling
>  Issue Type: Bug
>  Components: Engine
>Affects Versions: Engine 2.7.4
>Reporter: Dan Klco
>Priority: Critical
> Fix For: Engine 2.7.6
>
>
> SLING-9741 and the [associated 
> PR|https://github.com/apache/sling-org-apache-sling-engine/pull/11] 
> introduced a regression where the Sling Engine will return a 400 error on 
> requests based on the presence of ".." in the URL when not preceded by a 
> slash.
> This is an issue as file names may contain multiple periods and it is not 
> obvious that it would cause an issue to upload a file with two periods in the 
> name. 
> h2. Reproduction steps:
> * Update a Sling instance to use Engine 2.7.4
> * Upload a file containing .. in the path
> * Attempt to get the file or any path with the file as a suffix
> * Note this returns a 400 error



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (SLING-9767) Insecure Recommendation in Dynamic Include Documentation

2020-09-25 Thread Lars Krapf (Jira)
Lars Krapf created SLING-9767:
-

 Summary: Insecure Recommendation in Dynamic Include Documentation
 Key: SLING-9767
 URL: https://issues.apache.org/jira/browse/SLING-9767
 Project: Sling
  Issue Type: Improvement
  Components: Documentation
Affects Versions: Dynamic Include 3.2.0
Reporter: Lars Krapf


The [documentation for the Sling Dynamic 
Includes|https://sling.apache.org/documentation/bundles/dynamic-includes.html#enabling-ssi-in-apache-with-the-aem-dispatcher-module]
 mentions the following:

bq. Having added the SetOutputFilter directive, open the virtual host's 
configuration and add the Includes option to the Options directive:

This is an extremely unsafe recommendation. The "Includes" option will allow 
anyone who can change content on the backend (e.g. AEM) to run arbitrary 
commands on the webserver (dispatcher), by injecting the {{}} 
directive.
The recommendation should be to use the "IncludesNOEXEC" option instead, which 
will only allow to include static content (with a "safe" mime-type such as HTML 
or plain-text). 

See also: http://httpd.apache.org/docs/current/mod/mod_include.html



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (SLING-9741) Invalid path decomposition in case of multiple dots

2020-09-17 Thread Lars Krapf (Jira)
Lars Krapf created SLING-9741:
-

 Summary: Invalid path decomposition in case of multiple dots
 Key: SLING-9741
 URL: https://issues.apache.org/jira/browse/SLING-9741
 Project: Sling
  Issue Type: Bug
  Components: ResourceResolver
Affects Versions: Resource Resolver 1.7.0
Reporter: Lars Krapf


The resource resolver performs path normalization using 
[ResourceUtil.normalize()|https://github.com/apache/sling-org-apache-sling-api/blob/a459f157b87e2ca6a274a1d890aad1d86ff7a631/src/main/java/org/apache/sling/api/resource/ResourceUtil.java#L49].
 

This leads to unexpected results in the case of a combination of non-existing 
resources, and multiple dots in a path segment. 

E.g. the following request:
{{http://localhost/content/a.js/..children-1json/a.txt}}

will be decomposed as follows:
{code}
Extension=json
resourcePath=/content/a.js/..
selectors=[, , , children, , , , -1]
seclectorString=...children-1...
suffix=/a.txt
{code}

Note that the first two dots of the third path segment are interpreted as the 
parent path (a.js does not exist), which essentially turns this line into 
{{/content.children.-1.json/a.txt}}, which can confuse reverse proxies. 

I think the {{..}} should only be interpreted as the parent path if followed by 
a {{/}} (or potentially a semicolon if path parameters on {{..}} segments 
should be allowed).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (SLING-9740) Invalid handling of requests containing URL path parameters

2020-09-17 Thread Lars Krapf (Jira)
Lars Krapf created SLING-9740:
-

 Summary: Invalid handling of requests containing URL path 
parameters
 Key: SLING-9740
 URL: https://issues.apache.org/jira/browse/SLING-9740
 Project: Sling
  Issue Type: Bug
  Components: Engine
Affects Versions: Engine 2.7.2
Reporter: Lars Krapf


{{RequestData.initResource()}} has support for requests containing URL-path 
parameters (e.g. /path;foo=bar/path2;bar=baz/). It will split at the first 
semicolon, and concatenate this to the {{request.getPathInfo()}} (not 
containing such parameters). See 
[RequestData.java|https://github.com/apache/sling-org-apache-sling-engine/blob/master/src/main/java/org/apache/sling/engine/impl/request/RequestData.java#L232].
 However, this handling is incomplete as it only covers the case where one such 
parameter is added at the end of the request, but path parameters can be added 
to *any* path segment, leading to unexpected results.

E.g. the following request:
http://localhost:4502/content;foo=bar/we-retail;bar=baz/us/en.html

will result in {{path}} being:
/content/we-retail/us/en.html;foo=bar/we-retail;bar=baz/us/en.html

This gets especially confusing when path normalization happens in conjunction 
with path parameters:
http://localhost/content/we-retail.html/..;/..;/bin/querybuilder.json.css?path=/home/users

will result in {{path}} being:
/bin/querybuilder.json.css;/..;/bin/querybuilder.json.css

after the concatenation. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (SLING-9739) Wrong decomposition/resolution in Servlets Resolver Plugin

2020-09-17 Thread Lars Krapf (Jira)
Lars Krapf created SLING-9739:
-

 Summary: Wrong decomposition/resolution in Servlets Resolver Plugin
 Key: SLING-9739
 URL: https://issues.apache.org/jira/browse/SLING-9739
 Project: Sling
  Issue Type: Bug
  Components: Console
Affects Versions: Servlets Resolver 2.7.10
Reporter: Lars Krapf
 Attachments: 2020-09-17-104013_1554x416_scrot.png

The servlets resolver webconsole plugin is sometimes displaying a different 
decomposition and candidate servlet, than what Sling actually resolves. 

E.g. the path:
/content/we-retail.html/..;/..;/bin/querybuilder.json.css?path=/home/users

is resolved to Page.jsp (the resource-type of /content/we-retail.html), see:
 !2020-09-17-104013_1554x416_scrot.png! 

whereas the true decomposition/resolution will be:
{code}
Extension=css;
resourcePath=/bin/querybuilder.json
selectors=[]
seclectorString=null
suffix=/..;/bin/querybuilder.json.css
 {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (SLING-9441) Sling POST servlet responds with 500 if target is not modifiable

2020-05-12 Thread Lars Krapf (Jira)
Lars Krapf created SLING-9441:
-

 Summary: Sling POST servlet responds with 500 if target is not 
modifiable
 Key: SLING-9441
 URL: https://issues.apache.org/jira/browse/SLING-9441
 Project: Sling
  Issue Type: Bug
  Components: Servlets
Affects Versions: Servlets Post 2.3.36
Reporter: Lars Krapf


In case a POST is attempted to a node that the request session does not have 
write access to, the POST servlets throws a 500, e.g.

{code}
04.05.2020 21:34:09.162 *ERROR* [xxx.xxx.xxx.xxx [1588628049158] POST 
/content/cmo/us/en/home/users/geometrixx/james.dev...@spambob.com/fSPiyhliJm 
HTTP/1.1] org.apache.sling.servlets.post.impl.operations.ModifyOperation 
Exception during response processing.

org.apache.sling.api.resource.PersistenceException: Unable to create node at 
/content/cmo/us/en/home/users

 at 
org.apache.sling.jcr.resource.internal.helper.jcr.JcrResourceProvider.create(JcrResourceProvider.java:477)
 [org.apache.sling.jcr.resource:3.0.20]
{code}

Since this is actually a client error I think a 4xx response would be better 
suited, and make it more obvious that such errors can be safely ignored when 
doing health monitoring. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (SLING-9043) COPY should be in the referer filter's default list of protected HTTP methods

2020-02-07 Thread Lars Krapf (Jira)


[ 
https://issues.apache.org/jira/browse/SLING-9043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17032288#comment-17032288
 ] 

Lars Krapf commented on SLING-9043:
---

[~reschke], [~kwin]: 
[~sonagupt] has updated the PR and added MOVE. Any more concerns from your 
side? Merci!

> COPY should be in the referer filter's default list of protected HTTP methods
> -
>
> Key: SLING-9043
> URL: https://issues.apache.org/jira/browse/SLING-9043
> Project: Sling
>  Issue Type: Bug
>  Components: Resource Access Security
>Reporter: Sonal Gupta
>Priority: Major
>  Labels: vulnerability
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> The COPY method , by default, is not in the list of methods covered by the 
> CSRF Referer filter. This might allow an attacker to copy files (abusing the 
> privileges of a logged in victim) using CSRF.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (SLING-9043) COPY should be in the referer filter's default list of protected HTTP methods

2020-02-06 Thread Lars Krapf (Jira)


[ 
https://issues.apache.org/jira/browse/SLING-9043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17031455#comment-17031455
 ] 

Lars Krapf commented on SLING-9043:
---

[~kwin]: Yes, with proper CORS configuration this issue is mitigated - however, 
both headers might be stripped in some situations (see also: 
https://owasp.org/www-project-cheat-sheets/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html)
 and IMHO it wouldn't hurt to add the methods to the referrer filter given that 
it's already there, and given the many lax CORS configurations out there. 


> COPY should be in the referer filter's default list of protected HTTP methods
> -
>
> Key: SLING-9043
> URL: https://issues.apache.org/jira/browse/SLING-9043
> Project: Sling
>  Issue Type: Bug
>  Components: Resource Access Security
>Reporter: Sonal Gupta
>Priority: Major
>  Labels: vulnerability
>
> The COPY method , by default, is not in the list of methods covered by the 
> CSRF Referer filter. This might allow an attacker to copy files (abusing the 
> privileges of a logged in victim) using CSRF.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (SLING-9043) COPY should be in the referer filter's default list of protected HTTP methods

2020-02-06 Thread Lars Krapf (Jira)


[ 
https://issues.apache.org/jira/browse/SLING-9043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17031428#comment-17031428
 ] 

Lars Krapf commented on SLING-9043:
---

Hello [~reschke]

COPY (and yes, MOVE as well) are state-changing operations and should ideally 
be protected against CSRF as a defense-in-depth measure in case of an overly 
permissive CORS (or even same-site) configuration.
The attack consists of luring a victim to an attacker's site, and then do an 
COPY XHR to the target, abusing the fact that the victim is authenticated and 
their browser will be automatically sending a session token. 
Since the referrer will be the attackers site this attack would be mitigated by 
the referrer filter (analogous to PUT and DELETE already present in the list).

> COPY should be in the referer filter's default list of protected HTTP methods
> -
>
> Key: SLING-9043
> URL: https://issues.apache.org/jira/browse/SLING-9043
> Project: Sling
>  Issue Type: Bug
>  Components: Resource Access Security
>Reporter: Sonal Gupta
>Priority: Major
>  Labels: vulnerability
>
> The COPY method , by default, is not in the list of methods covered by the 
> CSRF Referer filter. This might allow an attacker to copy files (abusing the 
> privileges of a logged in victim) using CSRF.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (SLING-7777) XSSFilter is rejecting URLs containing only queries or fragments

2018-07-12 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-:
-

 Summary: XSSFilter is rejecting URLs containing only queries or 
fragments
 Key: SLING-
 URL: https://issues.apache.org/jira/browse/SLING-
 Project: Sling
  Issue Type: Bug
  Components: XSS Protection API
Affects Versions: XSS Protection API 2.0.10
Reporter: Lars Krapf
 Attachments: sling_xssfilter_patch.txt

The XSSFilter is erroneously rejecting URLs that consist only of queries, 
(potentially empty) fragments or both, e.g. "#", "#test", "?foo=bar" etc. 

Even though the RELATIVE_PART regexp contains an PATH_EMPTY group, it is 
explicitly matching the *entire* string, so will fail if the QUERY or FRAGMENT 
groups match.

A potential solution (see attached patch and tests) might be to remove the 
PATH_EMPTY group from the RELATIVE_PART, and make the entire RELATIVE_PART 
optional by adding ? to the group in RELATIVE_REF. This will still match 
completely empty URLs. 

 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


Mappings and Authentication

2017-09-21 Thread Lars Krapf
Hello list

IIUC the Sling Authenticator chooses an authentication handler based on
the request path, and *not* on the mapped path.

So (please correct me if I'm wrong), it seems not possible to have two
different internalRedirects from domain-names to sub-paths, which are
covered by two different authentication handlers.

E.g.

+ /etc/map/http/bla.4502
  - sling:internalRedirect = /content/bla
+ /etc/map/http/fasel.4502
  - sling:internalRedirect = /content/fasel

with two different authentication handlers, one registered for
/content/bla and one for /content/fasel is *not* possible, correct?

Now, two questions

a) what is the reasoning behind having the authenticator select handlers
*before* the mapping
b) is it possible to make this work somehow?

Also, to me, this slightly smells of a privilege escalation.
Say I have write access to /etc/map, I will be able to change
authentication handlers for an arbitrary sub-pat, potentially disabling
authentication altogether (by mapping a path without authentication
requirements to the target path). Of course, in most cases this will not
achieve anything, because you still won't have access to the resources,
but it does seem a little "shady" at least. No?


Thanks for your thoughts
Lars


[jira] [Created] (SLING-6438) Add encodeForHTMLAttrName() to XSSAPI

2017-01-05 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-6438:
-

 Summary: Add encodeForHTMLAttrName() to XSSAPI
 Key: SLING-6438
 URL: https://issues.apache.org/jira/browse/SLING-6438
 Project: Sling
  Issue Type: Improvement
  Components: XSS Protection API
Affects Versions: XSS Protection API 1.0.16
Reporter: Lars Krapf


The XSSAPI currently does not provide a method to safely encode HTML attribute 
names. This could cause problems if for example attackers were allowed to 
define arbitrary "data-" attributes. I don't think Encoder already provides a 
context for that, but {{forHtmlUnquotedAttribute}} might just do the trick.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Comment Edited] (SLING-4560) XSSAPI#getValidHref is empty for valid Bengali or Hindi characters

2016-08-18 Thread Lars Krapf (JIRA)

[ 
https://issues.apache.org/jira/browse/SLING-4560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15427306#comment-15427306
 ] 

Lars Krapf edited comment on SLING-4560 at 8/18/16 10:47 PM:
-

Hello [~radu.cotescu]

With this change {{onSiteURL}} will accept spaces and colons and thus does no 
longer filter external  (and/or " javascript:") URLs.

This could be caught by the following additional 
tests:{code:title=XSSAPIImplTest.testfilterHtml()} {"space","space"},
 {"http://www.google.com\;>", ""},
{code}

Please note however, that the test mentioned above does not contain bengali / 
hindi characters. FWIW, I tried to come up with a hindi test using google 
translate:
{code:title=XSSAPIImplTest.testGetValidHref()}
   {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
  
{"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995",
 
"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
{code}

Nonetheless, the summary is correct, this test too fails with the old regexps. 
The reason for this is that the unicode "letter" character class 
{code}\p{L}{code} is matching single unicode *code points* only. To match any 
letter including diacritics, one might use {code}\p{L}\p{M}*+{code}. See also 
[1] ("Unicode Categories").

I've added a corresponding patch to the regexp (changing only the character 
class) and added a couple of tests. 

Note, The test provided by [~jck] *would still fail* even with this change, but 
AFAICT that's correct, since these characters are symbols and not letters. 


was (Author: chaotic):
Hello [~radu.cotescu]

With this change {{onSiteURL}} will accept spaces and colons and thus does no 
longer filter external  (and/or " javascript:") URLs.

This could be caught by the following additional 
tests:{code:title=XSSAPIImplTest.testfilterHtml()} {"space","space"},
 {"http://www.google.com\;>", ""},
{code}

Please note however, that the test mentioned above does not contain bengali / 
hindi characters. FWIW, I tried to come up with a hindi test using google 
translate:
{code:title=XSSAPIImplTest.testGetValidHref()}
   {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
  
{"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995",
 
"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
{code}

Nonetheless, the summary is correct, this test too fails with the old regexps. 
The reason for this is that the unicode "letter" character class \\p\{L\} is 
matching single unicode *code points* only. To match any letter including 
diacritics, one might use \\p\{L\}\\p\{M\}*+. See also [1] ("Unicode 
Categories").

I've added a corresponding patch to the regexp (changing only the character 
class) and added a couple of tests. 

Note, The test provided by [~jck] *would still fail* even with this change, but 
AFAICT that's correct, since these characters are symbols and not letters. 

> XSSAPI#getValidHref is empty for valid Bengali or Hindi characters
> --
>
> Key: SLING-4560
> URL: https://issues.apache.org/jira/browse/SLING-4560
> Project: Sling
>  Issue Type: Bug
>  Components: XSS Protection API
>Affects Versions: XSS Protection API 1.0.0
>Reporter: Jean-Christophe Kautzmann
>Assignee: Radu Cotescu
> Fix For: XSS Protection API 1.0.14
>
> Attachments: xssapi.patch
>
>
> I added (locally) 2 test cases to 
> org.apache.sling.xss.impl.XSSAPIImplTest#testGetValidHref:
> {code}
> {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
> {"/etc/commerce/collections/⺁〡〢☉⊕〒", "/etc/commerce/collections/⺁〡〢☉⊕〒"},
> {code}
> the first test passes (chinese characters), the 2nd fails (bengali/hindi 
> characters) although it should pass as they are valid characters.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Comment Edited] (SLING-4560) XSSAPI#getValidHref is empty for valid Bengali or Hindi characters

2016-08-18 Thread Lars Krapf (JIRA)

[ 
https://issues.apache.org/jira/browse/SLING-4560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15427306#comment-15427306
 ] 

Lars Krapf edited comment on SLING-4560 at 8/18/16 10:47 PM:
-

Hello [~radu.cotescu]

With this change {{onSiteURL}} will accept spaces and colons and thus does no 
longer filter external  (and/or " javascript:") URLs.

This could be caught by the following additional 
tests:{code:title=XSSAPIImplTest.testfilterHtml()} {"space","space"},
 {"http://www.google.com\;>", ""},
{code}

Please note however, that the test mentioned above does not contain bengali / 
hindi characters. FWIW, I tried to come up with a hindi test using google 
translate:
{code:title=XSSAPIImplTest.testGetValidHref()}
   {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
  
{"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995",
 
"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
{code}

Nonetheless, the summary is correct, this test too fails with the old regexps. 
The reason for this is that the unicode "letter" character class \\p\{L\} is 
matching single unicode *code points* only. To match any letter including 
diacritics, one might use \\p\{L\}\\p\{M\}*+. See also [1] ("Unicode 
Categories").

I've added a corresponding patch to the regexp (changing only the character 
class) and added a couple of tests. 

Note, The test provided by [~jck] *would still fail* even with this change, but 
AFAICT that's correct, since these characters are symbols and not letters. 


was (Author: chaotic):
Hello [~radu.cotescu]

With this change {{onSiteURL}} will accept spaces and colons and thus does no 
longer filter external  (and/or " javascript:") URLs.

This could be caught by the following additional 
tests:{code:title=XSSAPIImplTest.testfilterHtml()} {"space","space"},
 {"http://www.google.com\;>", ""},
{code}

Please note however, that the test mentioned above does not contain bengali / 
hindi characters. FWIW, I tried to come up with a hindi test using google 
translate:
{code:title=XSSAPIImplTest.testGetValidHref()}
   {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
  
{"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995",
 
"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
{code}

Nonetheless, the summary is correct, this test too fails with the old regexps. 
The reason for this is that the unicode "letter" character class \p{L} is 
matching single unicode *code points* only. To match any letter including 
diacritics, one might use \P{L}\p{M}*+. See also [1] ("Unicode Categories").

I've added a corresponding patch to the regexp (changing only the character 
class) and added a couple of tests. 

Note, The test provided by [~jck] *would still fail* even with this change, but 
AFAICT that's correct, since these characters are symbols and not letters. 

> XSSAPI#getValidHref is empty for valid Bengali or Hindi characters
> --
>
> Key: SLING-4560
> URL: https://issues.apache.org/jira/browse/SLING-4560
> Project: Sling
>  Issue Type: Bug
>  Components: XSS Protection API
>Affects Versions: XSS Protection API 1.0.0
>Reporter: Jean-Christophe Kautzmann
>Assignee: Radu Cotescu
> Fix For: XSS Protection API 1.0.14
>
> Attachments: xssapi.patch
>
>
> I added (locally) 2 test cases to 
> org.apache.sling.xss.impl.XSSAPIImplTest#testGetValidHref:
> {code}
> {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
> {"/etc/commerce/collections/⺁〡〢☉⊕〒", "/etc/commerce/collections/⺁〡〢☉⊕〒"},
> {code}
> the first test passes (chinese characters), the 2nd fails (bengali/hindi 
> characters) although it should pass as they are valid characters.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (SLING-4560) XSSAPI#getValidHref is empty for valid Bengali or Hindi characters

2016-08-18 Thread Lars Krapf (JIRA)

 [ 
https://issues.apache.org/jira/browse/SLING-4560?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Lars Krapf updated SLING-4560:
--
Attachment: xssapi.patch

Adding potential patch.

> XSSAPI#getValidHref is empty for valid Bengali or Hindi characters
> --
>
> Key: SLING-4560
> URL: https://issues.apache.org/jira/browse/SLING-4560
> Project: Sling
>  Issue Type: Bug
>  Components: XSS Protection API
>Affects Versions: XSS Protection API 1.0.0
>Reporter: Jean-Christophe Kautzmann
>Assignee: Radu Cotescu
> Fix For: XSS Protection API 1.0.14
>
> Attachments: xssapi.patch
>
>
> I added (locally) 2 test cases to 
> org.apache.sling.xss.impl.XSSAPIImplTest#testGetValidHref:
> {code}
> {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
> {"/etc/commerce/collections/⺁〡〢☉⊕〒", "/etc/commerce/collections/⺁〡〢☉⊕〒"},
> {code}
> the first test passes (chinese characters), the 2nd fails (bengali/hindi 
> characters) although it should pass as they are valid characters.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Comment Edited] (SLING-4560) XSSAPI#getValidHref is empty for valid Bengali or Hindi characters

2016-08-18 Thread Lars Krapf (JIRA)

[ 
https://issues.apache.org/jira/browse/SLING-4560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15427306#comment-15427306
 ] 

Lars Krapf edited comment on SLING-4560 at 8/18/16 10:44 PM:
-

Hello [~radu.cotescu]

With this change {{onSiteURL}} will accept spaces and colons and thus does no 
longer filter external  (and/or " javascript:") URLs.

This could be caught by the following additional 
tests:{code:title=XSSAPIImplTest.testfilterHtml()} {"space","space"},
 {"http://www.google.com\;>", ""},
{code}

Please note however, that the test mentioned above does not contain bengali / 
hindi characters. FWIW, I tried to come up with a hindi test using google 
translate:
{code:title=XSSAPIImplTest.testGetValidHref()}
   {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
  
{"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995",
 
"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
{code}

Nonetheless, the summary is correct, this test too fails with the old regexps. 
The reason for this is that the unicode "letter" character class \p{L} is 
matching single unicode *code points* only. To match any letter including 
diacritics, one might use \P{L}\p{M}*+. See also [1] ("Unicode Categories").

I've added a corresponding patch to the regexp (changing only the character 
class) and added a couple of tests. 

Note, The test provided by [~jck] *would still fail* even with this change, but 
AFAICT that's correct, since these characters are symbols and not letters. 


was (Author: chaotic):
Hello [~radu.cotescu]

With this change {{onSiteURL}} will accept spaces and colons and thus does no 
longer filter external  (and/or " javascript:") URLs.

This could be caught by the following additional 
tests:{code:title=XSSAPIImplTest.testfilterHtml()} {"space","space"},
 {"http://www.google.com\;>", ""},
{code}

Please note however, that the added test does not contain bengali / hindi 
characters. FWIW, I tried to come up with a hindi test using google translate:
{code:title=XSSAPIImplTest.testGetValidHref()}
   {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
  
{"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995",
 
"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
{code}

Nonetheless, the summary is correct, this test too fails with the old regexps. 
The reason for this is that the unicode "letter" character class \p{L} is 
matching single unicode *code points* only. To match any letter including 
diacritics, one might use \P{L}\p{M}*+. See also [1] ("Unicode Categories").

I've added a corresponding patch to the regexp (changing only the character 
class) and added a couple of tests. 

Note, The test provided by [~jck] *would still fail* even with this change, but 
AFAICT that's correct, since these characters are symbols and not letters. 

> XSSAPI#getValidHref is empty for valid Bengali or Hindi characters
> --
>
> Key: SLING-4560
> URL: https://issues.apache.org/jira/browse/SLING-4560
> Project: Sling
>  Issue Type: Bug
>  Components: XSS Protection API
>Affects Versions: XSS Protection API 1.0.0
>Reporter: Jean-Christophe Kautzmann
>Assignee: Radu Cotescu
> Fix For: XSS Protection API 1.0.14
>
> Attachments: xssapi.patch
>
>
> I added (locally) 2 test cases to 
> org.apache.sling.xss.impl.XSSAPIImplTest#testGetValidHref:
> {code}
> {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
> {"/etc/commerce/collections/⺁〡〢☉⊕〒", "/etc/commerce/collections/⺁〡〢☉⊕〒"},
> {code}
> the first test passes (chinese characters), the 2nd fails (bengali/hindi 
> characters) although it should pass as they are valid characters.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (SLING-4560) XSSAPI#getValidHref is empty for valid Bengali or Hindi characters

2016-08-18 Thread Lars Krapf (JIRA)

[ 
https://issues.apache.org/jira/browse/SLING-4560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15427306#comment-15427306
 ] 

Lars Krapf commented on SLING-4560:
---

Hello [~radu.cotescu]

With this change {{onSiteURL}} will accept spaces and colons and thus does no 
longer filter external  (and/or " javascript:") URLs.

This could be caught by the following additional 
tests:{code:title=XSSAPIImplTest.testfilterHtml()} {"space","space"},
 {"http://www.google.com\;>", ""},
{code}

Please note however, that the added test does not contain bengali / hindi 
characters. FWIW, I tried to come up with a hindi test using google translate:
{code:title=XSSAPIImplTest.testGetValidHref()}
   {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
  
{"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995",
 
"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"},
{code}

Nonetheless, the summary is correct, this test too fails with the old regexps. 
The reason for this is that the unicode "letter" character class \p{L} is 
matching single unicode *code points* only. To match any letter including 
diacritics, one might use \P{L}\p{M}*+. See also [1] ("Unicode Categories").

I've added a corresponding patch to the regexp (changing only the character 
class) and added a couple of tests. 

Note, The test provided by [~jck] *would still fail* even with this change, but 
AFAICT that's correct, since these characters are symbols and not letters. 

> XSSAPI#getValidHref is empty for valid Bengali or Hindi characters
> --
>
> Key: SLING-4560
> URL: https://issues.apache.org/jira/browse/SLING-4560
> Project: Sling
>  Issue Type: Bug
>  Components: XSS Protection API
>Affects Versions: XSS Protection API 1.0.0
>Reporter: Jean-Christophe Kautzmann
>Assignee: Radu Cotescu
> Fix For: XSS Protection API 1.0.14
>
>
> I added (locally) 2 test cases to 
> org.apache.sling.xss.impl.XSSAPIImplTest#testGetValidHref:
> {code}
> {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
> {"/etc/commerce/collections/⺁〡〢☉⊕〒", "/etc/commerce/collections/⺁〡〢☉⊕〒"},
> {code}
> the first test passes (chinese characters), the 2nd fails (bengali/hindi 
> characters) although it should pass as they are valid characters.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Created] (SLING-5675) Logout only called if AuthenticationHandler is registered to "/"

2016-04-20 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-5675:
-

 Summary: Logout only called if AuthenticationHandler is registered 
to "/"
 Key: SLING-5675
 URL: https://issues.apache.org/jira/browse/SLING-5675
 Project: Sling
  Issue Type: Bug
  Components: Authentication
Affects Versions: Auth Core 1.3.14
Reporter: Lars Krapf


In {{SlingAuthenticator.logout()}} only the AuthenticationHandlers which are 
registered on paths which are roots of 
{{SlingAuthenticator.getHandlerSelectionPath()}} are selected.

This path should either be taken from the servlet path, or will be read from 
the {{Authenticator.LOGIN_RESOURCE}} request attribute _if it is present_.

Now, in {{LogoutServlet.service()}} the LOGIN_RESOURCE is _always_ set to it's 
default value ("/") by calling {{AuthUtil.setLoginResourceAttribute()}}. 

As a result, {{dropCredentials()}} will only be called on authentication 
handlers which are registered to "/". 

My expectation is that the selection of logout handlers should be independent 
of their registration paths, in order to allow a POST to 
{{/system/sling/logout}} have *all* registered handlers drop credentials. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


Re: security risk of allow empty referrer in Apache Sling Referrer Filter

2015-05-28 Thread Lars Krapf
Hello Daniel

On 28.05.2015 10:11, Daniel Sungjin Jung wrote:
 Checking “Allow Empty” checkbox in Apache Sling Referrer Filter is not 
 recommended in production service.
 I’d like to know what specific security risks we face if we turn it on for 
 production service.

Apart from the obvious cases (bugs in browser/plugins, MitM) which allow
for HTTP header manipulation but often allow complete circumvention of
CSRF protections anyway, there have been several cases where it was
possible to strip the referrer header client-side using some tricks with
javascript and iframes (e.g. [0], [1]).

Best greetings
Lars


[0]
http://homakov.blogspot.com/2012/04/playing-with-referer-origin-disquscom.html
[1]
http://webstersprodigy.net/2013/02/01/stripping-the-referer-in-a-cross-domain-post-request/




[jira] [Created] (SLING-4701) SlingAuthenticator.isAnonAllowed matches for all paths starting with the same characters

2015-05-07 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-4701:
-

 Summary: SlingAuthenticator.isAnonAllowed matches for all paths 
starting with the same characters
 Key: SLING-4701
 URL: https://issues.apache.org/jira/browse/SLING-4701
 Project: Sling
  Issue Type: Bug
  Components: Authentication
Affects Versions: Auth Core 1.3.6
Reporter: Lars Krapf


The SlingAuthenticator check if anonymous access is allowed compares paths with 
String.startsWith. If the holder.path does not end with a '/' it will 
erroneously match a different path that starts with the same characters, even 
if it is not a descendant of the first path. 

Example:
- Allow anonymous acces on '/'
- Deny anonymous access on a path '/blubb'  
- Authentication is enforced on a request to '/blubb-blah' - which is wrong.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (SLING-4701) SlingAuthenticator.isAnonAllowed matches for all paths starting with the same characters

2015-05-07 Thread Lars Krapf (JIRA)

 [ 
https://issues.apache.org/jira/browse/SLING-4701?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Lars Krapf updated SLING-4701:
--
Attachment: SlingAuthenticator.patch

Attached possible patch.

 SlingAuthenticator.isAnonAllowed matches for all paths starting with the same 
 characters
 

 Key: SLING-4701
 URL: https://issues.apache.org/jira/browse/SLING-4701
 Project: Sling
  Issue Type: Bug
  Components: Authentication
Affects Versions: Auth Core 1.3.6
Reporter: Lars Krapf
  Labels: authentication
 Attachments: SlingAuthenticator.patch


 The SlingAuthenticator check if anonymous access is allowed compares paths 
 with String.startsWith. If the holder.path does not end with a '/' it will 
 erroneously match a different path that starts with the same characters, even 
 if it is not a descendant of the first path. 
 Example:
 - Allow anonymous acces on '/'
 - Deny anonymous access on a path '/blubb'  
 - Authentication is enforced on a request to '/blubb-blah' - which is wrong.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (SLING-4413) :applyTo should send 403 instead of 500 when operation fails

2015-02-12 Thread Lars Krapf (JIRA)

 [ 
https://issues.apache.org/jira/browse/SLING-4413?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Lars Krapf updated SLING-4413:
--
Summary: :applyTo should send 403 instead of 500 when operation fails   
(was: :applyTo should send 403 instead of 500 when operation fails)

 :applyTo should send 403 instead of 500 when operation fails 
 -

 Key: SLING-4413
 URL: https://issues.apache.org/jira/browse/SLING-4413
 Project: Sling
  Issue Type: Bug
  Components: Servlets
Affects Versions: Servlets Post 2.3.6
Reporter: Lars Krapf
Priority: Minor

 Example:
 curl -vv curl -F:operation=delete -F:applyTo=/etc/* 
 http://localhost:4502/content/geometrixx
 Will give you a 500 (PersistenceException) in case /etc/* is not writable to 
 the request session - as discussed with Felix  Carsten this should rather be 
 a 403.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Created] (SLING-4414) :applyTo should only apply to requested resource (and below)

2015-02-12 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-4414:
-

 Summary: :applyTo should only apply to requested resource (and 
below)
 Key: SLING-4414
 URL: https://issues.apache.org/jira/browse/SLING-4414
 Project: Sling
  Issue Type: Improvement
  Components: Servlets
Affects Versions: Servlets Post 2.3.6
Reporter: Lars Krapf
Priority: Minor


:applyTo operations currently accept arbitrary paths that are independent from 
the requested resource. In the spirit of REST they should only accept relative 
paths below the requested node.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Created] (SLING-4415) :applyTo should not display changeLog (when operation fails)

2015-02-12 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-4415:
-

 Summary: :applyTo should not display changeLog (when operation 
fails)
 Key: SLING-4415
 URL: https://issues.apache.org/jira/browse/SLING-4415
 Project: Sling
  Issue Type: Bug
  Components: Servlets
Affects Versions: Servlets Post 2.3.6
Reporter: Lars Krapf


When the :applyTo operation fails the change-log leaks information about the 
internal path-structure even when the requesting session does not have access 
to these paths. 

One solution would be to completely omit the ChangeLog (at least when the 
operation fails), another option would be to make the :sendError behaviour 
configurable in the POST servlet, so that the error message can be reliably 
overlaid.




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Created] (SLING-4413) :applyTo should send 403 instead of 500 when operation fails

2015-02-12 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-4413:
-

 Summary: :applyTo should send 403 instead of 500 when operation 
fails
 Key: SLING-4413
 URL: https://issues.apache.org/jira/browse/SLING-4413
 Project: Sling
  Issue Type: Bug
  Components: Servlets
Affects Versions: Servlets Post 2.3.6
Reporter: Lars Krapf
Priority: Minor


Example:
curl -vv curl -F:operation=delete -F:applyTo=/etc/* 
http://localhost:4502/content/geometrixx

Will give you a 500 (PersistenceException) in case /etc/* is not writable to 
the request session - as discussed with Felix  Carsten this should rather be a 
403.




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


Re: Events, Jobs and admin sessions

2014-05-16 Thread Lars Krapf
Hello Marius

It depends on the use-case. In examples like yours a service-user would
most probably be the right choice. In other examples, for instance a job
that processes an asset, the job should be performed with the privileges
of the triggering user, to limit the possibilities of an potential exploit.

Best greetings
Lars

On 14.05.2014 19:39, Marius Petria wrote:
 Hi,
 
 When processing events and jobs, the corresponding subject triggering
 the event usually gets lost. This lead to event handlers / job
 consumers often operating with administrative sessions/resolvers to
 do their work, which in turn can lead to privilege escalations.
 
 Is that a good pattern to encourage, i.e. to execute the handlers using the 
 security context of the triggering subject? You could pass the information on 
 a case by case basis, but typically consumers might represent different 
 services than the one triggering the action. Imagine an indexing service that 
 listens to all modifications. If the user for that service does not have read 
 permissions for certain resources then those should not be indexed when an 
 admin is editing the content.
 
 Marius
 
  
 



Re: Events, Jobs and admin sessions

2014-05-14 Thread Lars Krapf
Hello Carsten

Thanks for your reply.

No, I don't see an obvious solution either.
It's just that while reviewing the loginAdmin() usages, I discovered
that a lot of the cases are based on this problem, and I was hoping for
a solution that is as generic as possible.

For the jobs, I could imagine an extension of the JobManager API that
allows passing the subject. The resource resolver factory could then
take the event/job as a parameter and return a resolver with the
privileges of the corresponding subject.

For the events, the situation seems to be even more complicated because
usualy the event is not created manually, and I'm not sure if it is
possible to assign a specific subject to an event in many cases.

The alternative is to use a service-user in the consumer who has access
to the respective payload, which somehow looks wrong to me from a
security perspective.

Well.. Ideas very welcome :)

Best greetings
Lars


On 13.05.2014 22:57, Carsten Ziegeler wrote:
 Hi Lars,
 
 I see your point, I don't see right now how a general approach could look
 like. However, the creator of a job could add the subject as a property to
 the job and the consumer can use this value to create a resource resolver
 based on that value. But I think this has to be done on a job by job base.
 
 Or do you see a general mechanism which always gets the subject of the
 sender?
 
 Carsten
 
 
 2014-05-13 17:21 GMT+02:00 Lars Krapf lkr...@adobe.com:
 
 Hello list

 When processing events and jobs, the corresponding subject triggering
 the event usually gets lost. This lead to event handlers / job consumers
 often operating with administrative sessions/resolvers to do their work,
 which in turn can lead to privilege escalations.

 A possible solution to this problem could be to add a serialization of
 the event-triggering subject (if available) as a property to the event
 by default, so the handlers could easily recreate the session by using
 JAAS doAsPrivileged().

 Would that make sense?

 Best greetings
 Lars

 
 
 



Events, Jobs and admin sessions

2014-05-13 Thread Lars Krapf
Hello list

When processing events and jobs, the corresponding subject triggering
the event usually gets lost. This lead to event handlers / job consumers
often operating with administrative sessions/resolvers to do their work,
which in turn can lead to privilege escalations.

A possible solution to this problem could be to add a serialization of
the event-triggering subject (if available) as a property to the event
by default, so the handlers could easily recreate the session by using
JAAS doAsPrivileged().

Would that make sense?

Best greetings
Lars


Re: Trusted credentials and loginByService

2014-01-17 Thread Lars Krapf

On 16.01.2014 23:28, Alexander Klimetschek wrote:

On 16.01.2014, at 05:19, Carsten Ziegeler cziege...@apache.org wrote:


Eagerly waiting for a patch which implements this :)

He he :)

This isn’t meant as something we should have soon - it is meant as a goal to 
guide around the jcr login mechanism discussion.

One opinion is: ah, don’t care, once code is running in the JVM, consider 
everything exploited, so we can put JCR authentication and convenience 
mechanisms everywhere.

My opinion is: no, write that authentication code with a clear boundary in mind 
(JCR), sling/application level code can’t just login as anyone on JCR unless 
the internal repository authenticates it. So that later my above vision is 
simpler to reach. As a side effect, code and security configuration becomes 
clearer (having to configure authentication both in Sling and Jackrabbit is 
just confusing).


For most cases this is true since the Sling authentication layer is 
basically responsible for credential extraction not authentication.


For some (SSO) usecases however validation has to happen on the sling 
layer - so there always has to be some means of trust delegation between 
these two layers.
Trusted credentials was the answer before, and using JCR 
preauthentication (null-login) is hopefully soon.




Otherwise I don’t even know why we removed the TrustedInfo in the first place.


Because it is inherently insecure to have a preconfigured plain-text 
admin password laying around, that needs to be changed in two places 
just to establish trust.


Cheers
Lars



Re: Trusted credentials and loginByService

2014-01-17 Thread Lars Krapf

Hello Ian

On 17.01.2014 12:19, Ian Boston wrote:

[...]


What was the problem with TrustedCredentials ? I might be thinking of 
the wrong thing and you might be talking about something different.


When I talk about trusted credentials, I refer only to the previous 
implementation that relied
on an attribute (configured once for the login-module(s) and once for 
the authentication handler).
which was stored in plain-text, and had a default value that you still 
can find in many production-instances out there.
Once that config leaks, it could easily lead to administrative 
compromise of the repo in many cases,

so this clearly was something that needed improvement.

The method we will use now makes use of the pre-authenticated handling 
of the LoginContextProvider (previously in JR RepositoryImpl),
which indeed relies solely on the JVM for security. What you describe 
below is much more sophisticated,
and would definitely improve the security of the 
authhandler-loginmodule trust-binding a great deal.


However I do not fully share Alex's optimism that the VM can be 
sufficiently secured to restrict access
to reflection, runtime or FS without a major rewrite of the whole Sling 
layer - so whoever can deploy
code to the instance will find a way to escalate privileges in one way 
or another -
This is reflected in the current threat-model and thus 
pre-authentication does not violate the specified trust
boundaries. Further this means that loginService() is primarily a 
measure against malicious users, not developers.


I see your method as a great security in depth measure, and a step in 
the right direction,
but without Java security, there are limits to what we can or should do 
(IMHO).


Best greetings
Lars



I am thinking of a class that implements Credentials that is trusted
by whatever logs into the repository. That class is available via
factory service that looks at who is calling it and does some basic
checking (eg get the byte code of the caller and sha1 it, or anything
else that verifies trust between the caller and the service, but
doesn't rely on the JVM for that verification). That class seals its
contents against tampering on construction and does not require a
password.

Anything that needs to get trusted credentials is configured to be
allowed access, in a place that the JVM cant tamper with. (ie OS file
containing sha1s of the class defs not writable by the owner of the
JVM process)

I am assuming (from memory) that Credentials can be added to
AuthenticationInfo and used by whatever repository (JCR2/Oak/etc)
chooses to trust them during the login operation.

I am not thinking about JAAS... because that hurts my brain, ;)


Ian


(null-login) is hopefully soon.



Otherwise I don’t even know why we removed the TrustedInfo in the first
place.


Because it is inherently insecure to have a preconfigured plain-text admin
password laying around, that needs to be changed in two places just to
establish trust.

Cheers
Lars


.





Re: Trusted credentials and loginByService

2014-01-16 Thread Lars Krapf

Hello Alex

As long as reflection is still permitted I think you could get around 
most (all?) of these restrictions.
I'm pretty sure this can only be solved properly on VM level 
(SecurityManager).


Cheers
Lars


On 15.01.2014 23:53, Alexander Klimetschek wrote:

On 15.01.2014, at 01:34, Carsten Ziegeler cziege...@apache.org wrote:


I think, that's already solved by the SecurityManager concept - so let's
not reinvent the wheel

One solution would be to add a new interface to use that only gives access to 
the allowed methods. For example, say you only want to allow 
System.currentTimeMillis(), you'd add a new interface MySystem which has this 
method. And deny access to java.lang.System but allow MySystem in the package 
importing.

Of course that won't work with existing code that makes use of 
java.lang.System. But it would be a clean way...

The difference of the security manager is that it will allow access to the 
class/method, but then do a security check if that caller is allowed (looking 
at the passed context) and throw an exception if not.

This naturally makes it a lot more difficult to implement, since the code 
itself has to set permissions, do the checks and handle exceptions.

Not giving access in the first place is a lot cleaner and simpler.

Cheers,
Alex.





[jira] [Created] (SLING-2966) Insufficient synchronization in SlingAuthenticator

2013-07-16 Thread Lars Krapf (JIRA)
Lars Krapf created SLING-2966:
-

 Summary: Insufficient synchronization in SlingAuthenticator
 Key: SLING-2966
 URL: https://issues.apache.org/jira/browse/SLING-2966
 Project: Sling
  Issue Type: Bug
  Components: Authentication
Affects Versions: Auth Core 1.1.2
Reporter: Lars Krapf


In the serviceChanged method of SlingAuthenticator a service is removed and 
subsequently re-added in case of a modification event. This can lead to 
synchronization problems (e.g. remove, remove, add, add) if two threads trigger 
this handler at the same time (e.g. duplication of CUG roots in the 
holder-cache of the CQ CUG handler after activation of a page with alias).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira


[jira] [Updated] (SLING-2966) Insufficient synchronization in SlingAuthenticator

2013-07-16 Thread Lars Krapf (JIRA)

 [ 
https://issues.apache.org/jira/browse/SLING-2966?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Lars Krapf updated SLING-2966:
--

Attachment: sling_authenticator.patch

Attached a possible patch: synchronizing the whole serviceChanged() method. 

 Insufficient synchronization in SlingAuthenticator
 --

 Key: SLING-2966
 URL: https://issues.apache.org/jira/browse/SLING-2966
 Project: Sling
  Issue Type: Bug
  Components: Authentication
Affects Versions: Auth Core 1.1.2
Reporter: Lars Krapf
  Labels: authentication, sling
 Attachments: sling_authenticator.patch


 In the serviceChanged method of SlingAuthenticator a service is removed and 
 subsequently re-added in case of a modification event. This can lead to 
 synchronization problems (e.g. remove, remove, add, add) if two threads 
 trigger this handler at the same time (e.g. duplication of CUG roots in the 
 holder-cache of the CQ CUG handler after activation of a page with alias).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira


Re: ResourceAccessGate (SLING-2698)

2013-03-06 Thread Lars Krapf
Hello

Please accept my 2.3 cents to this discussion:

I really agree with Angela and Bertrand on this one.
Something that could be called AccessGate is clearly a security
mechanism, and should thus be designed with single point of access in
mind.
Having ACL evaluations scattered among the code-base makes it
harder to configure (and thus easier to make mistakes), and increases
the overall attack surface. To me Sling is not the right layer for
such a feature, IMHO it should be implemented on repository level by
extending Jackrabbit's ACL evaluation if time-based access control
is really needed.
This is also an example of how this feature would weaken
security. In order to allow access to a resource within a certain
time-frame, you will have to open access completely on repository
level, so the whole access control would depend on the Sling layer,
and we introduce a bunch of new attack possibilities, and
unnecessarily complicate the threat model.

I think a ResourceResolverDecorator might be useful in some cases.
But having access control features in sling core just feels wrong to me.

Best greetings
Lars


On 03/06/2013 11:23 AM, Bertrand Delacretaz wrote:
 On Wed, Mar 6, 2013 at 11:03 AM, Carsten Ziegeler cziege...@apache.org 
 wrote:
 ...this would mean, we trust people implementing
 ResourceResolverDecorator, but we don't trust them implementing
 ResourceAccessGates
 
 I see your point...I guess it's very subjective factors here, I prefer
 ResourceResolverDecorator to ResourceAccessGate although both can
 potentially be abused (like many of our extension points).
 
 Let's see what others think.
 
 -Bertrand
 



Re: ResourceAccessGate (SLING-2698)

2013-03-06 Thread Lars Krapf
Hello Carsten

On 03/06/2013 12:45 PM, Carsten Ziegeler wrote:
 2013/3/6 Lars Krapf lkr...@adobe.com:
 if time-based access control
 is really needed.
 Time based access restriction is one of the main use cases as Mike has
 explained repeatedly.

Yes - I understand that. The important part of my quote is
the one before that. If needed (which I don't want to argue), then why
not implement it with the rest of the access control?
Why put it on a different layer, which until now has nothing
to do with access restrictions?

 
 This is also an example of how this feature would weaken
 security. In order to allow access to a resource within a certain
 time-frame, you will have to open access completely on repository
 level, so the whole access control would depend on the Sling layer,
 No, this is wrong - as I mentioned in my first post here and as has
 been explained over and over again since Mike came up with the
 proposal, this is an additional filter. The intention is not to
 replace ACLs.

As Angela mentionend in the bug, there seem to be two possible ways of
implementing time-based access on Sling layer.
Either remove ACLs on the repository, or do it with an admin session.
Both will shift access control enforcement from the repository to Sling.
Of course technically this is just an additional filter (which arguably
might already be dangerous), but in practice you will have to
replace/ignore repository access control to make it work.
This violates the single-point-of-access principle which has proven
to be very valuable for CQ security in the past.
I'm not against this idea in general, I just think this should all be
done at one central place. In Oak you can provide your custom
access control implementation. To me, this seems to be the natural place
to implement additional requirements. What are the arguments against
that?

 
 I'm really wondering why we are having this discussion over and over
 again - we agreed some months ago to implement this feature in Sling.
 Now Mike has started work and immediately everyone and his dog is
 going back to the old discussion. :(

I agree this is annoying, and I'm sorry if I missed the discussion the
first time. But it really seems there are multiple concerns from
different perspectives to this proposal which obviously were not
resolved the first time.
Sometimes it (or I) just needs a little concrete code to understand
what one is debating about and find a good solution.

Cheers
Lars


 
 Carsten