Plugins: net/haproxy: Action rules are not sorted properly in haproxy.conf

[astrandb]

Describe the bug
Redirect rules are not processed in proper order which leads to unpredictable behaviour. This is problematic e.g. when using haproxy and Let's encrypt combined.

I have looked into the generated haproxy.conf and I can see that ACL:s are sorted according to the order of entries in frontend gui (Public services). But linked actions are generated in wrong order. In fact the order is not changed if the rule entries are shuffled in gui.

Screenshots
Example 1

frontend the_front_80
    bind 0.0.0.0:80 name 0.0.0.0:80
    mode http
    option http-keep-alive
    # tuning options
    timeout client 30s

    # logging options
    # ACL: find_acme_challenge
    acl acl_5ec7b26972aa81.78795642 path_beg -i /.well-known/acme-challenge/
    # ACL: cam1_example_nu
    acl acl_5ec7baf41f9470.70661915 hdr(host) -i cam1.example.nu
    # ACL: is_http
    acl acl_5ec7b2fadbd856.27310243 ssl_fc
    # ACL: ull_example_nu
    acl acl_5ec7b3263f2652.69975208 hdr(host) -i ull.example.nu

    # ACTION: redirect_ha_http2https
    http-request redirect scheme https code 301 if !acl_5ec7b2fadbd856.27310243 acl_5ec7b3263f2652.69975208

    # ACTION: redirect_acme_challenges
    use_backend acme_challenge_backend if acl_5ec7b26972aa81.78795642
    # ACTION: redirect_cam1
    use_backend pool_cam1 if acl_5ec7baf41f9470.70661915

Example 2

frontend the_front_80
    bind 0.0.0.0:80 name 0.0.0.0:80
    mode http
    option http-keep-alive
    # tuning options
    timeout client 30s

    # logging options
    # ACL: find_acme_challenge
    acl acl_5ec7b26972aa81.78795642 path_beg -i /.well-known/acme-challenge/
    # ACL: is_http
    acl acl_5ec7b2fadbd856.27310243 ssl_fc
    # ACL: ull_example_nu
    acl acl_5ec7b3263f2652.69975208 hdr(host) -i ull.example.nu
    # ACL: cam1_example_nu
    acl acl_5ec7baf41f9470.70661915 hdr(host) -i cam1.example.nu

    # ACTION: redirect_ha_http2https
    http-request redirect scheme https code 301 if !acl_5ec7b2fadbd856.27310243 acl_5ec7b3263f2652.69975208

    # ACTION: redirect_acme_challenges
    use_backend acme_challenge_backend if acl_5ec7b26972aa81.78795642
    # ACTION: redirect_cam1
    use_backend pool_cam1 if acl_5ec7baf41f9470.70661915

Environment
OPNsense 20.1.8_1-amd64

[fraenki]

I was able to reproduce this issue. ~I think this was caused by the changes in https://github.com/opnsense/core/pull/4070 and https://github.com/opnsense/core/commit/3731bd1026963d0f3d3d66f662fb56ff58cc3f06.~

[AdSchellevis]

@fraenki these commits are likely not related (grid sorting shouldn't be used here). If you sort the config.xml, the output is as expected. My guess would be it's something related to Javascript (could be jquery).

[fraenki]

@AdSchellevis You're right, it looks like this was already broken in 20.1.4 (the oldest testbox I have available). :-/

[fraenki]

I did a new test on both 20.1.8 and 20.7-RC1 and I have to correct my previous statement: rule sorting works as expected.

However, there is one notable exception: use_[backend|server] rules are always added to the end of the ruleset. This was introduced back in 2018 with https://github.com/opnsense/plugins/commit/1f9250e15bdca9601535378b8a5b72fa3ab557f6. It was meant as a workaround to make these rules work (because rule sorting was not yet available back then). I have to say this is pretty unexpected behaviour and even though I've merged this change back then I was really surprised to see this now. Apparently this workaround was obsoleted by https://github.com/opnsense/plugins/issues/582.

I plan to remove the special handling of use_[backend|server] rules, so that the sort order is always honored.

[fraenki]

@astrandb Could you give https://github.com/opnsense/plugins/pull/1933/commits/1685999b9dd5cc659d99901d2eada79a8fa98c16 a try?

opnsense-patch -c plugins 1685999b9dd5cc659d99901d2eada79a8fa98c16

[astrandb]

@fraenki The patch works as expected, the rules are created in order, but "Test syntax" results in this message:
[WARNING] 206/084823 (97809) : parsing [/usr/local/etc/haproxy.conf:75] : a 'http-request' rule placed after a 'use_backend' rule will still be processed before. Configuration file is valid
After digging into HAProxy docs and other sources it seems that HAProxy processes http-request rules before use_backend. So in order to avoid the warning message it is probably best to create the use_backend rules after the http-request rules.

My use case is to redirect all http requests to https except those related to acme_challenges. When I got unpredictable results my first thought was that the order of the rules was the root of the problem. Perhaps I have to think of alternate ways to handle my use case, e.g. to process the http-https redirect on the backend instead.

[fraenki]

The HAProxy warning simply tells the user that the rules are placed in the wrong order. However, that's not a big issue, because (as stated by the warning message) HAProxy corrects this specific error internally.

I am aware that these messages will now appear for some users. They can easily be fixed by changing the order of these rules.

Still, I think it's a change for the better to have full control over the sort order.