You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ni...@apache.org on 2007/07/29 10:35:58 UTC
svn commit: r560689 - in /httpd/httpd/trunk: CHANGES
modules/filters/mod_deflate.c
Author: niq
Date: Sun Jul 29 01:35:56 2007
New Revision: 560689
URL: http://svn.apache.org/viewvc?view=rev&rev=560689
Log:
Fix protocol handling in mod_deflate input filter
PR 23287
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/filters/mod_deflate.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?view=diff&rev=560689&r1=560688&r2=560689
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sun Jul 29 01:35:56 2007
@@ -2,6 +2,9 @@
Changes with Apache 2.3.0
[Remove entries to the current 2.0 and 2.2 section below, when backported]
+ *) mod_deflate: fix protocol handling in deflate input filter
+ PR 23287 [Nick Kew]
+
*) mod_ldap, mod_authnzldap: Add support for nested groups (i.e. the ability
to authorize an authenticated user via a "require ldap-group X" directive
where the user is not in group X, but is in a subgroup contained in X.
Modified: httpd/httpd/trunk/modules/filters/mod_deflate.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_deflate.c?view=diff&rev=560689&r1=560688&r2=560689
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_deflate.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_deflate.c Sun Jul 29 01:35:56 2007
@@ -665,22 +665,52 @@
return ap_get_brigade(f->next, bb, mode, block, readbytes);
}
- /* Let's see what our current Content-Encoding is.
- * If gzip is present, don't gzip again. (We could, but let's not.)
+ /* Check whether request body is gzipped.
+ *
+ * If it is, we're transforming the contents, invalidating
+ * some request headers including Content-Encoding.
+ *
+ * If not, we just remove ourself.
*/
encoding = apr_table_get(r->headers_in, "Content-Encoding");
- if (encoding) {
+ if (encoding && *encoding) {
const char *tmp = encoding;
- token = ap_get_token(r->pool, &tmp, 0);
- while (token && token[0]) {
- if (!strcasecmp(token, "gzip")) {
- found = 1;
- break;
- }
- /* Otherwise, skip token */
- tmp++;
+ /* check the usual/simple case first */
+ if (!strcasecmp(encoding, "gzip")) {
+ found = 1;
+ apr_table_unset(r->headers_in, "Content-Encoding");
+ }
+ else {
token = ap_get_token(r->pool, &tmp, 0);
+ while (token && token[0]) {
+ if (!strcasecmp(token, "gzip")) {
+ char *new_encoding = apr_pstrdup(r->pool, encoding);
+ char *ptr = ap_strstr(new_encoding, token);
+ size_t sz = 5*sizeof(char);
+ if (ptr == new_encoding) {
+ /* remove "gzip:" at start */
+ memmove(ptr, ptr+sz, sz);
+ }
+ else {
+ /* remove ":gzip" as found */
+ memmove(ptr-sizeof(char), ptr+4*sizeof(char), sz);
+ }
+ /* Silly edge-case: if there's more than one "gzip"
+ * token, this is a maybe-bug, as we remove exactly
+ * one gzip token. But it's not really our bug:
+ * two gzips should mean it's double-gzipped,
+ * shouldn't it? Insert this filter twice!
+ */
+ apr_table_setn(r->headers_in, "Content-Encoding",
+ new_encoding);
+ found = 1;
+ break;
+ }
+ /* Otherwise, skip token */
+ tmp++;
+ token = ap_get_token(r->pool, &tmp, 0);
+ }
}
}
@@ -688,6 +718,9 @@
ap_remove_input_filter(f);
return ap_get_brigade(f->next, bb, mode, block, readbytes);
}
+ apr_table_unset(r->headers_in, "Content-Length");
+ apr_table_unset(r->headers_in, "Content-MD5");
+ apr_table_unset(r->headers_in, "Content-Range");
f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
ctx->bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
mod_auth_sspi
Posted by "Simon Fuhrer (privat)" <si...@tecbiz.ch>.
hello
I extend the module mod_auth_sspi to provide some password features
(password revoked, Password must change ...). All works fine but i think
that the modul has a bug and i don't know how to resolv it. i'm not the
absolute programmer but small things are ok for me. :-) I think you can
help me.
The Module provide a negotiate method and a normal Basic auth. Within
the basic auth the Auth-name that was specified in the configuration
will not be displayed on the basic auth box on Windows Firefox or
Internet Explorer. When the Module receive a Http Header with Basic Auth
data in it but not correct Username or Password the basic Auth Pop-op
and the Authname will displayed. I check the code but all things ok.
Can you help me?
Sorry for my english.
I hope you understand me.
Cheers Simon
static int check_cleartext_auth(sspi_auth_ctx* ctx)
{
int ret;
int i_exp_err;
DWORD cbOut, cbIn, maxTokenSize;
BYTE *clientbuf, *serverbuf;
SECURITY_STATUS ss;
maxTokenSize = get_package_max_token_size(sspiModuleInfo.pkgInfo,
sspiModuleInfo.numPackages, ctx->scr->package);
serverbuf = apr_palloc(ctx->r->pool, maxTokenSize);
clientbuf = NULL;
cbOut = 0;
do {
cbIn = cbOut;
cbOut = maxTokenSize;
ss = gen_client_context(sspiModuleInfo.functable,
&ctx->scr->client_credentials, &ctx->scr->client_context,
&ctx->scr->client_ctxtexpiry, clientbuf,
&cbIn, serverbuf, &cbOut,
ctx->scr->package);
if (ss == SEC_E_OK || ss == SEC_I_CONTINUE_NEEDED || ss ==
SEC_I_COMPLETE_AND_CONTINUE) {
if (clientbuf == NULL) {
clientbuf = apr_palloc(ctx->r->pool, maxTokenSize);
}
cbIn = cbOut;
cbOut = maxTokenSize;
ss = gen_server_context(sspiModuleInfo.functable,
&ctx->scr->server_credentials, &ctx->scr->server_context,
&ctx->scr->server_ctxtexpiry,
serverbuf, &cbIn, clientbuf, &cbOut);
}
} while (ss == SEC_I_CONTINUE_NEEDED || ss ==
SEC_I_COMPLETE_AND_CONTINUE);
switch (ss) {
case SEC_E_OK:
return OK;
case SEC_E_INVALID_HANDLE:
case SEC_E_INTERNAL_ERROR:
case SEC_E_NO_AUTHENTICATING_AUTHORITY:
case SEC_E_INSUFFICIENT_MEMORY:
ap_log_rerror(APLOG_MARK, APLOG_ERR,
APR_FROM_OS_ERROR(GetLastError()), ctx->r,
"access to %s failed, reason: cannot generate context",
ctx->r->uri);
return HTTP_INTERNAL_SERVER_ERROR;
case SEC_E_INVALID_TOKEN:
case SEC_E_LOGON_DENIED:
default:
i_exp_err = GetLastError();
log_sspi_logon_denied(ctx->r, &ctx->hdr,
APR_FROM_OS_ERROR(GetLastError()));
if (SSPI_PWDEXPIRED)
{
ret = ctx->crec->sspi_pwdexpired_httpcode;
ap_log_rerror(APLOG_MARK, APLOG_ERR,
APR_FROM_OS_ERROR(GetLastError()), ctx->r,
"access to %s failed, reason: pw must change", ctx->r->uri);
return ret;
}
else
{
ret = HTTP_UNAUTHORIZED;
note_sspi_auth_failure(ctx->r);
cleanup_sspi_connection(ctx->scr);
return ret;
}
}
int get_sspi_header(sspi_auth_ctx* ctx)
{
char* scheme;
char* basicline = 0;
//const char *auth_hdr = get_authenticate_header_name(r);
const char *auth_line = apr_table_get(ctx->r->headers_in,
get_authorization_header_name(ctx->r));
/*
* If the client didn't supply an Authorization: (or
Proxy-Authorization)
* header, we need to reply 401 and supply a WWW-Authenticate
* (or Proxy-Authenticate) header indicating acceptable authentication
* schemes.
*/
if (!auth_line) {
note_sspi_auth_failure(ctx->r);
return HTTP_UNAUTHORIZED;
}
/*
* Do a quick check of the Authorization: header. If it is 'Basic',
and we're
* allowed, try a cleartext logon. Else if it isn't the selected
package
* and we're authoritative, reply 401 again.
*/
scheme = ap_getword_white(ctx->r->pool, &auth_line);
if (ctx->crec->sspi_offersspi && 0 == check_package_valid(ctx,
scheme)) {
if (0 == ctx->scr->package)
ctx->scr->package = apr_pstrdup(ctx->r->connection->pool, scheme);
return get_sspi_userpass(ctx, auth_line);
} else if (ctx->crec->sspi_offerbasic && 0 == lstrcmpi(scheme,
"Basic")) {
ctx->scr->package = ctx->crec->sspi_package_basic;
return get_basic_userpass(ctx, auth_line);
}
if (ctx->crec->sspi_authoritative) {
ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, ctx->r,
"client used wrong authentication scheme: %s for %s (needed
%s)",
ctx->scr->package, ctx->r->uri, ctx->crec->sspi_packages);
note_sspi_auth_failure(ctx->r);
return HTTP_UNAUTHORIZED;
} else {
return DECLINED;
}
return HTTP_INTERNAL_SERVER_ERROR;
}
void note_sspi_auth_failure(request_rec *r)
{
const char *auth_hdr = get_authenticate_header_name(r);
sspi_config_rec *crec = get_sspi_config_rec(r);
char* basicline = 0;
apr_table_unset(r->err_headers_out, auth_hdr);
if (crec->sspi_offerbasic)
{
basicline = apr_psprintf(r->pool, "Basic realm=TESTSTSTSTST"); //,
//ap_auth_name(r));
}
if (crec->sspi_offersspi)
{
sspi_connection_rec* scr = 0;
apr_pool_userdata_get(&scr, sspiModuleInfo.userDataKeyString,
r->connection->pool);
if (scr == 0 || scr->sspi_failing == 0)
{
char* w;
const char* package_list = crec->sspi_packages;
if (crec->sspi_offerbasic && crec->sspi_basicpreferred)
{
apr_table_addn(r->err_headers_out, auth_hdr, basicline);
basicline = 0;
}
if (package_list) while (*package_list)
{
w = ap_getword_white(r->pool, &package_list);
if (w[0])
{
apr_table_addn(r->err_headers_out, auth_hdr, w);
}
}
}
}
apr_table_addn(r->err_headers_out, auth_hdr, basicline);
if (basicline != 0)
{
apr_table_addn(r->err_headers_out, auth_hdr, basicline);
}
}
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES
modules/filters/mod_deflate.c
Posted by Nick Kew <ni...@webthing.com>.
On Sat, 04 Aug 2007 13:46:23 +0200
Ruediger Pluem <rp...@apache.org> wrote:
> > This bit is also broken - unsetting C-L here is wrong presumably
> > because the HTTP input filter no longer knows how to fetch the
> > request body. t/modules/deflate.t in the test suite triggers the
> > issue; removing that unset fixes it.
>
> I have not investigated this further, but shouldn't the request body
> run through the HTTP input filter first before it runs through the
> inflate input filter?
Yep.
> The request body may have a chunked transfer
> encoding that needs dechunking first. And the HTTP input filter only
> needs the Content-Length header the very first time as it stores its
> contents in its filter context.
A possible issue here is where a handler reading the contents
requires the content-length header and fails without it (the
old REQUEST_CHUNKED_* stuff gives handlers the OK to do that).
We can fix this by reinserting content-length (at an obvious
cost) if a handler requires it. But that's another filter.
> > Also simply removing content-range is a semantic change, so I can't
> > see how that is correct. If the filter cannot adjust content-range
> > appropriately it should either do nothing or fail, I'd say.
>
> I agree with this. See my additional mail from today.
And mine.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 08/01/2007 11:53 AM, Joe Orton wrote:
> On Sun, Jul 29, 2007 at 08:35:58AM -0000, niq@apache.org wrote:
>> Author: niq
>> Date: Sun Jul 29 01:35:56 2007
>> New Revision: 560689
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=560689
>> Log:
>> Fix protocol handling in mod_deflate input filter
>> PR 23287
> ...
>> @@ -688,6 +718,9 @@
>> ap_remove_input_filter(f);
>> return ap_get_brigade(f->next, bb, mode, block, readbytes);
>> }
>> + apr_table_unset(r->headers_in, "Content-Length");
>> + apr_table_unset(r->headers_in, "Content-MD5");
>> + apr_table_unset(r->headers_in, "Content-Range");
>>
>> f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
>> ctx->bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
>
> This bit is also broken - unsetting C-L here is wrong presumably because
> the HTTP input filter no longer knows how to fetch the request body.
> t/modules/deflate.t in the test suite triggers the issue; removing that
> unset fixes it.
I have not investigated this further, but shouldn't the request body run through
the HTTP input filter first before it runs through the inflate input filter?
The request body may have a chunked transfer encoding that needs dechunking first.
And the HTTP input filter only needs the Content-Length header the very first time
as it stores its contents in its filter context.
>
> Also simply removing content-range is a semantic change, so I can't see
> how that is correct. If the filter cannot adjust content-range
> appropriately it should either do nothing or fail, I'd say.
I agree with this. See my additional mail from today.
Regards
Rüdiger
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES
modules/filters/mod_deflate.c
Posted by Nick Kew <ni...@webthing.com>.
On Wed, 1 Aug 2007 10:53:43 +0100
Joe Orton <jo...@redhat.com> wrote:
> This bit is also broken - unsetting C-L here is wrong presumably
> because the HTTP input filter no longer knows how to fetch the
> request body. t/modules/deflate.t in the test suite triggers the
> issue; removing that unset fixes it.
It's a fair cop. That was indeed broken. The fix is to postpone
unsetting C-L until after the first get_brigade from upstream,
where it's required.
Fixed in r563133
I've also hacked up a draft for another input filter that'll
re-insert content-length if it's required - e.g. proxying to
a 1.0 server. That's potentially expensive, because it means
buffering the input in order to count it, so the filter takes
care to run only when strictly necessary. Happy to include
it in the core distribution.
> Also simply removing content-range is a semantic change, so I can't
> see how that is correct. If the filter cannot adjust content-range
> appropriately it should either do nothing or fail, I'd say.
Yep. That wants another patch, which I'll be happy to do.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c
Posted by Joe Orton <jo...@redhat.com>.
On Sun, Jul 29, 2007 at 08:35:58AM -0000, niq@apache.org wrote:
> Author: niq
> Date: Sun Jul 29 01:35:56 2007
> New Revision: 560689
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=560689
> Log:
> Fix protocol handling in mod_deflate input filter
> PR 23287
...
> @@ -688,6 +718,9 @@
> ap_remove_input_filter(f);
> return ap_get_brigade(f->next, bb, mode, block, readbytes);
> }
> + apr_table_unset(r->headers_in, "Content-Length");
> + apr_table_unset(r->headers_in, "Content-MD5");
> + apr_table_unset(r->headers_in, "Content-Range");
>
> f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
> ctx->bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
This bit is also broken - unsetting C-L here is wrong presumably because
the HTTP input filter no longer knows how to fetch the request body.
t/modules/deflate.t in the test suite triggers the issue; removing that
unset fixes it.
Also simply removing content-range is a semantic change, so I can't see
how that is correct. If the filter cannot adjust content-range
appropriately it should either do nothing or fail, I'd say.
joe
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c
Posted by André Malo <nd...@perlig.de>.
* Joe Orton wrote:
> On Sun, Jul 29, 2007 at 01:37:20PM +0200, Ruediger Pluem wrote:
> > And BTW, as the order of the encodings in Content-Encoding must be the
> > order in which they have been applied, don't we need to ensure that
> > gzip needs to be at the *end* of the content encoding string, as we
> > have to decode things in the reverse order?
>
> Yes. Also: content-encoding is also not a colon-separated list, and the
> sizeof(char) pedantics just obfuscates the code, write for real
> machines, sizeof(char) == 1.
Not only for real machines. sizeof(char) is always 1, per definitionem.
nd
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c
Posted by Joe Orton <jo...@redhat.com>.
On Sun, Jul 29, 2007 at 01:37:20PM +0200, Ruediger Pluem wrote:
> And BTW, as the order of the encodings in Content-Encoding must be the
> order in which they have been applied, don't we need to ensure that
> gzip needs to be at the *end* of the content encoding string, as we
> have to decode things in the reverse order?
Yes. Also: content-encoding is also not a colon-separated list, and the
sizeof(char) pedantics just obfuscates the code, write for real
machines, sizeof(char) == 1.
joe
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES
modules/filters/mod_deflate.c
Posted by Nick Kew <ni...@webthing.com>.
On Sun, 29 Jul 2007 13:37:20 +0200
Ruediger Pluem <rp...@apache.org> wrote:
> [as usual, catching my bugs - thanks]
> > + if (!strcasecmp(token, "gzip")) {
> > + char *new_encoding = apr_pstrdup(r->pool,
> > encoding);
> > + char *ptr = ap_strstr(new_encoding, token);
> > + size_t sz = 5*sizeof(char);
> > + if (ptr == new_encoding) {
> > + /* remove "gzip:" at start */
> > + memmove(ptr, ptr+sz, sz);
>
> Maybe I am missing the point, but if gzip is at the start why moving
> memory and not simply increasing new_encoding by 5?
Good point.
>
> > + }
> > + else {
> > + /* remove ":gzip" as found */
> > + memmove(ptr-sizeof(char),
> > ), sz);
>
> Is this correct? memmove maybe bcopy on platforms where no memmove is
> available and as I understand the man pages of memmove and bcopy it
> would only copy sz bytes from behind gzip to the old position of the
> gzip string. What if you have
Even better point. That last sz should be strlen(ptr+4*sizeof(char))+1,
to include the terminator in what's moved.
> And BTW, as the order of the encodings in Content-Encoding must be
> the order in which they have been applied, don't we need to ensure
> that gzip needs to be at the *end* of the content encoding string, as
> we have to decode things in the reverse order? Otherwise we would try
> to decode a gzip encoding when there is still another encoding on top
> of gzip.
OK, that's inherited from the existing version, both input and output
filters. I guess the moral is, we've never done more than pay
lip-service to multiple content-encodings.
I'm about to go out now, so an update will have to wait:-)
Thanks,
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r560689 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 07/29/2007 10:35 AM, wrote:
> Author: niq
> Date: Sun Jul 29 01:35:56 2007
> New Revision: 560689
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=560689
> Log:
> Fix protocol handling in mod_deflate input filter
> PR 23287
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/filters/mod_deflate.c
>
> Modified: httpd/httpd/trunk/modules/filters/mod_deflate.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_deflate.c?view=diff&rev=560689&r1=560688&r2=560689
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_deflate.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_deflate.c Sun Jul 29 01:35:56 2007
> @@ -665,22 +665,52 @@
> return ap_get_brigade(f->next, bb, mode, block, readbytes);
> }
>
> - /* Let's see what our current Content-Encoding is.
> - * If gzip is present, don't gzip again. (We could, but let's not.)
> + /* Check whether request body is gzipped.
> + *
> + * If it is, we're transforming the contents, invalidating
> + * some request headers including Content-Encoding.
> + *
> + * If not, we just remove ourself.
> */
> encoding = apr_table_get(r->headers_in, "Content-Encoding");
> - if (encoding) {
> + if (encoding && *encoding) {
> const char *tmp = encoding;
>
> - token = ap_get_token(r->pool, &tmp, 0);
> - while (token && token[0]) {
> - if (!strcasecmp(token, "gzip")) {
> - found = 1;
> - break;
> - }
> - /* Otherwise, skip token */
> - tmp++;
> + /* check the usual/simple case first */
> + if (!strcasecmp(encoding, "gzip")) {
> + found = 1;
> + apr_table_unset(r->headers_in, "Content-Encoding");
> + }
> + else {
> token = ap_get_token(r->pool, &tmp, 0);
> + while (token && token[0]) {
> + if (!strcasecmp(token, "gzip")) {
> + char *new_encoding = apr_pstrdup(r->pool, encoding);
> + char *ptr = ap_strstr(new_encoding, token);
> + size_t sz = 5*sizeof(char);
> + if (ptr == new_encoding) {
> + /* remove "gzip:" at start */
> + memmove(ptr, ptr+sz, sz);
Maybe I am missing the point, but if gzip is at the start why moving memory and not
simply increasing new_encoding by 5?
> + }
> + else {
> + /* remove ":gzip" as found */
> + memmove(ptr-sizeof(char), ptr+4*sizeof(char), sz);
Is this correct? memmove maybe bcopy on platforms where no memmove is available and as
I understand the man pages of memmove and bcopy it would only copy sz bytes from behind
gzip to the old position of the gzip string. What if you have
Content-Encoding: compress:gzip:deflate
(OK, a silly case, but still valid).
As far as I understand this would result in
compress:defl:deflate
And BTW, as the order of the encodings in Content-Encoding must be the order in which they have been
applied, don't we need to ensure that gzip needs to be at the *end* of the content encoding string, as
we have to decode things in the reverse order?
Otherwise we would try to decode a gzip encoding when there is still another encoding on top of gzip.
Regards
Rüdiger