Commit 1c4f17b8 authored by Yann Ylavic's avatar Yann Ylavic
Browse files

util_filter: protect ap_filter_t private fields from external (ab)use. 

Introduce opaque struct ap_filter_private to move ap_filter_t "pending", "bb"
and "deferred_pool" fields to the "priv" side of things.

This allows to trust values set internally (only!) in util_filter code, and
make useful assertions between the different functions calls, along with the
usual nice extensibility property.

Likewise, the private struct ap_filter_conn_ctx in conn_rec (from r1839997)
allows now to implement the new ap_acquire_brigade() and ap_release_brigade()
functions useful to get a brigade with c->pool's lifetime. They obsolete
ap_reuse_brigade_from_pool() which is replaced where previously used.

Some comments added in ap_request_core_filter() regarding the lifetime of the
data it plays with, up to EOR...

MAJOR bumped (once again).


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1840149 13f79535-47bb-0310-9956-ffa450edef68
parent c5823ce8

include/ap_mmn.h

+6 −2
Changes for include/ap_mmn.h: 6 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -602,14 +602,18 @@ 
 *                         filter_conn_ctx, remove argument pool from

 *                         ap_filter_prepare_brigade()

 * 20180903.2 (2.5.1-dev)  Add ap_filter_recycle()

 * 20180905.1 (2.5.1-dev)  Axe ap_reuse_brigade_from_pool(), replaced by

 *                         ap_acquire_brigade()/ap_release_brigade(), and

 *                         and replace pending/bb/deferred_pool fields in

 *                         ap_filter_t by struct ap_filter_private priv field

 */



#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */



#ifndef MODULE_MAGIC_NUMBER_MAJOR

#define MODULE_MAGIC_NUMBER_MAJOR 20180903

#define MODULE_MAGIC_NUMBER_MAJOR 20180905

#endif

#define MODULE_MAGIC_NUMBER_MINOR 2                 /* 0...n */

#define MODULE_MAGIC_NUMBER_MINOR 1                 /* 0...n */



/**

 * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

include/httpd.h

+2 −15
Changes for include/httpd.h: 2 added lines, 15 removed lines.
Original line number Diff line number Diff line
@@ -1111,7 +1111,7 @@ typedef enum { 
    AP_CONN_KEEPALIVE

} ap_conn_keepalive_e;



/* For struct ap_filter_conn_ctx */

/* For struct ap_filter and ap_filter_conn_ctx */

#include "util_filter.h"



/**
@@ -1224,7 +1224,7 @@ struct conn_rec { 
    /** Array of requests being handled under this connection. */

    apr_array_header_t *requests;



    /** Filters' context for this connection */

    /** Filters private/opaque context for this connection */

    struct ap_filter_conn_ctx *filter_conn_ctx;



    /** The minimum level of filter type to allow setaside buckets */
@@ -2193,19 +2193,6 @@ AP_DECLARE(int) ap_request_has_body(request_rec *r); 
 */

AP_DECLARE(int) ap_request_tainted(request_rec *r, int flags);



/**

 * Reuse a brigade from a pool, or create it on the given pool/alloc and

 * associate it with the given key for further reuse.

 *

 * @param key the key/id of the brigade

 * @param pool the pool to cache and create the brigade from

 * @param alloc the bucket allocator to be used by the brigade

 * @return the reused and cleaned up brigade, or a new one

 */

AP_DECLARE(apr_bucket_brigade *) ap_reuse_brigade_from_pool(const char *key,

                                                            apr_pool_t *pool,

                                                    apr_bucket_alloc_t *alloc);



/**

 * Cleanup a string (mainly to be filesystem safe)

 * We only allow '_' and alphanumeric chars. Non-printable

include/util_filter.h

+22 −9
Changes for include/util_filter.h: 22 added lines, 9 removed lines.
Original line number Diff line number Diff line
@@ -263,6 +263,11 @@ struct ap_filter_rec_t { 
    ap_filter_direction_e direction;

};



/**

 * @brief The private/opaque data in ap_filter_t.

 */

struct ap_filter_private;



/**

 * @brief The representation of a filter chain.

 *
@@ -293,21 +298,29 @@ struct ap_filter_t { 
     */

    conn_rec *c;



    /** Buffered data associated with the current filter. */

    apr_bucket_brigade *bb;



    /** Dedicated pool to use for deferred writes. */

    apr_pool_t *deferred_pool;



    /** Entry in ring of pending filters (with setaside buckets). */

    APR_RING_ENTRY(ap_filter_t) pending;

    /** Filter private/opaque data */

    struct ap_filter_private *priv;

};



/**

 * @brief The filters' context in conn_rec (opaque).

 * @brief The filters private/opaque context in conn_rec.

 */

struct ap_filter_conn_ctx;



/**

 * Acquire a brigade created on the connection pool/alloc.

 * @param c The connection

 * @return The brigade (cleaned up)

 */

AP_DECLARE(apr_bucket_brigade *) ap_acquire_brigade(conn_rec *c);



/**

 * Release and cleanup a brigade (created on the connection pool/alloc!).

 * @param c The connection

 * @param bb The brigade

 */

AP_DECLARE(void) ap_release_brigade(conn_rec *c, apr_bucket_brigade *bb);



/**

 * Get the current bucket brigade from the next filter on the filter

 * stack.  The filter returns an apr_status_t value.  If the bottom-most

modules/http/http_request.c

+6 −4
Changes for modules/http/http_request.c: 6 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -352,11 +352,12 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r) 
    conn_rec *c = r->connection;

    ap_filter_t *f;



    bb = ap_acquire_brigade(c);



    /* Send an EOR bucket through the output filter chain.  When

     * this bucket is destroyed, the request will be logged and

     * its pool will be freed

     */

    bb = ap_reuse_brigade_from_pool("ap_prah_bb", c->pool, c->bucket_alloc);

    b = ap_bucket_eor_create(c->bucket_alloc, r);

    APR_BRIGADE_INSERT_HEAD(bb, b);
@@ -399,7 +400,8 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r) 
     * until the next/real request comes in or the keepalive timeout expires.

     */

    (void)ap_check_pipeline(c, bb, DEFAULT_LIMIT_BLANK_LINES);

    apr_brigade_cleanup(bb);



    ap_release_brigade(c, bb);



    if (!c->aborted) {

        ap_filter_recycle(c);
@@ -507,7 +509,7 @@ AP_DECLARE(void) ap_process_request(request_rec *r) 
    ap_process_async_request(r);



    if (ap_run_input_pending(c) != OK) {

        bb = ap_reuse_brigade_from_pool("ap_pr_bb", c->pool, c->bucket_alloc);

        bb = ap_acquire_brigade(c);

        b = apr_bucket_flush_create(c->bucket_alloc);

        APR_BRIGADE_INSERT_HEAD(bb, b);

        rv = ap_pass_brigade(c->output_filters, bb);
@@ -520,7 +522,7 @@ AP_DECLARE(void) ap_process_request(request_rec *r) 
            ap_log_cerror(APLOG_MARK, APLOG_INFO, rv, c, APLOGNO(01581)

                          "flushing data to the client");

        }

        apr_brigade_cleanup(bb);

        ap_release_brigade(c, bb);

    }

    if (ap_extended_status) {

        ap_time_process_request(c->sbh, STOP_PREQUEST);

server/core_filters.c

+20 −19
Changes for server/core_filters.c: 20 added lines, 19 removed lines.
Original line number Diff line number Diff line
@@ -85,6 +85,7 @@ struct core_output_filter_ctx { 
};



struct core_filter_ctx {

    apr_bucket_brigade *bb;

    apr_bucket_brigade *tmpbb;

};
@@ -116,19 +117,19 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
    if (!ctx)

    {

        net->in_ctx = ctx = apr_palloc(f->c->pool, sizeof(*ctx));

        ap_filter_prepare_brigade(f);

        ctx->bb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);

        ctx->tmpbb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);

        /* seed the brigade with the client socket. */

        rv = ap_run_insert_network_bucket(f->c, f->bb, net->client_socket);

        rv = ap_run_insert_network_bucket(f->c, ctx->bb, net->client_socket);

        if (rv != APR_SUCCESS)

            return rv;

    }

    else if (APR_BRIGADE_EMPTY(f->bb)) {

    else if (APR_BRIGADE_EMPTY(ctx->bb)) {

        return APR_EOF;

    }



    /* ### This is bad. */

    BRIGADE_NORMALIZE(f->bb);

    BRIGADE_NORMALIZE(ctx->bb);



    /* check for empty brigade again *AFTER* BRIGADE_NORMALIZE()

     * If we have lost our socket bucket (see above), we are EOF.
@@ -136,13 +137,13 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
     * Ideally, this should be returning SUCCESS with EOS bucket, but

     * some higher-up APIs (spec. read_request_line via ap_rgetline)

     * want an error code. */

    if (APR_BRIGADE_EMPTY(f->bb)) {

    if (APR_BRIGADE_EMPTY(ctx->bb)) {

        return APR_EOF;

    }



    if (mode == AP_MODE_GETLINE) {

        /* we are reading a single LF line, e.g. the HTTP headers */

        rv = apr_brigade_split_line(b, f->bb, block, HUGE_STRING_LEN);

        rv = apr_brigade_split_line(b, ctx->bb, block, HUGE_STRING_LEN);

        /* We should treat EAGAIN here the same as we do for EOF (brigade is

         * empty).  We do this by returning whatever we have read.  This may

         * or may not be bogus, but is consistent (for now) with EOF logic.
@@ -170,10 +171,10 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
         * mean that there is another request, just a blank line.

         */

        while (1) {

            if (APR_BRIGADE_EMPTY(f->bb))

            if (APR_BRIGADE_EMPTY(ctx->bb))

                return APR_EOF;



            e = APR_BRIGADE_FIRST(f->bb);

            e = APR_BRIGADE_FIRST(ctx->bb);



            rv = apr_bucket_read(e, &str, &len, APR_NONBLOCK_READ);
@@ -212,7 +213,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
        apr_bucket *e;



        /* Tack on any buckets that were set aside. */

        APR_BRIGADE_CONCAT(b, f->bb);

        APR_BRIGADE_CONCAT(b, ctx->bb);



        /* Since we've just added all potential buckets (which will most

         * likely simply be the socket bucket) we know this is the end,
@@ -230,7 +231,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 


        AP_DEBUG_ASSERT(readbytes > 0);



        e = APR_BRIGADE_FIRST(f->bb);

        e = APR_BRIGADE_FIRST(ctx->bb);

        rv = apr_bucket_read(e, &str, &len, block);



        if (APR_STATUS_IS_EAGAIN(rv) && block == APR_NONBLOCK_READ) {
@@ -247,7 +248,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
             *

             * When we are in normal mode, return an EOS bucket to the

             * caller.

             * When we are in speculative mode, leave ctx->b empty, so

             * When we are in speculative mode, leave ctx->bb empty, so

             * that the next call returns an EOS bucket.

             */

            apr_bucket_delete(e);
@@ -267,7 +268,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
            /* We already registered the data in e in len */

            e = APR_BUCKET_NEXT(e);

            while ((len < readbytes) && (rv == APR_SUCCESS)

                   && (e != APR_BRIGADE_SENTINEL(f->bb))) {

                   && (e != APR_BRIGADE_SENTINEL(ctx->bb))) {

                /* Check for the availability of buckets with known length */

                if (e->length != -1) {

                    len += e->length;
@@ -295,22 +296,22 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
            readbytes = len;

        }



        rv = apr_brigade_partition(f->bb, readbytes, &e);

        rv = apr_brigade_partition(ctx->bb, readbytes, &e);

        if (rv != APR_SUCCESS) {

            return rv;

        }



        /* Must do move before CONCAT */

        ctx->tmpbb = apr_brigade_split_ex(f->bb, e, ctx->tmpbb);

        ctx->tmpbb = apr_brigade_split_ex(ctx->bb, e, ctx->tmpbb);



        if (mode == AP_MODE_READBYTES) {

            APR_BRIGADE_CONCAT(b, f->bb);

            APR_BRIGADE_CONCAT(b, ctx->bb);

        }

        else if (mode == AP_MODE_SPECULATIVE) {

            apr_bucket *copy_bucket;



            for (e = APR_BRIGADE_FIRST(f->bb);

                 e != APR_BRIGADE_SENTINEL(f->bb);

            for (e = APR_BRIGADE_FIRST(ctx->bb);

                 e != APR_BRIGADE_SENTINEL(ctx->bb);

                 e = APR_BUCKET_NEXT(e))

            {

                rv = apr_bucket_copy(e, &copy_bucket);
@@ -321,8 +322,8 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
            }

        }



        /* Take what was originally there and place it back on ctx->b */

        APR_BRIGADE_CONCAT(f->bb, ctx->tmpbb);

        /* Take what was originally there and place it back on ctx->bb */

        APR_BRIGADE_CONCAT(ctx->bb, ctx->tmpbb);

    }

    return APR_SUCCESS;

}