For the most part, yes except the portions that make the header presence optional (the HDR_MISSING case). Those were added as it came into the code base to handle a use case I was working on. I've added some comments inline since I won't have time to poke around myself for a while yet.
For convenience, here's a link to the original code https://github.com/roadrunner2/mod-proxy-protocol/blob/master/mod_proxy_protocol.c On 1/16/2017 10:14 AM, Jim Jagielski wrote: > Let me take a look... afaict, this is a copy of what > was donated, which has been working for numerous people. > But that doesn't mean it can't have bugs ;) > >> On Jan 16, 2017, at 7:20 AM, Ruediger Pluem <rpl...@apache.org> wrote: >> >> Anyone? Apologies for missing the original reply >> Regards >> >> Rüdiger >> >> On 01/10/2017 12:39 PM, Ruediger Pluem wrote: >>> >>> On 12/30/2016 03:20 PM, drugg...@apache.org wrote: >>>> Author: druggeri >>>> Date: Fri Dec 30 14:20:48 2016 >>>> New Revision: 1776575 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1776575&view=rev >>>> Log: >>>> Merge new PROXY protocol code into mod_remoteip >>>> >>>> Modified: >>>> httpd/httpd/trunk/docs/log-message-tags/next-number >>>> httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml >>>> httpd/httpd/trunk/modules/metadata/mod_remoteip.c >>>> >>>> ============================================================================== >>>> --- httpd/httpd/trunk/modules/metadata/mod_remoteip.c (original) >>>> +++ httpd/httpd/trunk/modules/metadata/mod_remoteip.c Fri Dec 30 14:20:48 >>>> 2016 >>>> @@ -427,6 +730,464 @@ static int remoteip_modify_request(reque >>>> return OK; >>>> } >>>> >>>> +static int remoteip_is_server_port(apr_port_t port) >>>> +{ >>>> + ap_listen_rec *lr; >>>> + >>>> + for (lr = ap_listeners; lr; lr = lr->next) { >>>> + if (lr->bind_addr && lr->bind_addr->port == port) { >>>> + return 1; >>>> + } >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +/* >>>> + * Human readable format: >>>> + * PROXY {TCP4|TCP6|UNKNOWN} <client-ip-addr> <dest-ip-addr> >>>> <client-port> <dest-port><CR><LF> >>>> + */ >>>> +static remoteip_parse_status_t remoteip_process_v1_header(conn_rec *c, >>>> + >>>> remoteip_conn_config_t *conn_conf, >>>> + proxy_header >>>> *hdr, apr_size_t len, >>>> + apr_size_t >>>> *hdr_len) >>>> +{ >>>> + char *end, *word, *host, *valid_addr_chars, *saveptr; >>>> + char buf[sizeof(hdr->v1.line)]; >>>> + apr_port_t port; >>>> + apr_status_t ret; >>>> + apr_int32_t family; >>>> + >>>> +#define GET_NEXT_WORD(field) \ >>>> + word = apr_strtok(NULL, " ", &saveptr); \ >>>> + if (!word) { \ >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(03497) \ >>>> + "RemoteIPProxyProtocol: no " field " found in >>>> header '%s'", \ >>>> + hdr->v1.line); \ >>>> + return HDR_ERROR; \ >>>> + } >>>> + >>>> + end = memchr(hdr->v1.line, '\r', len - 1); >>>> + if (!end || end[1] != '\n') { >>>> + return HDR_NEED_MORE; /* partial or invalid header */ >>>> + } >>>> + >>>> + *end = '\0'; >>>> + *hdr_len = end + 2 - hdr->v1.line; /* skip header + CRLF */ >>>> + >>>> + /* parse in separate buffer so have the original for error messages */ >>>> + strcpy(buf, hdr->v1.line); >>>> + >>>> + apr_strtok(buf, " ", &saveptr); >>>> + >>>> + /* parse family */ >>>> + GET_NEXT_WORD("family") >>>> + if (strcmp(word, "UNKNOWN") == 0) { >>>> + conn_conf->client_addr = c->client_addr; >>>> + conn_conf->client_ip = c->client_ip; >>>> + return HDR_DONE; >>>> + } >>>> + else if (strcmp(word, "TCP4") == 0) { >>>> + family = APR_INET; >>>> + valid_addr_chars = "0123456789."; >>>> + } >>>> + else if (strcmp(word, "TCP6") == 0) { >>>> +#if APR_HAVE_IPV6 >>>> + family = APR_INET6; >>>> + valid_addr_chars = "0123456789abcdefABCDEF:"; >>>> +#else >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(03498) >>>> + "RemoteIPProxyProtocol: Unable to parse v6 address >>>> - APR is not compiled with IPv6 support", >>>> + word, hdr->v1.line); >>>> + return HDR_ERROR; >>>> +#endif >>>> + } >>>> + else { >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(03499) >>>> + "RemoteIPProxyProtocol: unknown family '%s' in >>>> header '%s'", >>>> + word, hdr->v1.line); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> + /* parse client-addr */ >>>> + GET_NEXT_WORD("client-address") >>>> + >>>> + if (strspn(word, valid_addr_chars) != strlen(word)) { >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(03500) >>>> + "RemoteIPProxyProtocol: invalid client-address '%s' >>>> found in " >>>> + "header '%s'", word, hdr->v1.line); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> + host = word; >>>> + >>>> + /* parse dest-addr */ >>>> + GET_NEXT_WORD("destination-address") >>>> + >>>> + /* parse client-port */ >>>> + GET_NEXT_WORD("client-port") >>>> + if (sscanf(word, "%hu", &port) != 1) { >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(03501) >>>> + "RemoteIPProxyProtocol: error parsing port '%s' in >>>> header '%s'", >>>> + word, hdr->v1.line); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> + /* parse dest-port */ >>>> + /* GET_NEXT_WORD("destination-port") - no-op since we don't care >>>> about it */ >>>> + >>>> + /* create a socketaddr from the info */ >>>> + ret = apr_sockaddr_info_get(&conn_conf->client_addr, host, family, >>>> port, 0, >>>> + c->pool); >>>> + if (ret != APR_SUCCESS) { >>>> + conn_conf->client_addr = NULL; >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, ret, c, APLOGNO(03502) >>>> + "RemoteIPProxyProtocol: error converting family >>>> '%d', host '%s'," >>>> + " and port '%hu' to sockaddr; header was '%s'", >>>> + family, host, port, hdr->v1.line); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> + conn_conf->client_ip = apr_pstrdup(c->pool, host); >>>> + >>>> + return HDR_DONE; >>>> +} >>>> + >>>> +/** Add our filter to the connection if it is requested >>>> + */ >>>> +static int remoteip_hook_pre_connection(conn_rec *c, void *csd) >>>> +{ >>>> + remoteip_config_t *conf; >>>> + remoteip_conn_config_t *conn_conf; >>>> + int optional; >>>> + >>>> + conf = ap_get_module_config(ap_server_conf->module_config, >>>> + &remoteip_module); >>>> + >>>> + /* Used twice - do the check only once */ >>>> + optional = remoteip_addr_in_list(conf->proxy_protocol_optional, >>>> c->local_addr); >>>> + >>>> + /* check if we're enabled for this connection */ >>>> + if ((!remoteip_addr_in_list(conf->proxy_protocol_enabled, >>>> c->local_addr) >>>> + && !optional ) >>>> + || remoteip_addr_in_list(conf->proxy_protocol_disabled, >>>> c->local_addr)) { >>>> + return DECLINED; >>>> + } >>>> + >>>> + /* mod_proxy creates outgoing connections - we don't want those */ >>>> + if (!remoteip_is_server_port(c->local_addr->port)) { >>>> + return DECLINED; >>>> + } >>> Why is the c->local_addr->port set to a port we are listening on in case of >>> proxy connections? This is from the original code, but wouldn't this be the local port on the outbound connection (some high number)? The remoteip_is_server_port should therefore fail this check. >>> >>>> + >>>> + /* add our filter */ >>>> + if (!ap_add_input_filter(remoteip_filter_name, NULL, NULL, c)) { >>>> + /* XXX: Shouldn't this WARN in log? */ >>>> + return DECLINED; >>>> + } >>>> + >>>> + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(03503) >>>> + "RemoteIPProxyProtocol: enabled on connection to >>>> %s:%hu", >>>> + c->local_ip, c->local_addr->port); >>>> + >>>> + /* this holds the resolved proxy info for this connection */ >>>> + conn_conf = apr_pcalloc(c->pool, sizeof(*conn_conf)); >>>> + >>>> + /* Propagate the optional flag so the connection handler knows not to >>>> + abort if the header is mising. NOTE: This means we must check after >>>> + we read the request that the header was NOT optional, too. >>>> + */ >>>> + conn_conf->proxy_protocol_optional = optional; >>>> + >>>> + ap_set_module_config(c->conn_config, &remoteip_module, conn_conf); >>>> + >>>> + return OK; >>>> +} >>>> + >>>> +/* Binary format: >>>> + * <sig><cmd><proto><addr-len><addr> >>>> + * sig = \x0D \x0A \x0D \x0A \x00 \x0D \x0A \x51 \x55 \x49 \x54 \x0A >>>> + * cmd = <4-bits-version><4-bits-command> >>>> + * 4-bits-version = \x02 >>>> + * 4-bits-command = {\x00|\x01} (\x00 = LOCAL: discard con info; \x01 = >>>> PROXY) >>>> + * proto = <4-bits-family><4-bits-protocol> >>>> + * 4-bits-family = {\x00|\x01|\x02|\x03} (AF_UNSPEC, AF_INET, AF_INET6, >>>> AF_UNIX) >>>> + * 4-bits-protocol = {\x00|\x01|\x02} (UNSPEC, STREAM, DGRAM) >>>> + */ >>>> +static remoteip_parse_status_t remoteip_process_v2_header(conn_rec *c, >>>> + remoteip_conn_config_t >>>> *conn_conf, >>>> + proxy_header *hdr) >>>> +{ >>>> + apr_status_t ret; >>>> + >>>> + switch (hdr->v2.ver_cmd & 0xF) { >>>> + case 0x01: /* PROXY command */ >>>> + switch (hdr->v2.fam) { >>>> + case 0x11: /* TCPv4 */ >>>> + ret = apr_sockaddr_info_get(&conn_conf->client_addr, >>>> NULL, >>>> + APR_INET, >>>> + >>>> ntohs(hdr->v2.addr.ip4.src_port), >>>> + 0, c->pool); >>>> + if (ret != APR_SUCCESS) { >>>> + conn_conf->client_addr = NULL; >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, ret, c, >>>> APLOGNO(03504) >>>> + "RemoteIPPProxyProtocol: error >>>> creating sockaddr"); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> + conn_conf->client_addr->sa.sin.sin_addr.s_addr = >>>> + hdr->v2.addr.ip4.src_addr; >>>> + break; >>>> + >>>> + case 0x21: /* TCPv6 */ >>>> +#if APR_HAVE_IPV6 >>>> + ret = apr_sockaddr_info_get(&conn_conf->client_addr, >>>> NULL, >>>> + APR_INET6, >>>> + >>>> ntohs(hdr->v2.addr.ip6.src_port), >>>> + 0, c->pool); >>>> + if (ret != APR_SUCCESS) { >>>> + conn_conf->client_addr = NULL; >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, ret, c, >>>> APLOGNO(03505) >>>> + "RemoteIPProxyProtocol: error >>>> creating sockaddr"); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> memcpy(&conn_conf->client_addr->sa.sin6.sin6_addr.s6_addr, >>>> + hdr->v2.addr.ip6.src_addr, 16); >>>> + break; >>>> +#else >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, >>>> APLOGNO(03506) >>>> + "RemoteIPProxyProtocol: APR is not >>>> compiled with IPv6 support"); >>>> + return HDR_ERROR; >>>> +#endif >>>> + default: >>>> + /* unsupported protocol, keep local connection >>>> address */ >>>> + return HDR_DONE; >>>> + } >>>> + break; /* we got a sockaddr now */ >>>> + >>>> + case 0x00: /* LOCAL command */ >>>> + /* keep local connection address for LOCAL */ >>>> + return HDR_DONE; >>>> + >>>> + default: >>>> + /* not a supported command */ >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(03507) >>>> + "RemoteIPProxyProtocol: unsupported command >>>> %.2hx", >>>> + hdr->v2.ver_cmd); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> + /* got address - compute the client_ip from it */ >>>> + ret = apr_sockaddr_ip_get(&conn_conf->client_ip, >>>> conn_conf->client_addr); >>>> + if (ret != APR_SUCCESS) { >>>> + conn_conf->client_addr = NULL; >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, ret, c, APLOGNO(03508) >>>> + "RemoteIPProxyProtocol: error converting address to >>>> string"); >>>> + return HDR_ERROR; >>>> + } >>>> + >>>> + return HDR_DONE; >>>> +} >>>> + >>>> +/** Determine if this is a v1 or v2 PROXY header. >>>> + */ >>>> +static int remoteip_determine_version(conn_rec *c, const char *ptr) >>>> +{ >>>> + proxy_header *hdr = (proxy_header *) ptr; >>>> + >>>> + /* assert len >= 14 */ >>>> + >>>> + if (memcmp(&hdr->v2, v2sig, sizeof(v2sig)) == 0 && >>>> + (hdr->v2.ver_cmd & 0xF0) == 0x20) { >>>> + return 2; >>>> + } >>>> + else if (memcmp(hdr->v1.line, "PROXY ", 6) == 0) { >>>> + return 1; >>>> + } >>>> + else { >>>> + return -1; >>>> + } >>>> +} >>>> + >>>> +/* Capture the first bytes on the protocol and parse the proxy protocol >>>> header. >>>> + * Removes itself when the header is complete. >>>> + */ >>>> +static apr_status_t remoteip_input_filter(ap_filter_t *f, >>>> + apr_bucket_brigade *bb_out, >>>> + ap_input_mode_t mode, >>>> + apr_read_type_e block, >>>> + apr_off_t readbytes) >>>> +{ >>>> + apr_status_t ret; >>>> + remoteip_filter_context *ctx = f->ctx; >>>> + remoteip_conn_config_t *conn_conf; >>>> + apr_bucket *b; >>>> + remoteip_parse_status_t psts; >>>> + const char *ptr; >>>> + apr_size_t len; >>>> + >>>> + if (f->c->aborted) { >>>> + return APR_ECONNABORTED; >>>> + } >>>> + >>>> + /* allocate/retrieve the context that holds our header */ >>>> + if (!ctx) { >>>> + ctx = f->ctx = apr_palloc(f->c->pool, sizeof(*ctx)); >>>> + ctx->rcvd = 0; >>>> + ctx->need = MIN_HDR_LEN; >>>> + ctx->version = 0; >>>> + ctx->mode = AP_MODE_READBYTES; >>>> + ctx->bb = apr_brigade_create(f->c->pool, f->c->bucket_alloc); >>>> + ctx->done = 0; >>>> + } >>>> + >>>> + if (ctx->done) { >>>> + /* Note: because we're a connection filter we can't remove >>>> ourselves >>>> + * when we're done, so we have to stay in the chain and just go >>>> into >>>> + * passthrough mode. >>>> + */ >>>> + return ap_get_brigade(f->next, bb_out, mode, block, readbytes); >>>> + } >>>> + >>>> + conn_conf = ap_get_module_config(f->c->conn_config, &remoteip_module); >>>> + >>>> + /* try to read a header's worth of data */ >>>> + while (!ctx->done) { >>>> + if (APR_BRIGADE_EMPTY(ctx->bb)) { >>>> + ret = ap_get_brigade(f->next, ctx->bb, ctx->mode, block, >>>> + ctx->need - ctx->rcvd); >>>> + if (ret != APR_SUCCESS) { >>>> + return ret; >>>> + } >>>> + } >>>> + if (APR_BRIGADE_EMPTY(ctx->bb)) { >>> What about the case of an non blocking read where the upstream filter >>> returns an empty brigade >>> and APR_SUCCESS. This is equal to returning EAGAIN. Also from the original code >>>> + return APR_EOF; >>>> + } >>>> + >>>> + while (!ctx->done && !APR_BRIGADE_EMPTY(ctx->bb)) { >>>> + b = APR_BRIGADE_FIRST(ctx->bb); >>>> + >>>> + ret = apr_bucket_read(b, &ptr, &len, block); >>>> + if (APR_STATUS_IS_EAGAIN(ret) && block == APR_NONBLOCK_READ) { >>>> + return APR_SUCCESS; >>>> + } >>>> + if (ret != APR_SUCCESS) { >>>> + return ret; >>>> + } >>>> + >>>> + memcpy(ctx->header + ctx->rcvd, ptr, len); >>>> + ctx->rcvd += len; >>>> + >>>> + /* Remove instead of delete - we may put this bucket >>>> + back into bb_out if the header was optional and we >>>> + pass down the chain */ >>>> + APR_BUCKET_REMOVE(b); Note: In the original code this was apr_bucket_delete. >>>> + psts = HDR_NEED_MORE; >>>> + >>>> + if (ctx->version == 0) { >>>> + /* reading initial chunk */ >>>> + if (ctx->rcvd >= MIN_HDR_LEN) { >>>> + ctx->version = remoteip_determine_version(f->c, >>>> ctx->header); >>>> + if (ctx->version < 0) { >>>> + psts = HDR_MISSING; >>>> + } >>>> + else if (ctx->version == 1) { >>>> + ctx->mode = AP_MODE_GETLINE; >>>> + ctx->need = sizeof(proxy_v1); >>>> + } >>>> + else if (ctx->version == 2) { >>>> + ctx->need = MIN_V2_HDR_LEN; >>>> + } >>>> + } >>>> + } >>>> + else if (ctx->version == 1) { >>>> + psts = remoteip_process_v1_header(f->c, conn_conf, >>>> + (proxy_header *) ctx->header, >>>> + ctx->rcvd, &ctx->need); >>>> + } >>>> + else if (ctx->version == 2) { >>>> + if (ctx->rcvd >= MIN_V2_HDR_LEN) { >>>> + ctx->need = MIN_V2_HDR_LEN + >>>> + ntohs(((proxy_header *) >>>> ctx->header)->v2.len); >>>> + } >>>> + if (ctx->rcvd >= ctx->need) { >>>> + psts = remoteip_process_v2_header(f->c, conn_conf, >>>> + (proxy_header *) >>>> ctx->header); >>>> + } >>>> + } >>>> + else { >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, f->c, >>>> APLOGNO(03509) >>>> + "RemoteIPProxyProtocol: internal error: >>>> unknown version " >>>> + "%d", ctx->version); >>>> + f->c->aborted = 1; >>>> + apr_bucket_delete(b); >>>> + apr_brigade_destroy(ctx->bb); >>>> + return APR_ECONNABORTED; >>>> + } >>>> + >>>> + switch (psts) { >>>> + case HDR_MISSING: >>>> + if (conn_conf->proxy_protocol_optional) { >>>> + /* Same as DONE, but don't delete the bucket. >>>> Rather, put it >>>> + back into the brigade and move the request >>>> along the stack */ >>>> + ctx->done = 1; >>>> + APR_BRIGADE_INSERT_HEAD(bb_out, b); >>> See below. We need to restore all buckets. What if the original read was >>> speculative? >>> >>>> + return ap_pass_brigade(f->next, ctx->bb); >>> Why using ap_pass_brigade on f->next? We are an input filter and f->next is >>> our upstream input filter. This is probably my own misunderstanding... what would be the preferred way to move this down the stack, then? >>> >>>> + } >>>> + else { >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, f->c, >>>> APLOGNO(03510) >>>> + "RemoteIPProxyProtocol: no valid >>>> PROXY header found"); >>>> + /* fall through to error case */ >>>> + } >>>> + case HDR_ERROR: >>>> + f->c->aborted = 1; >>>> + apr_bucket_delete(b); >>>> + apr_brigade_destroy(ctx->bb); >>>> + return APR_ECONNABORTED; >>>> + >>>> + case HDR_DONE: >>>> + apr_bucket_delete(b); >>>> + ctx->done = 1; >>>> + break; >>>> + >>>> + case HDR_NEED_MORE: >>>> + /* It is safe to delete this bucket if we get here >>>> since we know >>>> + that we are definitely processing a header (we've >>>> read enough to >>>> + know if the signatures exist on the line) */ >>> No. We might not even received MIN_HDR_LEN data so far and thus did not >>> check for the signature >>> so far. We need to safe all the buckets that we might need to restore in a >>> separate bucket brigade, >>> that we need to be set aside before doing the next ap_get_brigade to avoid >>> killing transient buckets. This is a good point. As a side note, in the original code, this delete didn't exist on this line, but was done earlier (noted above). Editorially, while I was testing a patch I was writing to do this same work with mod_ssl down stack, I was consistently getting only 11 bytes read and I would have expected that same thing to be an issue here (since MIN_HDR_LEN is 16)... but never actually observed that. I'm sure that's because ap_get_brigade on the first round through is called requesting MIN_HDR_LEN bytes (as assigned to ctx->need). Would ap_get_brigade ever turn less than what was requested? >>>> + apr_bucket_delete(b); >>>> + break; >>>> + } >>>> + } >>>> + } >>>> + >>>> + /* we only get here when done == 1 */ >>>> + if (psts == HDR_DONE) { >>>> + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c, APLOGNO(03511) >>>> + "RemoteIPProxyProtocol: received valid PROXY >>>> header: %s:%hu", >>>> + conn_conf->client_ip, conn_conf->client_addr->port); >>>> + } >>>> + else { >>>> + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c, APLOGNO(03512) >>>> + "RemoteIPProxyProtocol: PROXY header was missing"); >>>> + } >>>> + >>>> + if (ctx->rcvd > ctx->need || !APR_BRIGADE_EMPTY(ctx->bb)) { >>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, f->c, APLOGNO(03513) >>>> + "RemoteIPProxyProtocol: internal error: have data >>>> left over; " >>>> + " need=%lu, rcvd=%lu, brigade-empty=%d", ctx->need, >>>> + ctx->rcvd, APR_BRIGADE_EMPTY(ctx->bb)); >>>> + f->c->aborted = 1; >>>> + apr_brigade_destroy(ctx->bb); >>>> + return APR_ECONNABORTED; >>>> + } >>>> + >>>> + /* clean up */ >>>> + apr_brigade_destroy(ctx->bb); >>>> + ctx->bb = NULL; >>>> + >>>> + /* now do the real read for the upper layer */ >>>> + return ap_get_brigade(f->next, bb_out, mode, block, readbytes); >>>> +} >>>> + >>>> static const command_rec remoteip_cmds[] = >>>> { >>>> AP_INIT_TAKE1("RemoteIPHeader", header_name_set, NULL, RSRC_CONF, >>> Regards >>> >>> Rüdiger >>> -- Daniel Ruggeri
