Environment variable inheritance using MDNotifyCmd and MDMessageCmd

[andreblanke]

Using httpd v2.4.57 and mod_md v2.4.21 on Linux 6.4.10, I've been trying to gracefully reload httpd from a simple script executed by MDNotifyCmd or MDMessageCmd:

#!/usr/bin/env sh
/usr/local/apache2/bin/apachectl -k graceful

However, it appears that environment variables are not correctly inherited by the child process, resulting in warnings akin to

AH00111: Config variable ${ENV_VAR} is not defined

because my httpd.conf relies on certain environment variables being set. If I extend the above script to include an invocation of printenv, only PWD=/usr/local/apache2 is printed as environment variable which seems to confirm my assumption.

Issue #198 looks related, but it is resolved and inheritance appears to work correctly in that case.

Reproducible example

I have written a small example program which – at least on my machine – demonstrates the issue. It uses the Apache Portable Runtime and md_util_exec along with stubs copied from mod_md.

md_util_exec is used to invoke /usr/bin/printenv twice, passing ENV_VAR=value as environment variable first and NULL second. The following output is produced by the program:

Executing printenv with env={"ENV_VAR=value"}:
ENV_VAR=value
printenv finished with exit code 0

Executing printenv with env=NULL:
printenv finished with exit code 0

As you can see, neither of the process executions inherit the environment variables of the parent process. By changing the cmdtype passed to apr_procattr_cmdtype_set from APR_PROGRAM to APR_PROGRAM_ENV, both processes seem to inherit the environment of the parent, but the additional ENV_VAR=value set in the first configuration is not set for the child process and thus lost.

-           && APR_SUCCESS == (rv = apr_procattr_cmdtype_set(procattr, APR_PROGRAM))
+           && APR_SUCCESS == (rv = apr_procattr_cmdtype_set(procattr, APR_PROGRAM_ENV))

Source code

#include <apr-1/apr_thread_proc.h>

//region md_util_exec and stubs
typedef enum {
    MD_LOG_EMERG,
    MD_LOG_ALERT,
    MD_LOG_CRIT,
    MD_LOG_ERR,
    MD_LOG_WARNING,
    MD_LOG_NOTICE,
    MD_LOG_INFO,
    MD_LOG_DEBUG,
    MD_LOG_TRACE1,
    MD_LOG_TRACE2,
    MD_LOG_TRACE3,
    MD_LOG_TRACE4,
    MD_LOG_TRACE5,
    MD_LOG_TRACE6,
    MD_LOG_TRACE7,
    MD_LOG_TRACE8,
} md_log_level_t;

#define MD_LOG_MARK __FILE__,__LINE__

void md_log_perror(const char *file, int line, md_log_level_t level,
                   apr_status_t rv, apr_pool_t *p, const char *fmt, ...)
{
}

apr_status_t md_util_exec(apr_pool_t *p, const char *cmd, const char * const *argv,
                          apr_array_header_t *env, int *exit_code)
{
    apr_status_t rv;
    apr_procattr_t *procattr;
    apr_proc_t *proc;
    apr_exit_why_e ewhy;
    const char * const *envp = NULL;
    char buffer[1024];

    *exit_code = 0;
    if (!(proc = apr_pcalloc(p, sizeof(*proc)))) {
        return APR_ENOMEM;
    }
    if (env && env->nelts > 0) {
        apr_array_header_t *nenv;

        nenv = apr_array_copy(p, env);
        APR_ARRAY_PUSH(nenv, const char *) = NULL;
        envp = (const char * const *)nenv->elts;
    }
    if (   APR_SUCCESS == (rv = apr_procattr_create(&procattr, p))
           && APR_SUCCESS == (rv = apr_procattr_io_set(procattr, APR_NO_FILE,
                                                       APR_NO_PIPE, APR_FULL_BLOCK))
           && APR_SUCCESS == (rv = apr_procattr_cmdtype_set(procattr, APR_PROGRAM))
           && APR_SUCCESS == (rv = apr_proc_create(proc, cmd, argv, envp, procattr, p))) {

        /* read stderr and log on INFO for possible fault analysis. */
        while(APR_SUCCESS == (rv = apr_file_gets(buffer, sizeof(buffer)-1, proc->err))) {
            md_log_perror(MD_LOG_MARK, MD_LOG_INFO, 0, p, "cmd(%s) stderr: %s", cmd, buffer);
        }
        if (!APR_STATUS_IS_EOF(rv)) goto out;
        apr_file_close(proc->err);

        if (APR_CHILD_DONE == (rv = apr_proc_wait(proc, exit_code, &ewhy, APR_WAIT))) {
            /* let's not dwell on exit stati, but core should signal something's bad */
            if (*exit_code > 127 || APR_PROC_SIGNAL_CORE == ewhy) {
                return APR_EINCOMPLETE;
            }
            return APR_SUCCESS;
        }
    }
    out:
    return rv;
}
//endregion

int main() {
    const char *prog = "/usr/bin/printenv";
    const char *argv[] = {prog, NULL};

    apr_initialize();

    apr_pool_t *pool;
    apr_pool_create(&pool, NULL);

    int exit_code;
    {
        const char *env_var = "ENV_VAR=value";

        apr_array_header_t *env = apr_array_make(pool, 1, sizeof(char *));
        APR_ARRAY_PUSH(env, const char *) = env_var;

        printf("Executing printenv with env={\"%s\"}:\n", env_var);
        md_util_exec(pool, prog, argv, env, &exit_code);
        printf("printenv finished with exit code %d\n", exit_code);
    }
    printf("\n");
    {
        printf("Executing printenv with env=NULL:\n");
        md_util_exec(pool, prog, argv, NULL, &exit_code);
        printf("printenv finished with exit code %d\n", exit_code);
    }
    return 0;
}

Closing words

I don't really understand why the functionality looks to be working in #198, but it sadly isn't in my case unless I am missing something.

As a fix, it could be possible to implement a function to retrieve all environment variables for POSIX and WIN32, include ones relevant for mod_md, and then pass the new list of variables to md_util_exec. The function could be extracted to APR at a later time.

[icing]

Thanks for the detailed report. Looking at this, I believe md_util_exec should use APR_PROGRAM_ENV and remove its env parameter since any value given there will not have an effect then, as you observed.

In the case of #198, we tried to supply additional vars to the called process, but that resulted in the inherited environment of httpd to be completely replaced. Which did lead to the failures reported there.

Note that #198 was on Windows, where APR_PROGRAM and APR_PROGRAM_ENV have the same implementation. So it did not matter there.

As to your original intention of reloading Apache in such a program, that will not work in default Linux setups where root is needed for reloads, but the invoked program will run as an unprivileged user like www-data.

[icing]

Could you give #320 a try?

[andreblanke]

Note that #198 was on Windows, where APR_PROGRAM and APR_PROGRAM_ENV have the same implementation. So it did not matter there.

Ah, so that was confusing me.

As to your original intention of reloading Apache in such a program, that will not work in default Linux setups where root is needed for reloads, but the invoked program will run as an unprivileged user like www-data.

Thanks for the heads-up. My original plan was to give www-data permission to execute only /usr/local/apache2/apachectl -k graceful with elevated rights, but using ports other than 80 and 443 might be the more secure approach for my use case.

Could you give #320 a try?

The PR fixes the issue for me. Thank you for your quick help.

[icing]

Thanks for testing.