You are viewing a plain text version of this content. The canonical link for it is here.
Posted to rpc-dev@xml.apache.org by Adam Megacz <ad...@xwt.org> on 2002/08/14 01:56:47 UTC
patch to correct improper handling of HTTP Basic authentication
XmlRpc.java does not provide adequate support to implement HTTP Basic
Authentication. WebServer.java implements it incorrectly. This patch
fixes both problems.
If an HTTP request requires authentication, the server MUST return a
401 Unauthorized:
http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2
Without the 401, the client doesn't know which type of authentication
to use. It can't assume Basic, because that would mean sending
passwords in the clear even to Digest-capable servers (thereby
rendering digest auth useless). It can't assume Digest, because the
401 contains the nonce (which is required to perform digest
authentication).
Patch appended. This also fixes a bug whereby WebServer would return
an HTTP/1.1 response without a Content-Length or chunked encoding,
which is not allowed.
- a
------------------------------------------------------------------------------
Index: src/java/org/apache/xmlrpc/WebServer.java
===================================================================
RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/WebServer.java,v
retrieving revision 1.13
diff -u -r1.13 WebServer.java
--- src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 22:27:16 -0000 1.13
+++ src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 23:53:40 -0000
@@ -94,6 +94,7 @@
protected static final byte[] conclose = "Connection: close\r\n".getBytes();
protected static final byte[] ok = " 200 OK\r\n".getBytes();
protected static final byte[] server = "Server: Apache XML-RPC 1.0\r\n".getBytes();
+ protected static final byte[] www_authenticate = "WWW-Authenticate: Basic realm=XMLRPC\r\n".getBytes();
private static final String HTTP_11 = "HTTP/1.1";
private static final String STAR = "*";
@@ -584,29 +585,40 @@
{
ServerInputStream sin = new ServerInputStream(input,
contentLength);
- byte result[] = xmlrpc.execute(sin, user, password);
- output.write(httpversion.getBytes());
- output.write(ok);
- output.write(server);
- if (keepalive)
- {
- output.write(conkeep);
- }
- else
- {
- output.write (conclose);
+ try {
+ byte result[] = xmlrpc.execute(sin, user, password);
+ output.write(httpversion.getBytes());
+ output.write(ok);
+ output.write(server);
+ if (keepalive)
+ {
+ output.write(conkeep);
+ }
+ else
+ {
+ output.write (conclose);
+ }
+ output.write(ctype);
+ output.write(clength);
+ output.write(Integer.toString(result.length)
+ .getBytes());
+ output.write(doubleNewline);
+ output.write(result);
+ } catch (XmlRpcServer.AuthenticationRequiredException are) {
+ output.write("HTTP/1.0".getBytes());
+ output.write(" 401 Unauthorized\r\n".getBytes());
+ output.write(server);
+ output.write(www_authenticate);
+ output.write("\r\n".getBytes());
+ output.write(("Method " + method
+ + " requires a username and password").getBytes());
+ keepalive = false;
}
- output.write(ctype);
- output.write(clength);
- output.write(Integer.toString(result.length)
- .getBytes());
- output.write(doubleNewline);
- output.write(result);
output.flush();
}
else
{
- output.write(httpversion.getBytes());
+ output.write("HTTP/1.0".getBytes());
output.write(" 400 Bad Request\r\n".getBytes());
output.write(server);
output.write("\r\n".getBytes());
Index: src/java/org/apache/xmlrpc/XmlRpcServer.java
===================================================================
RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcServer.java,v
retrieving revision 1.28
diff -u -r1.28 XmlRpcServer.java
--- src/java/org/apache/xmlrpc/XmlRpcServer.java 9 Aug 2002 09:14:24 -0000 1.28
+++ src/java/org/apache/xmlrpc/XmlRpcServer.java 13 Aug 2002 23:53:40 -0000
@@ -136,6 +136,7 @@
* since this is all packed into the response.
*/
public byte[] execute(InputStream is)
+ throws AuthenticationRequiredException
{
return execute(is, null, null);
}
@@ -146,6 +147,7 @@
* use the credentials to authenticate the user.
*/
public byte[] execute(InputStream is, String user, String password)
+ throws AuthenticationRequiredException
{
Worker worker = getWorker();
byte[] retval = worker.execute(is, user, password);
@@ -202,6 +204,7 @@
* Given a request for the server, generates a response.
*/
public byte[] execute(InputStream is, String user, String password)
+ throws AuthenticationRequiredException
{
try
{
@@ -224,7 +227,7 @@
* @return
*/
private byte[] executeInternal(InputStream is, String user,
- String password)
+ String password) throws AuthenticationRequiredException
{
byte[] result;
long now = 0;
@@ -284,6 +287,7 @@
Object outParam;
if (handler instanceof AuthenticatedXmlRpcHandler)
{
+ if (user == null) throw new XmlRpcServer.AuthenticationRequiredException();
outParam =((AuthenticatedXmlRpcHandler) handler)
.execute(methodName, inParams, user, password);
}
@@ -303,6 +307,9 @@
}
catch(Exception x)
{
+ if (x instanceof XmlRpcServer.AuthenticationRequiredException)
+ throw (XmlRpcServer.AuthenticationRequiredException)x;
+
if (XmlRpc.debug)
{
x.printStackTrace();
@@ -427,6 +434,11 @@
writer.endElement("methodResponse");
}
} // end of inner class Worker
+
+ public static class AuthenticationRequiredException extends IOException {
+ AuthenticationRequiredException() { }
+ }
+
} // XmlRpcServer
/**
@@ -555,4 +567,6 @@
}
return returnValue;
}
+
}
+
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Seeing as how I seem to be the only one still doing maintainence on
this package, it will be committed when I get around to it. I'm going
to Santa Baraba this weekend, so don't hold your breath. ;)
As I mentioned in a previous message, I will definitely look at it
when I have time. Thanks again for the patch.
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Seeing as how I seem to be the only one still doing maintainence on
this package, it will be committed when I get around to it. I'm going
to Santa Baraba this weekend, so don't hold your breath. ;)
As I mentioned in a previous message, I will definitely look at it
when I have time. Thanks again for the patch.
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Uh, is this going to get committed?
The Zope/Python XML-RPC people are fixing a similar defect in their
XML-RPC implementation; it would be good if Apache followed suit.
http://lists.zope.org/pipermail/zope-dev/2002-August/017126.html
http://lists.zope.org/pipermail/zope-dev/2002-August/017143.html
- a
Adam Megacz <ad...@xwt.org> writes:
> XmlRpc.java does not provide adequate support to implement HTTP Basic
> Authentication. WebServer.java implements it incorrectly. This patch
> fixes both problems.
>
> If an HTTP request requires authentication, the server MUST return a
> 401 Unauthorized:
>
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2
>
> Without the 401, the client doesn't know which type of authentication
> to use. It can't assume Basic, because that would mean sending
> passwords in the clear even to Digest-capable servers (thereby
> rendering digest auth useless). It can't assume Digest, because the
> 401 contains the nonce (which is required to perform digest
> authentication).
>
> Patch appended. This also fixes a bug whereby WebServer would return
> an HTTP/1.1 response without a Content-Length or chunked encoding,
> which is not allowed.
>
> - a
>
> ------------------------------------------------------------------------------
>
> Index: src/java/org/apache/xmlrpc/WebServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/WebServer.java,v
> retrieving revision 1.13
> diff -u -r1.13 WebServer.java
> --- src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 22:27:16 -0000 1.13
> +++ src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 23:53:40 -0000
> @@ -94,6 +94,7 @@
> protected static final byte[] conclose = "Connection: close\r\n".getBytes();
> protected static final byte[] ok = " 200 OK\r\n".getBytes();
> protected static final byte[] server = "Server: Apache XML-RPC 1.0\r\n".getBytes();
> + protected static final byte[] www_authenticate = "WWW-Authenticate: Basic realm=XMLRPC\r\n".getBytes();
>
> private static final String HTTP_11 = "HTTP/1.1";
> private static final String STAR = "*";
> @@ -584,29 +585,40 @@
> {
> ServerInputStream sin = new ServerInputStream(input,
> contentLength);
> - byte result[] = xmlrpc.execute(sin, user, password);
> - output.write(httpversion.getBytes());
> - output.write(ok);
> - output.write(server);
> - if (keepalive)
> - {
> - output.write(conkeep);
> - }
> - else
> - {
> - output.write (conclose);
> + try {
> + byte result[] = xmlrpc.execute(sin, user, password);
> + output.write(httpversion.getBytes());
> + output.write(ok);
> + output.write(server);
> + if (keepalive)
> + {
> + output.write(conkeep);
> + }
> + else
> + {
> + output.write (conclose);
> + }
> + output.write(ctype);
> + output.write(clength);
> + output.write(Integer.toString(result.length)
> + .getBytes());
> + output.write(doubleNewline);
> + output.write(result);
> + } catch (XmlRpcServer.AuthenticationRequiredException are) {
> + output.write("HTTP/1.0".getBytes());
> + output.write(" 401 Unauthorized\r\n".getBytes());
> + output.write(server);
> + output.write(www_authenticate);
> + output.write("\r\n".getBytes());
> + output.write(("Method " + method
> + + " requires a username and password").getBytes());
> + keepalive = false;
> }
> - output.write(ctype);
> - output.write(clength);
> - output.write(Integer.toString(result.length)
> - .getBytes());
> - output.write(doubleNewline);
> - output.write(result);
> output.flush();
> }
> else
> {
> - output.write(httpversion.getBytes());
> + output.write("HTTP/1.0".getBytes());
> output.write(" 400 Bad Request\r\n".getBytes());
> output.write(server);
> output.write("\r\n".getBytes());
> Index: src/java/org/apache/xmlrpc/XmlRpcServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcServer.java,v
> retrieving revision 1.28
> diff -u -r1.28 XmlRpcServer.java
> --- src/java/org/apache/xmlrpc/XmlRpcServer.java 9 Aug 2002 09:14:24 -0000 1.28
> +++ src/java/org/apache/xmlrpc/XmlRpcServer.java 13 Aug 2002 23:53:40 -0000
> @@ -136,6 +136,7 @@
> * since this is all packed into the response.
> */
> public byte[] execute(InputStream is)
> + throws AuthenticationRequiredException
> {
> return execute(is, null, null);
> }
> @@ -146,6 +147,7 @@
> * use the credentials to authenticate the user.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> Worker worker = getWorker();
> byte[] retval = worker.execute(is, user, password);
> @@ -202,6 +204,7 @@
> * Given a request for the server, generates a response.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> try
> {
> @@ -224,7 +227,7 @@
> * @return
> */
> private byte[] executeInternal(InputStream is, String user,
> - String password)
> + String password) throws AuthenticationRequiredException
> {
> byte[] result;
> long now = 0;
> @@ -284,6 +287,7 @@
> Object outParam;
> if (handler instanceof AuthenticatedXmlRpcHandler)
> {
> + if (user == null) throw new XmlRpcServer.AuthenticationRequiredException();
> outParam =((AuthenticatedXmlRpcHandler) handler)
> .execute(methodName, inParams, user, password);
> }
> @@ -303,6 +307,9 @@
> }
> catch(Exception x)
> {
> + if (x instanceof XmlRpcServer.AuthenticationRequiredException)
> + throw (XmlRpcServer.AuthenticationRequiredException)x;
> +
> if (XmlRpc.debug)
> {
> x.printStackTrace();
> @@ -427,6 +434,11 @@
> writer.endElement("methodResponse");
> }
> } // end of inner class Worker
> +
> + public static class AuthenticationRequiredException extends IOException {
> + AuthenticationRequiredException() { }
> + }
> +
> } // XmlRpcServer
>
> /**
> @@ -555,4 +567,6 @@
> }
> return returnValue;
> }
> +
> }
> +
>
--
Sick of HTML user interfaces?
www.xwt.org
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam Megacz <ad...@megacz.com> writes:
> Daniel Rall <dl...@finemaltcoding.com> writes:
> > > The key concept here is that HTTP simply does not support the notion
> > > of "optional authentication".
>
> > HTTP does not support the notation of optional auth, but a XML-RPC
> > handler might (say, based on some configuration parameter).
>
> Er, if HTTP Basic authentication is being used, then XML-RPC *cannot*
> support optional authentication without violating the HTTP spec. If
> the username and password are XML-RPC values, then you can do whatever
> you like.
>
>
> > If it does not, were you trying to keep AuthenticatedXmlRpcHandler
> > authors from shooting themselves in the foot?
>
> Exactly. If the handler uses authentication, and user==null,
> returning a 401 is the *only* valid response. This is something most
> people aren't aware of, and are extremely likely to screw up.
Done, let me know if it matches up with how you were seeing it.
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam Megacz <ad...@megacz.com> writes:
> Daniel Rall <dl...@finemaltcoding.com> writes:
> > > The key concept here is that HTTP simply does not support the notion
> > > of "optional authentication".
>
> > HTTP does not support the notation of optional auth, but a XML-RPC
> > handler might (say, based on some configuration parameter).
>
> Er, if HTTP Basic authentication is being used, then XML-RPC *cannot*
> support optional authentication without violating the HTTP spec. If
> the username and password are XML-RPC values, then you can do whatever
> you like.
>
>
> > If it does not, were you trying to keep AuthenticatedXmlRpcHandler
> > authors from shooting themselves in the foot?
>
> Exactly. If the handler uses authentication, and user==null,
> returning a 401 is the *only* valid response. This is something most
> people aren't aware of, and are extremely likely to screw up.
Done, let me know if it matches up with how you were seeing it.
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Daniel Rall <dl...@finemaltcoding.com> writes:
> > The key concept here is that HTTP simply does not support the notion
> > of "optional authentication".
> HTTP does not support the notation of optional auth, but a XML-RPC
> handler might (say, based on some configuration parameter).
Er, if HTTP Basic authentication is being used, then XML-RPC *cannot*
support optional authentication without violating the HTTP spec. If
the username and password are XML-RPC values, then you can do whatever
you like.
> If it does not, were you trying to keep AuthenticatedXmlRpcHandler
> authors from shooting themselves in the foot?
Exactly. If the handler uses authentication, and user==null,
returning a 401 is the *only* valid response. This is something most
people aren't aware of, and are extremely likely to screw up.
- a
--
"Cassette tapes are killing the music industry"
-- RIAA spokesperson, 1978
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Daniel Rall <dl...@finemaltcoding.com> writes:
> > The key concept here is that HTTP simply does not support the notion
> > of "optional authentication".
> HTTP does not support the notation of optional auth, but a XML-RPC
> handler might (say, based on some configuration parameter).
Er, if HTTP Basic authentication is being used, then XML-RPC *cannot*
support optional authentication without violating the HTTP spec. If
the username and password are XML-RPC values, then you can do whatever
you like.
> If it does not, were you trying to keep AuthenticatedXmlRpcHandler
> authors from shooting themselves in the foot?
Exactly. If the handler uses authentication, and user==null,
returning a 401 is the *only* valid response. This is something most
people aren't aware of, and are extremely likely to screw up.
- a
--
"Cassette tapes are killing the music industry"
-- RIAA spokesperson, 1978
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam Megacz <ad...@megacz.com> writes:
> Daniel Rall <dl...@finemaltcoding.com> writes:
> > My intention was to leave the authentication step up to the handler,
> > an approach which gives more freedom to the author. However, if HTTP
> > demands a user name for Basic auth ('scuse my ignorance), we should
> > not only do as you suggest but also throw an AuthenticationFailed
> > exception if the empty string is used. Does this sound okay?
>
> That sounds fine.
>
> The key concept here is that HTTP simply does not support the notion
> of "optional authentication".
HTTP does not support the notation of optional auth, but a XML-RPC
handler might (say, based on some configuration parameter). Sorry if
I'm being dense here, but how does having the handler take care of the
authentication prevent proper HTTP basic auth? If it does not, were
you trying to keep AuthenticatedXmlRpcHandler authors from shooting
themselves in the foot?
> A resource is either authenticated or not, and the process of
> authenticating to a resource involves first deliberately sending a
> failed attempt (which is how you get stuff like the realm, authtype,
> and digest nonce) before you send an authenticated attempt.
Right.
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam Megacz <ad...@megacz.com> writes:
> Daniel Rall <dl...@finemaltcoding.com> writes:
> > My intention was to leave the authentication step up to the handler,
> > an approach which gives more freedom to the author. However, if HTTP
> > demands a user name for Basic auth ('scuse my ignorance), we should
> > not only do as you suggest but also throw an AuthenticationFailed
> > exception if the empty string is used. Does this sound okay?
>
> That sounds fine.
>
> The key concept here is that HTTP simply does not support the notion
> of "optional authentication".
HTTP does not support the notation of optional auth, but a XML-RPC
handler might (say, based on some configuration parameter). Sorry if
I'm being dense here, but how does having the handler take care of the
authentication prevent proper HTTP basic auth? If it does not, were
you trying to keep AuthenticatedXmlRpcHandler authors from shooting
themselves in the foot?
> A resource is either authenticated or not, and the process of
> authenticating to a resource involves first deliberately sending a
> failed attempt (which is how you get stuff like the realm, authtype,
> and digest nonce) before you send an authenticated attempt.
Right.
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Daniel Rall <dl...@finemaltcoding.com> writes:
> My intention was to leave the authentication step up to the handler,
> an approach which gives more freedom to the author. However, if HTTP
> demands a user name for Basic auth ('scuse my ignorance), we should
> not only do as you suggest but also throw an AuthenticationFailed
> exception if the empty string is used. Does this sound okay?
That sounds fine.
The key concept here is that HTTP simply does not support the notion
of "optional authentication".
A resource is either authenticated or not, and the process of
authenticating to a resource involves first deliberately sending a
failed attempt (which is how you get stuff like the realm, authtype,
and digest nonce) before you send an authenticated attempt.
Thanks!
- a
--
"Cassette tapes are killing the music industry"
-- RIAA spokesperson, 1978
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Daniel Rall <dl...@finemaltcoding.com> writes:
> My intention was to leave the authentication step up to the handler,
> an approach which gives more freedom to the author. However, if HTTP
> demands a user name for Basic auth ('scuse my ignorance), we should
> not only do as you suggest but also throw an AuthenticationFailed
> exception if the empty string is used. Does this sound okay?
That sounds fine.
The key concept here is that HTTP simply does not support the notion
of "optional authentication".
A resource is either authenticated or not, and the process of
authenticating to a resource involves first deliberately sending a
failed attempt (which is how you get stuff like the realm, authtype,
and digest nonce) before you send an authenticated attempt.
Thanks!
- a
--
"Cassette tapes are killing the music industry"
-- RIAA spokesperson, 1978
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam Megacz <ad...@megacz.com> writes:
> Daniel Rall <dl...@finemaltcoding.com> writes:
> > Adam, this is mostly in there now. I heavily modified your patch.
> > Please take a look when you have time and let us know what you think.
>
> Yeah, I took a look at it. You should throw an AuthenticationFailed
> excption in XmlRpc.java as soon as it is determined that (user==null
> && handler instanceof AuthenticatedXmlRpcHandler).
>
> As your code is written right now, it is a violation of HTTP for an
> AuthenticatedXmlRpcHandler to do anything other than throw an
> AuthenticationFailedException if user==null. By not automatically
> throwing the exception, the new structure encourages people to write
> broken code.
Hi Adam.
My intention was to leave the authentication step up to the handler,
an approach which gives more freedom to the author. However, if HTTP
demands a user name for Basic auth ('scuse my ignorance), we should
not only do as you suggest but also throw an AuthenticationFailed
exception if the empty string is used. Does this sound okay?
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam Megacz <ad...@megacz.com> writes:
> Daniel Rall <dl...@finemaltcoding.com> writes:
> > Adam, this is mostly in there now. I heavily modified your patch.
> > Please take a look when you have time and let us know what you think.
>
> Yeah, I took a look at it. You should throw an AuthenticationFailed
> excption in XmlRpc.java as soon as it is determined that (user==null
> && handler instanceof AuthenticatedXmlRpcHandler).
>
> As your code is written right now, it is a violation of HTTP for an
> AuthenticatedXmlRpcHandler to do anything other than throw an
> AuthenticationFailedException if user==null. By not automatically
> throwing the exception, the new structure encourages people to write
> broken code.
Hi Adam.
My intention was to leave the authentication step up to the handler,
an approach which gives more freedom to the author. However, if HTTP
demands a user name for Basic auth ('scuse my ignorance), we should
not only do as you suggest but also throw an AuthenticationFailed
exception if the empty string is used. Does this sound okay?
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Daniel Rall <dl...@finemaltcoding.com> writes:
> Adam, this is mostly in there now. I heavily modified your patch.
> Please take a look when you have time and let us know what you think.
Yeah, I took a look at it. You should throw an AuthenticationFailed
excption in XmlRpc.java as soon as it is determined that (user==null
&& handler instanceof AuthenticatedXmlRpcHandler).
As your code is written right now, it is a violation of HTTP for an
AuthenticatedXmlRpcHandler to do anything other than throw an
AuthenticationFailedException if user==null. By not automatically
throwing the exception, the new structure encourages people to write
broken code.
- a
--
"Cassette tapes are killing the music industry"
-- RIAA spokesperson, 1978
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Daniel Rall <dl...@finemaltcoding.com> writes:
> Adam, this is mostly in there now. I heavily modified your patch.
> Please take a look when you have time and let us know what you think.
Yeah, I took a look at it. You should throw an AuthenticationFailed
excption in XmlRpc.java as soon as it is determined that (user==null
&& handler instanceof AuthenticatedXmlRpcHandler).
As your code is written right now, it is a violation of HTTP for an
AuthenticatedXmlRpcHandler to do anything other than throw an
AuthenticationFailedException if user==null. By not automatically
throwing the exception, the new structure encourages people to write
broken code.
- a
--
"Cassette tapes are killing the music industry"
-- RIAA spokesperson, 1978
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam, this is mostly in there now. I heavily modified your patch.
Please take a look when you have time and let us know what you think.
- Dan
Adam Megacz <ad...@xwt.org> writes:
> XmlRpc.java does not provide adequate support to implement HTTP Basic
> Authentication. WebServer.java implements it incorrectly. This patch
> fixes both problems.
>
> If an HTTP request requires authentication, the server MUST return a
> 401 Unauthorized:
>
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2
>
> Without the 401, the client doesn't know which type of authentication
> to use. It can't assume Basic, because that would mean sending
> passwords in the clear even to Digest-capable servers (thereby
> rendering digest auth useless). It can't assume Digest, because the
> 401 contains the nonce (which is required to perform digest
> authentication).
>
> Patch appended. This also fixes a bug whereby WebServer would return
> an HTTP/1.1 response without a Content-Length or chunked encoding,
> which is not allowed.
>
> - a
>
> ------------------------------------------------------------------------------
>
> Index: src/java/org/apache/xmlrpc/WebServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/WebServer.java,v
> retrieving revision 1.13
> diff -u -r1.13 WebServer.java
> --- src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 22:27:16 -0000 1.13
> +++ src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 23:53:40 -0000
> @@ -94,6 +94,7 @@
> protected static final byte[] conclose = "Connection: close\r\n".getBytes();
> protected static final byte[] ok = " 200 OK\r\n".getBytes();
> protected static final byte[] server = "Server: Apache XML-RPC 1.0\r\n".getBytes();
> + protected static final byte[] www_authenticate = "WWW-Authenticate: Basic realm=XMLRPC\r\n".getBytes();
>
> private static final String HTTP_11 = "HTTP/1.1";
> private static final String STAR = "*";
> @@ -584,29 +585,40 @@
> {
> ServerInputStream sin = new ServerInputStream(input,
> contentLength);
> - byte result[] = xmlrpc.execute(sin, user, password);
> - output.write(httpversion.getBytes());
> - output.write(ok);
> - output.write(server);
> - if (keepalive)
> - {
> - output.write(conkeep);
> - }
> - else
> - {
> - output.write (conclose);
> + try {
> + byte result[] = xmlrpc.execute(sin, user, password);
> + output.write(httpversion.getBytes());
> + output.write(ok);
> + output.write(server);
> + if (keepalive)
> + {
> + output.write(conkeep);
> + }
> + else
> + {
> + output.write (conclose);
> + }
> + output.write(ctype);
> + output.write(clength);
> + output.write(Integer.toString(result.length)
> + .getBytes());
> + output.write(doubleNewline);
> + output.write(result);
> + } catch (XmlRpcServer.AuthenticationRequiredException are) {
> + output.write("HTTP/1.0".getBytes());
> + output.write(" 401 Unauthorized\r\n".getBytes());
> + output.write(server);
> + output.write(www_authenticate);
> + output.write("\r\n".getBytes());
> + output.write(("Method " + method
> + + " requires a username and password").getBytes());
> + keepalive = false;
> }
> - output.write(ctype);
> - output.write(clength);
> - output.write(Integer.toString(result.length)
> - .getBytes());
> - output.write(doubleNewline);
> - output.write(result);
> output.flush();
> }
> else
> {
> - output.write(httpversion.getBytes());
> + output.write("HTTP/1.0".getBytes());
> output.write(" 400 Bad Request\r\n".getBytes());
> output.write(server);
> output.write("\r\n".getBytes());
> Index: src/java/org/apache/xmlrpc/XmlRpcServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcServer.java,v
> retrieving revision 1.28
> diff -u -r1.28 XmlRpcServer.java
> --- src/java/org/apache/xmlrpc/XmlRpcServer.java 9 Aug 2002 09:14:24 -0000 1.28
> +++ src/java/org/apache/xmlrpc/XmlRpcServer.java 13 Aug 2002 23:53:40 -0000
> @@ -136,6 +136,7 @@
> * since this is all packed into the response.
> */
> public byte[] execute(InputStream is)
> + throws AuthenticationRequiredException
> {
> return execute(is, null, null);
> }
> @@ -146,6 +147,7 @@
> * use the credentials to authenticate the user.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> Worker worker = getWorker();
> byte[] retval = worker.execute(is, user, password);
> @@ -202,6 +204,7 @@
> * Given a request for the server, generates a response.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> try
> {
> @@ -224,7 +227,7 @@
> * @return
> */
> private byte[] executeInternal(InputStream is, String user,
> - String password)
> + String password) throws AuthenticationRequiredException
> {
> byte[] result;
> long now = 0;
> @@ -284,6 +287,7 @@
> Object outParam;
> if (handler instanceof AuthenticatedXmlRpcHandler)
> {
> + if (user == null) throw new XmlRpcServer.AuthenticationRequiredException();
> outParam =((AuthenticatedXmlRpcHandler) handler)
> .execute(methodName, inParams, user, password);
> }
> @@ -303,6 +307,9 @@
> }
> catch(Exception x)
> {
> + if (x instanceof XmlRpcServer.AuthenticationRequiredException)
> + throw (XmlRpcServer.AuthenticationRequiredException)x;
> +
> if (XmlRpc.debug)
> {
> x.printStackTrace();
> @@ -427,6 +434,11 @@
> writer.endElement("methodResponse");
> }
> } // end of inner class Worker
> +
> + public static class AuthenticationRequiredException extends IOException {
> + AuthenticationRequiredException() { }
> + }
> +
> } // XmlRpcServer
>
> /**
> @@ -555,4 +567,6 @@
> }
> return returnValue;
> }
> +
> }
> +
>
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Adam, this is mostly in there now. I heavily modified your patch.
Please take a look when you have time and let us know what you think.
- Dan
Adam Megacz <ad...@xwt.org> writes:
> XmlRpc.java does not provide adequate support to implement HTTP Basic
> Authentication. WebServer.java implements it incorrectly. This patch
> fixes both problems.
>
> If an HTTP request requires authentication, the server MUST return a
> 401 Unauthorized:
>
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2
>
> Without the 401, the client doesn't know which type of authentication
> to use. It can't assume Basic, because that would mean sending
> passwords in the clear even to Digest-capable servers (thereby
> rendering digest auth useless). It can't assume Digest, because the
> 401 contains the nonce (which is required to perform digest
> authentication).
>
> Patch appended. This also fixes a bug whereby WebServer would return
> an HTTP/1.1 response without a Content-Length or chunked encoding,
> which is not allowed.
>
> - a
>
> ------------------------------------------------------------------------------
>
> Index: src/java/org/apache/xmlrpc/WebServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/WebServer.java,v
> retrieving revision 1.13
> diff -u -r1.13 WebServer.java
> --- src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 22:27:16 -0000 1.13
> +++ src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 23:53:40 -0000
> @@ -94,6 +94,7 @@
> protected static final byte[] conclose = "Connection: close\r\n".getBytes();
> protected static final byte[] ok = " 200 OK\r\n".getBytes();
> protected static final byte[] server = "Server: Apache XML-RPC 1.0\r\n".getBytes();
> + protected static final byte[] www_authenticate = "WWW-Authenticate: Basic realm=XMLRPC\r\n".getBytes();
>
> private static final String HTTP_11 = "HTTP/1.1";
> private static final String STAR = "*";
> @@ -584,29 +585,40 @@
> {
> ServerInputStream sin = new ServerInputStream(input,
> contentLength);
> - byte result[] = xmlrpc.execute(sin, user, password);
> - output.write(httpversion.getBytes());
> - output.write(ok);
> - output.write(server);
> - if (keepalive)
> - {
> - output.write(conkeep);
> - }
> - else
> - {
> - output.write (conclose);
> + try {
> + byte result[] = xmlrpc.execute(sin, user, password);
> + output.write(httpversion.getBytes());
> + output.write(ok);
> + output.write(server);
> + if (keepalive)
> + {
> + output.write(conkeep);
> + }
> + else
> + {
> + output.write (conclose);
> + }
> + output.write(ctype);
> + output.write(clength);
> + output.write(Integer.toString(result.length)
> + .getBytes());
> + output.write(doubleNewline);
> + output.write(result);
> + } catch (XmlRpcServer.AuthenticationRequiredException are) {
> + output.write("HTTP/1.0".getBytes());
> + output.write(" 401 Unauthorized\r\n".getBytes());
> + output.write(server);
> + output.write(www_authenticate);
> + output.write("\r\n".getBytes());
> + output.write(("Method " + method
> + + " requires a username and password").getBytes());
> + keepalive = false;
> }
> - output.write(ctype);
> - output.write(clength);
> - output.write(Integer.toString(result.length)
> - .getBytes());
> - output.write(doubleNewline);
> - output.write(result);
> output.flush();
> }
> else
> {
> - output.write(httpversion.getBytes());
> + output.write("HTTP/1.0".getBytes());
> output.write(" 400 Bad Request\r\n".getBytes());
> output.write(server);
> output.write("\r\n".getBytes());
> Index: src/java/org/apache/xmlrpc/XmlRpcServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcServer.java,v
> retrieving revision 1.28
> diff -u -r1.28 XmlRpcServer.java
> --- src/java/org/apache/xmlrpc/XmlRpcServer.java 9 Aug 2002 09:14:24 -0000 1.28
> +++ src/java/org/apache/xmlrpc/XmlRpcServer.java 13 Aug 2002 23:53:40 -0000
> @@ -136,6 +136,7 @@
> * since this is all packed into the response.
> */
> public byte[] execute(InputStream is)
> + throws AuthenticationRequiredException
> {
> return execute(is, null, null);
> }
> @@ -146,6 +147,7 @@
> * use the credentials to authenticate the user.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> Worker worker = getWorker();
> byte[] retval = worker.execute(is, user, password);
> @@ -202,6 +204,7 @@
> * Given a request for the server, generates a response.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> try
> {
> @@ -224,7 +227,7 @@
> * @return
> */
> private byte[] executeInternal(InputStream is, String user,
> - String password)
> + String password) throws AuthenticationRequiredException
> {
> byte[] result;
> long now = 0;
> @@ -284,6 +287,7 @@
> Object outParam;
> if (handler instanceof AuthenticatedXmlRpcHandler)
> {
> + if (user == null) throw new XmlRpcServer.AuthenticationRequiredException();
> outParam =((AuthenticatedXmlRpcHandler) handler)
> .execute(methodName, inParams, user, password);
> }
> @@ -303,6 +307,9 @@
> }
> catch(Exception x)
> {
> + if (x instanceof XmlRpcServer.AuthenticationRequiredException)
> + throw (XmlRpcServer.AuthenticationRequiredException)x;
> +
> if (XmlRpc.debug)
> {
> x.printStackTrace();
> @@ -427,6 +434,11 @@
> writer.endElement("methodResponse");
> }
> } // end of inner class Worker
> +
> + public static class AuthenticationRequiredException extends IOException {
> + AuthenticationRequiredException() { }
> + }
> +
> } // XmlRpcServer
>
> /**
> @@ -555,4 +567,6 @@
> }
> return returnValue;
> }
> +
> }
> +
>
--
Daniel Rall <dl...@finemaltcoding.com>
Re: patch to correct improper handling of HTTP Basic authentication
Posted by Adam Megacz <ad...@megacz.com>.
Uh, is this going to get committed?
The Zope/Python XML-RPC people are fixing a similar defect in their
XML-RPC implementation; it would be good if Apache followed suit.
http://lists.zope.org/pipermail/zope-dev/2002-August/017126.html
http://lists.zope.org/pipermail/zope-dev/2002-August/017143.html
- a
Adam Megacz <ad...@xwt.org> writes:
> XmlRpc.java does not provide adequate support to implement HTTP Basic
> Authentication. WebServer.java implements it incorrectly. This patch
> fixes both problems.
>
> If an HTTP request requires authentication, the server MUST return a
> 401 Unauthorized:
>
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2
>
> Without the 401, the client doesn't know which type of authentication
> to use. It can't assume Basic, because that would mean sending
> passwords in the clear even to Digest-capable servers (thereby
> rendering digest auth useless). It can't assume Digest, because the
> 401 contains the nonce (which is required to perform digest
> authentication).
>
> Patch appended. This also fixes a bug whereby WebServer would return
> an HTTP/1.1 response without a Content-Length or chunked encoding,
> which is not allowed.
>
> - a
>
> ------------------------------------------------------------------------------
>
> Index: src/java/org/apache/xmlrpc/WebServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/WebServer.java,v
> retrieving revision 1.13
> diff -u -r1.13 WebServer.java
> --- src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 22:27:16 -0000 1.13
> +++ src/java/org/apache/xmlrpc/WebServer.java 13 Aug 2002 23:53:40 -0000
> @@ -94,6 +94,7 @@
> protected static final byte[] conclose = "Connection: close\r\n".getBytes();
> protected static final byte[] ok = " 200 OK\r\n".getBytes();
> protected static final byte[] server = "Server: Apache XML-RPC 1.0\r\n".getBytes();
> + protected static final byte[] www_authenticate = "WWW-Authenticate: Basic realm=XMLRPC\r\n".getBytes();
>
> private static final String HTTP_11 = "HTTP/1.1";
> private static final String STAR = "*";
> @@ -584,29 +585,40 @@
> {
> ServerInputStream sin = new ServerInputStream(input,
> contentLength);
> - byte result[] = xmlrpc.execute(sin, user, password);
> - output.write(httpversion.getBytes());
> - output.write(ok);
> - output.write(server);
> - if (keepalive)
> - {
> - output.write(conkeep);
> - }
> - else
> - {
> - output.write (conclose);
> + try {
> + byte result[] = xmlrpc.execute(sin, user, password);
> + output.write(httpversion.getBytes());
> + output.write(ok);
> + output.write(server);
> + if (keepalive)
> + {
> + output.write(conkeep);
> + }
> + else
> + {
> + output.write (conclose);
> + }
> + output.write(ctype);
> + output.write(clength);
> + output.write(Integer.toString(result.length)
> + .getBytes());
> + output.write(doubleNewline);
> + output.write(result);
> + } catch (XmlRpcServer.AuthenticationRequiredException are) {
> + output.write("HTTP/1.0".getBytes());
> + output.write(" 401 Unauthorized\r\n".getBytes());
> + output.write(server);
> + output.write(www_authenticate);
> + output.write("\r\n".getBytes());
> + output.write(("Method " + method
> + + " requires a username and password").getBytes());
> + keepalive = false;
> }
> - output.write(ctype);
> - output.write(clength);
> - output.write(Integer.toString(result.length)
> - .getBytes());
> - output.write(doubleNewline);
> - output.write(result);
> output.flush();
> }
> else
> {
> - output.write(httpversion.getBytes());
> + output.write("HTTP/1.0".getBytes());
> output.write(" 400 Bad Request\r\n".getBytes());
> output.write(server);
> output.write("\r\n".getBytes());
> Index: src/java/org/apache/xmlrpc/XmlRpcServer.java
> ===================================================================
> RCS file: /home/cvspublic/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcServer.java,v
> retrieving revision 1.28
> diff -u -r1.28 XmlRpcServer.java
> --- src/java/org/apache/xmlrpc/XmlRpcServer.java 9 Aug 2002 09:14:24 -0000 1.28
> +++ src/java/org/apache/xmlrpc/XmlRpcServer.java 13 Aug 2002 23:53:40 -0000
> @@ -136,6 +136,7 @@
> * since this is all packed into the response.
> */
> public byte[] execute(InputStream is)
> + throws AuthenticationRequiredException
> {
> return execute(is, null, null);
> }
> @@ -146,6 +147,7 @@
> * use the credentials to authenticate the user.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> Worker worker = getWorker();
> byte[] retval = worker.execute(is, user, password);
> @@ -202,6 +204,7 @@
> * Given a request for the server, generates a response.
> */
> public byte[] execute(InputStream is, String user, String password)
> + throws AuthenticationRequiredException
> {
> try
> {
> @@ -224,7 +227,7 @@
> * @return
> */
> private byte[] executeInternal(InputStream is, String user,
> - String password)
> + String password) throws AuthenticationRequiredException
> {
> byte[] result;
> long now = 0;
> @@ -284,6 +287,7 @@
> Object outParam;
> if (handler instanceof AuthenticatedXmlRpcHandler)
> {
> + if (user == null) throw new XmlRpcServer.AuthenticationRequiredException();
> outParam =((AuthenticatedXmlRpcHandler) handler)
> .execute(methodName, inParams, user, password);
> }
> @@ -303,6 +307,9 @@
> }
> catch(Exception x)
> {
> + if (x instanceof XmlRpcServer.AuthenticationRequiredException)
> + throw (XmlRpcServer.AuthenticationRequiredException)x;
> +
> if (XmlRpc.debug)
> {
> x.printStackTrace();
> @@ -427,6 +434,11 @@
> writer.endElement("methodResponse");
> }
> } // end of inner class Worker
> +
> + public static class AuthenticationRequiredException extends IOException {
> + AuthenticationRequiredException() { }
> + }
> +
> } // XmlRpcServer
>
> /**
> @@ -555,4 +567,6 @@
> }
> return returnValue;
> }
> +
> }
> +
>
--
Sick of HTML user interfaces?
www.xwt.org