From: Graham Leggett Date: Sat, 27 Jun 2020 23:41:00 +0000 (+0000) Subject: "[mod_dav_fs etag handling] should really honor the FileETag setting". X-Git-Tag: 2.5.0-alpha2-ci-test-only~1328 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8de97e5fabff6069844cc14f41a4bcb7759d5cdc;p=thirdparty%2Fapache%2Fhttpd.git "[mod_dav_fs etag handling] should really honor the FileETag setting". - It now does. - Add "Digest" to FileETag directive, allowing a strong ETag to be generated using a file digest. - Add ap_make_etag_ex() and ap_set_etag_fd() to allow full control over ETag generation. - Add concept of "binary notes" to request_rec, allowing packed bit flags to be added to a request. - First binary note - AP_REQUEST_STRONG_ETAG - allows modules to force the ETag to a strong ETag to comply with RFC requirements, such as those mandated by various WebDAV extensions. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1879285 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 2ef98d168d1..ae5c7e715fc 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,19 @@ -*- coding: utf-8 -*- Changes with Apache 2.5.1 + *) "[mod_dav_fs etag handling] should really honor the FileETag setting". + - It now does. + - Add "Digest" to FileETag directive, allowing a strong ETag to be + generated using a file digest. + - Add ap_make_etag_ex() and ap_set_etag_fd() to allow full control over + ETag generation. + - Add concept of "binary notes" to request_rec, allowing packed bit flags + to be added to a request. + - First binary note - AP_REQUEST_STRONG_ETAG - allows modules to force + the ETag to a strong ETag to comply with RFC requirements, such as those + mandated by various WebDAV extensions. + [Graham Leggett] + *) mod_ssl: Fix a race condition and possible crash when using a proxy client certificate (SSLProxyMachineCertificateFile). [Armin Abfalterer ] diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml index af609890cf5..1ca415bbd09 100644 --- a/docs/manual/mod/core.xml +++ b/docs/manual/mod/core.xml @@ -1926,15 +1926,18 @@ earlier. FileETag INode MTime Size +
Digest
+
If a document is file-based, the ETag field will be + calculated by taking the digest over the file.
None
If a document is file-based, no ETag field will be included in the response
-

The INode, MTime, and Size - keywords may be prefixed with either + or -, - which allow changes to be made to the default setting inherited - from a broader scope. Any keyword appearing without such a prefix +

The INode, MTime, Size and + Digest keywords may be prefixed with either + + or -, which allow changes to be made to the default setting + inherited from a broader scope. Any keyword appearing without such a prefix immediately and completely cancels the inherited setting.

If a directory's configuration includes @@ -1943,18 +1946,10 @@ FileETag INode MTime Size the setting for that subdirectory (which will be inherited by any sub-subdirectories that don't override it) will be equivalent to FileETag MTime Size.

- Warning - Do not change the default for directories or locations that have WebDAV - enabled and use mod_dav_fs as a storage provider. - mod_dav_fs uses MTime Size - as a fixed format for ETag comparisons on conditional requests. - These conditional requests will break if the ETag format is - changed via FileETag. - Server Side Includes An ETag is not generated for responses parsed by mod_include - since the response entity can change without a change of the INode, MTime, or Size - of the static file with embedded SSI directives. + since the response entity can change without a change of the INode, MTime, + Size or Digest of the static file with embedded SSI directives. diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 7d9f5e279e4..fc32a1ea1de 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -636,6 +636,11 @@ * 20200420.5 (2.5.1-dev) Add pre_translate_name hook * 20200420.6 (2.5.1-dev) Add map_encoded_one and map_encoded_all bits to * proxy_server_conf + * 20200420.7 (2.5.1-dev) Add struct etag_rec, ap_make_etag_ex(), + * ap_set_etag_fd(). Add typedef ap_request_bnotes_t, + * macros AP_REQUEST_GET_BNOTE, AP_REQUEST_SET_BNOTE, + * AP_REQUEST_STRONG_ETAG, AP_REQUEST_IS_STRONG_ETAG. + * Add bnotes to request_rec. */ #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */ @@ -643,7 +648,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20200420 #endif -#define MODULE_MAGIC_NUMBER_MINOR 6 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 7 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/include/http_core.h b/include/http_core.h index a64a1888d6f..2bcd713560c 100644 --- a/include/http_core.h +++ b/include/http_core.h @@ -489,12 +489,13 @@ typedef unsigned int overrides_t; */ typedef unsigned long etag_components_t; -#define ETAG_UNSET 0 -#define ETAG_NONE (1 << 0) -#define ETAG_MTIME (1 << 1) -#define ETAG_INODE (1 << 2) -#define ETAG_SIZE (1 << 3) -#define ETAG_ALL (ETAG_MTIME | ETAG_INODE | ETAG_SIZE) +#define ETAG_UNSET 0 +#define ETAG_NONE (1 << 0) +#define ETAG_MTIME (1 << 1) +#define ETAG_INODE (1 << 2) +#define ETAG_SIZE (1 << 3) +#define ETAG_DIGEST (1 << 4) +#define ETAG_ALL (ETAG_MTIME | ETAG_INODE | ETAG_SIZE) /* This is the default value used */ #define ETAG_BACKWARD (ETAG_MTIME | ETAG_SIZE) diff --git a/include/http_protocol.h b/include/http_protocol.h index b7953d0a6b1..9c9cb952b23 100644 --- a/include/http_protocol.h +++ b/include/http_protocol.h @@ -166,6 +166,27 @@ AP_DECLARE(const char *) ap_make_content_type(request_rec *r, */ AP_DECLARE(void) ap_setup_make_content_type(apr_pool_t *pool); +/** A structure with the ingredients for a file based etag */ +typedef struct etag_rec etag_rec; + +/** + * @brief A structure with the ingredients for a file based etag + */ +struct etag_rec { + /** Optional vary list validator */ + const char *vlist_validator; + /** Time when the request started */ + apr_time_t request_time; + /** finfo.protection (st_mode) set to zero if no such file */ + apr_finfo_t *finfo; + /** File pathname used when generating a digest */ + const char *pathname; + /** File descriptor used when generating a digest */ + apr_file_t *fd; + /** Force a non-digest etag to be weak */ + int force_weak; +}; + /** * Construct an entity tag from the resource information. If it's a real * file, build in some of the file characteristics. @@ -176,12 +197,27 @@ AP_DECLARE(void) ap_setup_make_content_type(apr_pool_t *pool); */ AP_DECLARE(char *) ap_make_etag(request_rec *r, int force_weak); +/** + * Construct an entity tag from information provided in the etag_rec + * structure. + * @param r The current request + * @param er The etag record, containing ingredients for the etag. + */ +AP_DECLARE(char *) ap_make_etag_ex(request_rec *r, etag_rec *er); + /** * Set the E-tag outgoing header * @param r The current request */ AP_DECLARE(void) ap_set_etag(request_rec *r); +/** + * Set the E-tag outgoing header, with the option of forcing a strong ETag. + * @param r The current request + * @param fd The file descriptor + */ +AP_DECLARE(void) ap_set_etag_fd(request_rec *r, apr_file_t *fd); + /** * Set the last modified time for the file being sent * @param r The current request @@ -762,7 +798,7 @@ AP_DECLARE_HOOK(const char *,http_scheme,(const request_rec *r)) AP_DECLARE_HOOK(apr_port_t,default_port,(const request_rec *r)) -#define AP_PROTOCOL_HTTP1 "http/1.1" +#define AP_PROTOCOL_HTTP1 "http/1.1" /** * Determine the list of protocols available for a connection/request. This may @@ -1011,6 +1047,7 @@ AP_DECLARE(void) ap_finalize_sub_req_protocol(request_rec *sub_r); */ AP_DECLARE(void) ap_send_interim_response(request_rec *r, int send_headers); + #ifdef __cplusplus } #endif diff --git a/include/httpd.h b/include/httpd.h index ad38f2c9271..5e4c036d8a6 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -645,8 +645,6 @@ typedef apr_uint64_t ap_method_mask_t; * The method mask bit to shift for anding with a bitmask. */ #define AP_METHOD_BIT ((ap_method_mask_t)1) -/** @} */ - /** @see ap_method_list_t */ typedef struct ap_method_list_t ap_method_list_t; @@ -664,6 +662,49 @@ struct ap_method_list_t { /** the array used for extension methods */ apr_array_header_t *method_list; }; +/** @} */ + +/** + * @defgroup bnotes Binary notes recognized by the server + * @ingroup APACHE_CORE_DAEMON + * @{ + * + * @brief Binary notes recognized by the server. + */ + +/** + * The type used for request binary notes. + */ +typedef apr_uint64_t ap_request_bnotes_t; + +/** + * These constants represent bitmasks for notes associated with this + * request. There are space for 64 bits in the apr_uint64_t. + * + */ +#define AP_REQUEST_STRONG_ETAG 1 >> 0 + +/** + * This is a convenience macro to ease with getting specific request + * binary notes. + */ +#define AP_REQUEST_GET_BNOTE(r, mask) \ + ((mask) & ((r)->bnotes)) + +/** + * This is a convenience macro to ease with setting specific request + * binary notes. + */ +#define AP_REQUEST_SET_BNOTE(r, mask, val) \ + (r)->bnotes = (((r)->bnotes & ~(mask)) | (val)) + +/** + * Returns true if the strong etag flag is set for this request. + */ +#define AP_REQUEST_IS_STRONG_ETAG(r) \ + AP_REQUEST_GET_BNOTE((r), AP_REQUEST_STRONG_ETAG) +/** @} */ + /** * @defgroup module_magic Module Magic mime types @@ -1097,6 +1138,11 @@ struct request_rec { * TODO: compact elsewhere */ unsigned int flushed:1; + /** Request flags associated with this request. Use + * AP_REQUEST_GET_FLAGS() and AP_REQUEST_SET_FLAGS() to access + * the elements of this field. + */ + ap_request_bnotes_t bnotes; }; /** diff --git a/modules/dav/fs/repos.c b/modules/dav/fs/repos.c index 58d410ba9cc..ea08dfd924a 100644 --- a/modules/dav/fs/repos.c +++ b/modules/dav/fs/repos.c @@ -1853,27 +1853,26 @@ static dav_error * dav_fs_walk(const dav_walk_params *params, int depth, return dav_fs_internal_walk(params, depth, 0, NULL, response); } -/* dav_fs_etag: Stolen from ap_make_etag. Creates a strong etag - * for file path. - * ### do we need to return weak tags sometimes? +/* dav_fs_etag: Creates an etag for the file path. */ static const char *dav_fs_getetag(const dav_resource *resource) { - dav_resource_private *ctx = resource->info; - /* XXX: This should really honor the FileETag setting */ + etag_rec er; - if (!resource->exists) - return apr_pstrdup(ctx->pool, ""); + dav_resource_private *ctx = resource->info; - if (ctx->finfo.filetype != APR_NOFILE) { - return apr_psprintf(ctx->pool, "\"%" APR_UINT64_T_HEX_FMT "-%" - APR_UINT64_T_HEX_FMT "\"", - (apr_uint64_t) ctx->finfo.size, - (apr_uint64_t) ctx->finfo.mtime); + if (!resource->exists) { + return ""; } - return apr_psprintf(ctx->pool, "\"%" APR_UINT64_T_HEX_FMT "\"", - (apr_uint64_t) ctx->finfo.mtime); + er.vlist_validator = NULL; + er.request_time = ctx->r->request_time; + er.finfo = &ctx->finfo; + er.pathname = ctx->pathname; + er.fd = NULL; + er.force_weak = 0; + + return ap_make_etag_ex(ctx->r, &er); } static const dav_hooks_repository dav_hooks_repository_fs = diff --git a/modules/test/mod_dialup.c b/modules/test/mod_dialup.c index 2ec23844263..ef80d86c457 100644 --- a/modules/test/mod_dialup.c +++ b/modules/test/mod_dialup.c @@ -187,7 +187,7 @@ dialup_handler(request_rec *r) /* copied from default handler: */ ap_update_mtime(r, r->finfo.mtime); ap_set_last_modified(r); - ap_set_etag(r); + ap_set_etag_fd(r, fd); ap_set_accept_ranges(r); ap_set_content_length(r, r->finfo.size); diff --git a/server/core.c b/server/core.c index a0e10fec23b..8bdac75bf0f 100644 --- a/server/core.c +++ b/server/core.c @@ -2241,6 +2241,9 @@ static const char *set_etag_bits(cmd_parms *cmd, void *mconfig, else if (ap_cstr_casecmp(token, "INode") == 0) { bit = ETAG_INODE; } + else if (ap_cstr_casecmp(token, "Digest") == 0) { + bit = ETAG_DIGEST; + } else { return apr_pstrcat(cmd->pool, "Unknown keyword '", token, "' for ", cmd->cmd->name, @@ -5181,7 +5184,7 @@ static int default_handler(request_rec *r) ap_update_mtime(r, r->finfo.mtime); ap_set_last_modified(r); - ap_set_etag(r); + ap_set_etag_fd(r, fd); ap_set_accept_ranges(r); ap_set_content_length(r, r->finfo.size); if (bld_content_md5) { diff --git a/server/util_etag.c b/server/util_etag.c index 7f3c6d93590..bc2f9861e0d 100644 --- a/server/util_etag.c +++ b/server/util_etag.c @@ -16,6 +16,8 @@ #include "apr_strings.h" #include "apr_thread_proc.h" /* for RLIMIT stuff */ +#include "apr_sha1.h" +#include "apr_base64.h" #define APR_WANT_STRFUNC #include "apr_want.h" @@ -53,18 +55,42 @@ static char *etag_uint64_to_hex(char *next, apr_uint64_t u) #define ETAG_WEAK "W/" #define CHARS_PER_UINT64 (sizeof(apr_uint64_t) * 2) + +static void etag_start(char *etag, const char *weak, char **next) +{ + if (weak) { + while (*weak) { + *etag++ = *weak++; + } + } + *etag++ = '"'; + + *next = etag; +} + +static void etag_end(char *next, const char *vlv, apr_size_t vlv_len) +{ + if (vlv) { + *next++ = ';'; + apr_cpystrn(next, vlv, vlv_len); + } + else { + *next++ = '"'; + *next = '\0'; + } +} + /* * Construct an entity tag (ETag) from resource information. If it's a real * file, build in some of the file characteristics. If the modification time * is newer than (request-time minus 1 second), mark the ETag as weak - it - * could be modified again in as short an interval. We rationalize the - * modification time we're given to keep it from being in the future. + * could be modified again in as short an interval. */ -AP_DECLARE(char *) ap_make_etag(request_rec *r, int force_weak) +AP_DECLARE(char *) ap_make_etag_ex(request_rec *r, etag_rec *er) { - char *weak; - apr_size_t weak_len; - char *etag; + char *weak = NULL; + apr_size_t weak_len = 0, vlv_len = 0; + char *etag, *vlv; char *next; core_dir_config *cfg; etag_components_t etag_bits; @@ -73,13 +99,96 @@ AP_DECLARE(char *) ap_make_etag(request_rec *r, int force_weak) cfg = (core_dir_config *)ap_get_core_module_config(r->per_dir_config); etag_bits = (cfg->etag_bits & (~ cfg->etag_remove)) | cfg->etag_add; + if (er->force_weak) { + weak = ETAG_WEAK; + weak_len = sizeof(ETAG_WEAK); + } + + if (r->vlist_validator) { + + /* If we have a variant list validator (vlv) due to the + * response being negotiated, then we create a structured + * entity tag which merges the variant etag with the variant + * list validator (vlv). This merging makes revalidation + * somewhat safer, ensures that caches which can deal with + * Vary will (eventually) be updated if the set of variants is + * changed, and is also a protocol requirement for transparent + * content negotiation. + */ + + /* if the variant list validator is weak, we make the whole + * structured etag weak. If we would not, then clients could + * have problems merging range responses if we have different + * variants with the same non-globally-unique strong etag. + */ + + vlv = r->vlist_validator; + if (vlv[0] == 'W') { + vlv += 3; + weak = ETAG_WEAK; + weak_len = sizeof(ETAG_WEAK); + } + else { + vlv++; + } + vlv_len = strlen(vlv); + + } + else { + vlv = NULL; + vlv_len = 0; + } + + /* + * Did a module flag the need for a strong etag, or did the + * configuration tell us to generate a digest? + */ + if (er->finfo->filetype == APR_REG && + (AP_REQUEST_IS_STRONG_ETAG(r) || (etag_bits & ETAG_DIGEST))) { + + apr_sha1_ctx_t context; + unsigned char buf[4096]; /* keep this a multiple of 64 */ + unsigned char digest[APR_SHA1_DIGESTSIZE]; + apr_file_t *fd = NULL; + + apr_size_t nbytes; + apr_off_t offset = 0L; + + if (er->fd) { + fd = er->fd; + } + else if (er->pathname) { + apr_file_open(&fd, er->pathname, APR_READ | APR_BINARY, 0, r->pool); + } + if (!fd) { + return ""; + } + + etag = apr_palloc(r->pool, weak_len + sizeof("\"\"") + + 4*(APR_SHA1_DIGESTSIZE/3) + vlv_len + 4); + + apr_sha1_init(&context); + nbytes = sizeof(buf); + while (apr_file_read(fd, buf, &nbytes) == APR_SUCCESS) { + apr_sha1_update_binary(&context, buf, nbytes); + nbytes = sizeof(buf); + } + apr_file_seek(fd, APR_SET, &offset); + apr_sha1_final(digest, &context); + + etag_start(etag, weak, &next); + next += apr_base64_encode_binary(next, digest, APR_SHA1_DIGESTSIZE) - 1; + etag_end(next, vlv, vlv_len); + + return etag; + } + /* * If it's a file (or we wouldn't be here) and no ETags * should be set for files, return an empty string and * note it for the header-sender to ignore. */ if (etag_bits & ETAG_NONE) { - apr_table_setn(r->notes, "no-etag", "omit"); return ""; } @@ -98,123 +207,117 @@ AP_DECLARE(char *) ap_make_etag(request_rec *r, int force_weak) * be modified again later in the second, and the validation * would be incorrect. */ - if ((r->request_time - r->mtime > (1 * APR_USEC_PER_SEC)) && - !force_weak) { - weak = NULL; - weak_len = 0; - } - else { + if ((er->request_time - er->finfo->mtime < (1 * APR_USEC_PER_SEC))) { weak = ETAG_WEAK; weak_len = sizeof(ETAG_WEAK); } - if (r->finfo.filetype != APR_NOFILE) { + if (er->finfo->filetype != APR_NOFILE) { /* * ETag gets set to [W/]"inode-size-mtime", modulo any * FileETag keywords. */ etag = apr_palloc(r->pool, weak_len + sizeof("\"--\"") + - 3 * CHARS_PER_UINT64 + 1); - next = etag; - if (weak) { - while (*weak) { - *next++ = *weak++; - } - } - *next++ = '"'; + 3 * CHARS_PER_UINT64 + vlv_len + 2); + + etag_start(etag, weak, &next); + bits_added = 0; if (etag_bits & ETAG_INODE) { - next = etag_uint64_to_hex(next, r->finfo.inode); + next = etag_uint64_to_hex(next, er->finfo->inode); bits_added |= ETAG_INODE; } if (etag_bits & ETAG_SIZE) { if (bits_added != 0) { *next++ = '-'; } - next = etag_uint64_to_hex(next, r->finfo.size); + next = etag_uint64_to_hex(next, er->finfo->size); bits_added |= ETAG_SIZE; } if (etag_bits & ETAG_MTIME) { if (bits_added != 0) { *next++ = '-'; } - next = etag_uint64_to_hex(next, r->mtime); + next = etag_uint64_to_hex(next, er->finfo->mtime); } - *next++ = '"'; - *next = '\0'; + + etag_end(next, vlv, vlv_len); + } else { /* * Not a file document, so just use the mtime: [W/]"mtime" */ etag = apr_palloc(r->pool, weak_len + sizeof("\"\"") + - CHARS_PER_UINT64 + 1); - next = etag; - if (weak) { - while (*weak) { - *next++ = *weak++; - } - } - *next++ = '"'; - next = etag_uint64_to_hex(next, r->mtime); - *next++ = '"'; - *next = '\0'; + CHARS_PER_UINT64 + vlv_len + 2); + + etag_start(etag, weak, &next); + next = etag_uint64_to_hex(next, er->finfo->mtime); + etag_end(next, vlv, vlv_len); + } return etag; } +AP_DECLARE(char *) ap_make_etag(request_rec *r, int force_weak) +{ + etag_rec er; + + er.vlist_validator = NULL; + er.request_time = r->request_time; + er.finfo = &r->finfo; + er.pathname = r->filename; + er.fd = NULL; + er.force_weak = force_weak; + + return ap_make_etag_ex(r, &er); +} + AP_DECLARE(void) ap_set_etag(request_rec *r) { char *etag; - char *variant_etag, *vlv; - int vlv_weak; - if (!r->vlist_validator) { - etag = ap_make_etag(r, 0); + etag_rec er; - /* If we get a blank etag back, don't set the header. */ - if (!etag[0]) { - return; - } + er.vlist_validator = r->vlist_validator; + er.request_time = r->request_time; + er.finfo = &r->finfo; + er.pathname = r->filename; + er.fd = NULL; + er.force_weak = 0; + + etag = ap_make_etag_ex(r, &er); + + if (etag && etag[0]) { + apr_table_setn(r->headers_out, "ETag", etag); } else { - /* If we have a variant list validator (vlv) due to the - * response being negotiated, then we create a structured - * entity tag which merges the variant etag with the variant - * list validator (vlv). This merging makes revalidation - * somewhat safer, ensures that caches which can deal with - * Vary will (eventually) be updated if the set of variants is - * changed, and is also a protocol requirement for transparent - * content negotiation. - */ + apr_table_setn(r->notes, "no-etag", "omit"); + } - /* if the variant list validator is weak, we make the whole - * structured etag weak. If we would not, then clients could - * have problems merging range responses if we have different - * variants with the same non-globally-unique strong etag. - */ +} - vlv = r->vlist_validator; - vlv_weak = (vlv[0] == 'W'); +AP_DECLARE(void) ap_set_etag_fd(request_rec *r, apr_file_t *fd) +{ + char *etag; - variant_etag = ap_make_etag(r, vlv_weak); + etag_rec er; - /* If we get a blank etag back, don't append vlv and stop now. */ - if (!variant_etag[0]) { - return; - } + er.vlist_validator = r->vlist_validator; + er.request_time = r->request_time; + er.finfo = &r->finfo; + er.pathname = NULL; + er.fd = fd; + er.force_weak = 0; - /* merge variant_etag and vlv into a structured etag */ - variant_etag[strlen(variant_etag) - 1] = '\0'; - if (vlv_weak) { - vlv += 3; - } - else { - vlv++; - } - etag = apr_pstrcat(r->pool, variant_etag, ";", vlv, NULL); + etag = ap_make_etag_ex(r, &er); + + if (etag && etag[0]) { + apr_table_setn(r->headers_out, "ETag", etag); + } + else { + apr_table_setn(r->notes, "no-etag", "omit"); } - apr_table_setn(r->headers_out, "ETag", etag); }