Process phase 1 in the same Apache hook as phase 2

[rcbarnett-zz]

MODSEC-98: I have this idea that ModSecurity should not use post_read for its phase 1. Instead, phase 1 should use the same hook as phase 2. With this change, users would be able to override configuration from a

or container, removing the problem that has been causing confusion for years. The only advantage of having phase 1 early is to allow for rules that are protecting Apache itself, but I am yet to see a single such rule. Besides, we can still keep one such early phase (although we'd better move to using names for phases, instead of numbers).

[rcbarnett-zz]

Original reporter: ivanr

[rcbarnett-zz]

ivanr: Please review and close.

[rcbarnett-zz]

marcstern: What's the impact of this change on interactions with other modules? For instance, phase:1 was performed before SetEnvIf, RewriteRule, RequestHeader. Is it still the case?

[rcbarnett-zz]

marcstern: > allow for rules that are protecting Apache itself,

but I am yet to see a single such rule What about rules protecting the "scoping", like ".." in URL, invalid UTF-8 encoding, invalid request line, inconsistent content-length and transfer-encoding, ... Doesn't this introduce a risk? What if Apache has a hole in such an area? A rule could have blocked the request.

[rcbarnett-zz]

ivanr: I am going to comment on this because I made this change (long time ago, while I was still actively coding). Because rules are not known and cannot be evaluated in phase 1, we have had many problem reports from our users. I have tripped on the issue several times. Thus the motivation for this change is usability. People often bring up the need to protect Apache itself, but, as I commented in the main body of this issue, no one has ever written such a rule as far as I know. Protecting Apache means protecting against buffer overflows. All the other problems you bring up, I think you can protect against from a later phase. Finally, another phase can be added for the protection of Apache itself.

[rcbarnett-zz]

marcstern: Ivan,

1. You did not answer the question about compatibility (phase:1 was performed before SetEnvIf, RewriteRule, RequestHeader, etc.) phase:1 is not only used to protect Apache, but also to prepare environment variables to other modules
2. Rules on URI (directory traversal, etc.) are protecting Apache itself, no?
3. If I understand correctly, phase:1 is now totally redundant with phase:2. The only reasons to keep it are back-ward compatibility and allowing to have 2 (artificial) sub-phases in phase:2, right? I find this misleading (phase:1 is a fake phase compared to all other eal phases), but also very restrictive. Why not creating real sub-phases (full integers range), internal to ModSecurity to allow to sort rules inside any phase. Ex: "phase:2,sub-phase:286,..." "phase:3,sub-phase:14,..."? This would be more orthogonal and much more powerful.

[rcbarnett-zz]

ivanr: Missing responses:

1. Yes, the change will likely break inter-module communication.
2. I am not sure what you mean: can you give some examples of rules that worked before and don't work with 2.6?
3. The role of phase 1 is the same as it's always been (in my designs): it's main purpose is to decide how to handle request bodies and perform other similar activities for which you don't want to wait to see entire request bodies.

I am not responding to your feature suggestion as I am no longer in charge.

[rcbarnett-zz]

bpinto: Marc,

I agree with you this is now like a fake phase and maybe will break some module inter-communication. As it is a new code and it not clear the real impact of this for the community. I will wait to hear more from community. Revert this change made is a possibility in the future. If not your sub-phase idea can also be a feature to remove the sense of "fake" with these two phases. However i think the sub-phases can confuse users when write rules. I have heard from many users that SecRules are corrently confuse to write. So... i'm thinking in a solution for this.

Thanks

Breno

[rcbarnett-zz]

marcstern: 1. As this is incompatible with 2.5.x, I strongly recommend to revert asap to old behaviour before finding a good solution.

1. Previously, when checking, in phase:1, that a URL is well formed, does not contain .., etc., the tests were performed before Apache compted the location, and potentially applied other settings. I understand that the rules will now run after Apache computes the location, so this part is not protected any more. Let's take a (fictive) example:
   • I protect my "/admin" location with SSLRequireSSL (maybe even with client authentication)
   • There is a directory traversal bug in the location computation
   • I am able to access "/public/../admin" in HTTP because of the bug
   • I have a rule in phase:1 that forbids ".." in URL It seems that 2.5 is protected with my rule, 2.6 not (completely) as the rule runs in (early) phase:2. I completely bypass SSL client authentication , at least to send the first part of the request (I can send corrupted headers).
2. About rule complexity: I think that having the phases like now is the simplest as it is consistent - users only have to be warned that phase:1 cannot be included in a location. Sub-phases would not be confusing, as this would be a consistent implementation. However, we could maybe find a better term (order, step, sorting, ...) I don't think sub-phases would be difficult, even for novice users, as they are optional. If you don't use them, you are in the current situation - nothing more complex. That also means that this mechanism is fully back-ward compatible.

[rcbarnett-zz]

ivanr: 1. Marc, your problem can be easily solved with an introduction of another phase, which would be equivalent to pre-2.6.x phase 1. In that way, ModSecurity would work for you as well as for others (who have suffered from other phase 1 related issues, e.g., no phase 1 rules working in tags).

[rcbarnett-zz]

rbarnett: We need to review this ticket and consider a change back to normal phase:1 - post-read-request.

[rcbarnett-zz]

marcstern: What is the status? Last remark was to change it back. Will it be the case?

Otherwise, I should have missed something ... I understand the concern about simplification, even if it breaks the compatibility. However, we now have a phase 1 that is strictly equivalent to phase 2! Is that a real simplification?

[rcbarnett-zz]

bpinto: Hi Marc,

We have a new configure option : ./configure --enable-request-early

So, you can define what you want : use the same hook for phase 1 and 2 or not.

[rcbarnett-zz]

marcstern: Great

[rcbarnett-zz]

ivanr: The new default in 2.7 is just plain wrong. If anything, the default 2.7 behaviour should have been the same as 2.6, with a configure-time switch for Marc to revert back to the old ways. As it is, we're again seeing users get confused because of the inability to control phase 1 rules from and contexts (e.g., MOSEC-381).

[rcbarnett-zz]

rbarnett: The default is the same as 2.6 - both phase:1 and phase:2 are in Apache fixup phase which allows controlling setting using Apache scope containers. If the user wants to move phase:1 back to post-read-request Apache hook, they need to use the --enable-request-early configure flag.

[rcbarnett-zz]

ivanr: Then there's a bug in 2.7.2, because I am seeing phase 1 rules executing in the early hook by default.

[rcbarnett-zz]

bpinto: No there is no bug. By default 2.7.x the phase 1 is not is post-read-request (like 2.5.x). Userd must do --enable-request-early=no to run as 2.6.x