AH00526 when using long ProxyPass worker name

[Brian Morton]

When using a long ProxyPass worker name such as unix:///var/php-fpm/146527084714328.sock|fcgi://localhost/home/mysite/domains/subdomain.com/public_html/$1 Apache issues the fatal error AH00526 and refuses to proceed during reload. This is a typical configuration generated by Virtualmin for a subdomain running php-fpm.

A couple of workarounds are available using mod_rewrite, but they do not use connection pooling for the proxy and aren't available for packaged solutions like Virtualmin. The patch from trunk is fairly straightforward.

[Brian Morton (rokclimb15)]

Patch attached for trusty. Unsure how to generate a debdiff in this case.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "Patch for trusty" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Robie Basak (racb)]

Brian, thank you for the patch.

Please see https://wiki.ubuntu.com/StableReleaseUpdates for information on getting Trusty updated. If this patch qualifies to land in Trusty, then please follow as much as you can of https://wiki.ubuntu.com/StableReleaseUpdates#Procedure. In particular, we need to make sure this bug is fixed in Zesty first.

It would be also helpful to know if this impacts Xenial, as I don't think it would be appropriate to fix Trusty without also fixing Xenial (otherwise users will regress when they upgrade).

Don't worry about the debdiff. Your patch is simple enough that we can convert it when we're ready to upload.

[Brian Morton (rokclimb15)]

Hi Robie, this fix is in Apache SVN trunk but isn't in upstream stable 2.4 release branch yet so all Ubuntu versions will be affected. In addition, no such patch exists in Debian source. https://anonscm.debian.org/cgit/pkg-apache/apache2.git/tree/modules/proxy/mod_proxy.h#n350 and https://anonscm.debian.org/cgit/pkg-apache/apache2.git/tree/debian/patches

I think the best bet is to add it as a debian patch for Zesty and coordinate it with the Debian Apache maintainers until it lands in a stable upstream release. I adjusted the line number in the patch so it should apply cleanly against Zesty.

[Robie Basak (racb)]

Hi Brian,

Do you know why this isn't in upstream's 2.4 branch? I'm wondering if it may result in an ABI break or similar issue.

[Brian Morton (rokclimb15)]

Hi Robie,

I do not know for certain. One user on the upstream bug mentions it can't be backported because of an API compat issue, but if there's one present I'm sure not seeing it. The define is entirely contained within that header file. The only thing I would think it could affect would be external modules built against mod_proxy.h

root@apache2-mod-proxy-length-xenial:~/apache2-2.4.18# grep -R 'PROXY_WORKER_MAX_NAME_SIZE' *
modules/proxy/mod_proxy.h:#define PROXY_WORKER_MAX_NAME_SIZE 96
modules/proxy/mod_proxy.h:#define PROXY_BALANCER_MAX_NAME_SIZE PROXY_WORKER_MAX_NAME_SIZE
modules/proxy/mod_proxy.h: char name[PROXY_WORKER_MAX_NAME_SIZE];
modules/proxy/mod_proxy.h: char uds_path[PROXY_WORKER_MAX_NAME_SIZE]; /* path to worker's unix domain socket if applicable */
root@apache2-mod-proxy-length-xenial:~/apache2-2.4.18# grep -R 'PROXY_BALANCER_MAX_NAME_SIZE' *
modules/proxy/mod_proxy.h:#define PROXY_BALANCER_MAX_NAME_SIZE PROXY_WORKER_MAX_NAME_SIZE
modules/proxy/mod_proxy.h: char name[PROXY_BALANCER_MAX_NAME_SIZE];
modules/proxy/mod_proxy.h: char sname[PROXY_BALANCER_MAX_NAME_SIZE];

[Robie Basak (racb)]

On Wed, Mar 29, 2017 at 01:36:17AM -0000, Brian Morton wrote:
> The only thing I would think it could affect would be
> external modules built against mod_proxy.h

Right - that's my concern.

[Brian Morton (rokclimb15)]

I don't think it is an actual incompatibility since the util function for copying those values takes into consideration the size of the dst buffer. As long as it is being lengthened, I think it's OK. It would be a problem if we were reducing it and src were larger than dst.

#define PROXY_STRNCPY(dst, src) ap_proxy_strncpy((dst), (src), (sizeof(dst)))

if (PROXY_STRNCPY(bshared->name, uri) != APR_SUCCESS) {
         return apr_psprintf(p, "balancer name (%s) too long", uri);
}

[Robie Basak (racb)]

OK, but why can't that argument be accepted by upstream in their 2.4 branch first?

[Brian Morton (rokclimb15)]

An excellent question/point. That should probably get hashed out on the upstream BZ first. The discussion about it was very minimal. I'll get on there and poke at that issue.

[Robie Basak (racb)]

Thanks! So I think our position for the time being is to hold this pending upstream's acceptance of the patch into their 2.4 branch. If something changes so we should reconsider, further discussion is welcome.