adobe / aem-project-archetype

Maven template to create best-practice websites on AEM.
https://experienceleague.adobe.com/docs/experience-manager-core-components/using/developing/archetype/overview.html
Apache License 2.0
546 stars 421 forks source link

Disable passing on authorization header by default #1081

Open kwin opened 1 year ago

kwin commented 1 year ago

In https://github.com/adobe/aem-project-archetype/blob/183706c16758fc02a8becc16d2dc8a5b55e7ef46/src/main/archetype/dispatcher.cloud/src/conf.dispatcher.d/clientheaders/default_clientheaders.any#L43 the Authorization header is forwarded to AEM. That can be abused with certain endpoints to do brute-force credential attacks on the Basic Auth Handler (compare with https://experienceleaguecommunities.adobe.com/t5/adobe-experience-manager/disable-basic-authentication/m-p/359084/highlight/true#M48638). Therefore I would argue that by default those headers should not be forwarded to the backend. The default AEM authorization relies on cookies only (and not the authorization header which is only used for OAuth and Basic Auth). As that is an immutable file and customers can only add additional headers on top, this base configuration makes it impossible to strip Authorization headers (except with workarounds like using mod_headers)

kwin commented 1 year ago

Also compare with the check in https://github.com/0ang3el/aem-hacker/blob/3ce91f217b259b0b4e6abd07f56d453b0c82b46b/aem_hacker.py#L619.

krystiannowak commented 1 year ago

makes it impossible to strip Authorization headers (except with workarounds like using mod_headers)

@kwin default_clientheaders.any does NOT need to be included from clientheaders.any: https://github.com/adobe/aem-project-archetype/blob/develop/src/main/archetype/dispatcher.cloud/src/conf.dispatcher.d/clientheaders/clientheaders.any#L7

As per https://experienceleague.adobe.com/docs/experience-manager-cloud-service/content/implementing/content-delivery/validation-debug.html?lang=en#flexible-mode-file-structure

Customizable Files (...) conf.dispatcher.d/clientheaders/clientheaders.any This file is included from inside your .farm files. It specifies what request headers should be forwarded to the backend. vs Immutable Configuration Files (...) conf.dispatcher.d/clientheaders/default_clientheaders.any Default request headers to forward to the backend, suitable for a standard project. If you need customization, modify clientheaders.any. In your customization, you can still include the default request headers first, if they suit your needs.

AFAIK Authorization header has been added 2 years back to support authentication for Sync Doc APIs - see https://experienceleague.adobe.com/docs/experience-manager-cloud-service/content/implementing/developing/generating-access-tokens-for-server-side-apis.html?lang=en#the-server-to-server-flow

/cc: @jalagari

kwin commented 1 year ago

@krystiannowak Thanks for the pointers. Still I would consider that an insecure default. Maybe you can somehow tweak the dispatcher to only allow Bearer authentication scheme (https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#authentication_schemes) or at least block the Basic one by default. That would probably require an adjustment of the dispatcher module....

krystiannowak commented 1 year ago

@kwin /clientheaders is a simple list of strings (representing header names) as per https://experienceleague.adobe.com/docs/experience-manager-dispatcher/using/configuring/dispatcher-configuration.html?lang=en#specifying-the-http-headers-to-pass-through-clientheaders - so there is no filtering by value or matching any kind of regular expression in it