[jira] [Commented] (SLING-11115) Allow path exemptions for referrer filter
[ 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
[ 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
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
[ 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
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
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
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
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
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
[ 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
[ 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
[ 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
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
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
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
[ 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
[ 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
[ 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
[ 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
[ 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 "/"
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
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
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
[ 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
[ 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)
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)
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
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
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
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
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
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
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
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
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
[ 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)
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)
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