You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2010/05/12 20:33:17 UTC
svn commit: r943620 - in /httpcomponents/httpclient/trunk: ./
httpclient-cache/src/test/java/org/apache/http/client/cache/impl/
httpclient/src/main/java/org/apache/http/impl/auth/
httpclient/src/test/java/org/apache/http/impl/auth/
Author: olegk
Date: Wed May 12 18:33:17 2010
New Revision: 943620
URL: http://svn.apache.org/viewvc?rev=943620&view=rev
Log:
HTTPCLIENT-936: Fixed bug causing NPE or an infinite loop in the authentication code in case of a SPNEGO authentication failure
Modified:
httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/HttpTestUtils.java
httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/TestCacheEntry.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/AuthSchemeBase.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NTLMScheme.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NegotiateScheme.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/auth/TestNegotiateScheme.java
Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=943620&r1=943619&r2=943620&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Wed May 12 18:33:17 2010
@@ -22,6 +22,10 @@ maintained as of 4.1 GA release.
Changelog
-------------------
+* [HTTPCLIENT-936] Fixed bug causing NPE or an infinite loop in
+ the authentication code in case of a SPNEGO authentication failure.
+ Contributed by Oleg Kalnichevski <olegk at apache.org>
+
* [HTTPCLIENT-427] HTTP caching support
Contributed by Joe Campbell, David Cleaver, David Mays, Jon Moore, Brad Spenla
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/HttpTestUtils.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/HttpTestUtils.java?rev=943620&r1=943619&r2=943620&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/HttpTestUtils.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/HttpTestUtils.java Wed May 12 18:33:17 2010
@@ -79,6 +79,17 @@ public class HttpTestUtils {
}
/*
+ * Determines whether a given header name may appear multiple times.
+ */
+ public static boolean isMultiHeader(String name) {
+ for (String s : MULTI_HEADERS) {
+ if (s.equalsIgnoreCase(name))
+ return true;
+ }
+ return false;
+ }
+
+ /*
* Determines whether a given header name may only appear once in a message.
*/
public static boolean isSingleHeader(String name) {
@@ -88,7 +99,6 @@ public class HttpTestUtils {
}
return false;
}
-
/*
* Assert.asserts that two request or response bodies are byte-equivalent.
*/
Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/TestCacheEntry.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/TestCacheEntry.java?rev=943620&r1=943619&r2=943620&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/TestCacheEntry.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/impl/TestCacheEntry.java Wed May 12 18:33:17 2010
@@ -26,8 +26,6 @@
*/
package org.apache.http.client.cache.impl;
-import static junit.framework.Assert.assertFalse;
-
import java.util.Date;
import java.util.Set;
@@ -376,8 +374,7 @@ public class TestCacheEntry {
new BasicHeader("Cache-Control", "public") };
CacheEntry entry = getEntry(headers);
-
- assertFalse(entry.isRevalidatable());
+ Assert.assertFalse(entry.isRevalidatable());
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/AuthSchemeBase.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/AuthSchemeBase.java?rev=943620&r1=943619&r2=943620&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/AuthSchemeBase.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/AuthSchemeBase.java Wed May 12 18:33:17 2010
@@ -125,7 +125,7 @@ public abstract class AuthSchemeBase imp
}
protected abstract void parseChallenge(
- CharArrayBuffer buffer, int pos, int len) throws MalformedChallengeException;
+ CharArrayBuffer buffer, int beginIndex, int endIndex) throws MalformedChallengeException;
/**
* Returns <code>true</code> if authenticating against a proxy, <code>false</code>
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NTLMScheme.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NTLMScheme.java?rev=943620&r1=943619&r2=943620&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NTLMScheme.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NTLMScheme.java Wed May 12 18:33:17 2010
@@ -99,8 +99,9 @@ public class NTLMScheme extends AuthSche
@Override
protected void parseChallenge(
- final CharArrayBuffer buffer, int pos, int len) throws MalformedChallengeException {
- String challenge = buffer.substringTrimmed(pos, len);
+ final CharArrayBuffer buffer,
+ int beginIndex, int endIndex) throws MalformedChallengeException {
+ String challenge = buffer.substringTrimmed(beginIndex, endIndex);
if (challenge.length() == 0) {
if (this.state == State.UNINITIATED) {
this.state = State.CHALLENGE_RECEIVED;
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NegotiateScheme.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NegotiateScheme.java?rev=943620&r1=943619&r2=943620&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NegotiateScheme.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/auth/NegotiateScheme.java Wed May 12 18:33:17 2010
@@ -33,15 +33,14 @@ import org.apache.commons.logging.LogFac
import org.apache.http.Header;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
-import org.apache.http.auth.AUTH;
import org.apache.http.auth.AuthenticationException;
-import org.apache.http.auth.ContextAwareAuthScheme;
import org.apache.http.auth.Credentials;
import org.apache.http.auth.InvalidCredentialsException;
import org.apache.http.auth.MalformedChallengeException;
import org.apache.http.message.BasicHeader;
import org.apache.http.protocol.ExecutionContext;
import org.apache.http.protocol.HttpContext;
+import org.apache.http.util.CharArrayBuffer;
import org.ietf.jgss.GSSContext;
import org.ietf.jgss.GSSException;
import org.ietf.jgss.GSSManager;
@@ -54,15 +53,17 @@ import org.ietf.jgss.Oid;
*
* @since 4.1
*/
-public class NegotiateScheme implements ContextAwareAuthScheme {
+public class NegotiateScheme extends AuthSchemeBase {
- private static final int UNINITIATED = 0;
- private static final int INITIATED = 1;
- private static final int NEGOTIATING = 3;
- private static final int ESTABLISHED = 4;
- private static final int FAILED = Integer.MAX_VALUE;
- private static final String SPNEGO_OID = "1.3.6.1.5.5.2";
- private static final String KERBEROS_OID = "1.2.840.113554.1.2.2";
+ enum State {
+ UNINITIATED,
+ CHALLENGE_RECEIVED,
+ TOKEN_GENERATED,
+ FAILED,
+ }
+
+ private static final String SPNEGO_OID = "1.3.6.1.5.5.2";
+ private static final String KERBEROS_OID = "1.2.840.113554.1.2.2";
private final Log log = LogFactory.getLog(getClass());
@@ -70,12 +71,10 @@ public class NegotiateScheme implements
private final boolean stripPort;
- private boolean proxy;
-
private GSSContext gssContext = null;
/** Authentication process state */
- private int state;
+ private State state;
/** base64 decoded challenge **/
private byte[] token;
@@ -88,7 +87,7 @@ public class NegotiateScheme implements
*/
public NegotiateScheme(final SpnegoTokenGenerator spengoGenerator, boolean stripPort) {
super();
- this.state = UNINITIATED;
+ this.state = State.UNINITIATED;
this.spengoGenerator = spengoGenerator;
this.stripPort = stripPort;
}
@@ -109,7 +108,7 @@ public class NegotiateScheme implements
*
*/
public boolean isComplete() {
- return this.state == ESTABLISHED || this.state == FAILED;
+ return this.state == State.TOKEN_GENERATED || this.state == State.FAILED;
}
/**
@@ -152,7 +151,7 @@ public class NegotiateScheme implements
if (request == null) {
throw new IllegalArgumentException("HTTP request may not be null");
}
- if (state == UNINITIATED) {
+ if (state != State.CHALLENGE_RECEIVED) {
throw new IllegalStateException(
"Negotiation authentication process has not been initiated");
}
@@ -227,19 +226,13 @@ public class NegotiateScheme implements
gssContext.requestMutualAuth(true);
gssContext.requestCredDeleg(true);
}
- state = INITIATED;
-
if (token == null) {
token = new byte[0];
}
- // HTTP 1.1 issue:
- // Mutual auth will never complete to do 200 instead of 401 in
- // return from server. "state" will never reach ESTABLISHED
- // but it works anyway
-
token = gssContext.initSecContext(token, 0, token.length);
if (token == null) {
- throw new AuthenticationException("Failed to initialize security context");
+ state = State.FAILED;
+ throw new AuthenticationException("GSS security context initialization failed");
}
/*
@@ -250,11 +243,14 @@ public class NegotiateScheme implements
token = spengoGenerator.generateSpnegoDERObject(token);
}
+ state = State.TOKEN_GENERATED;
+ String tokenstr = new String(Base64.encodeBase64(token, false));
if (log.isDebugEnabled()) {
- log.debug("got token, sending " + token.length + " bytes to server");
+ log.debug("Sending response '" + tokenstr + "' back to the auth server");
}
+ return new BasicHeader("Authorization", "Negotiate " + tokenstr);
} catch (GSSException gsse) {
- state = FAILED;
+ state = State.FAILED;
if (gsse.getMajor() == GSSException.DEFECTIVE_CREDENTIAL
|| gsse.getMajor() == GSSException.CREDENTIALS_EXPIRED)
throw new InvalidCredentialsException(gsse.getMessage(), gsse);
@@ -267,11 +263,9 @@ public class NegotiateScheme implements
// other error
throw new AuthenticationException(gsse.getMessage());
} catch (IOException ex){
- state = FAILED;
+ state = State.FAILED;
throw new AuthenticationException(ex.getMessage());
}
- return new BasicHeader("Authorization", "Negotiate " +
- new String(Base64.encodeBase64(token, false)) );
}
@@ -312,41 +306,21 @@ public class NegotiateScheme implements
return true;
}
- /**
- * Processes the Negotiate challenge.
- *
- */
- public void processChallenge(final Header header) throws MalformedChallengeException {
+ @Override
+ protected void parseChallenge(
+ final CharArrayBuffer buffer,
+ int beginIndex, int endIndex) throws MalformedChallengeException {
+ String challenge = buffer.substringTrimmed(beginIndex, endIndex);
if (log.isDebugEnabled()) {
- log.debug("Challenge header: " + header);
+ log.debug("Received challenge '" + challenge + "' from the auth server");
}
- String authheader = header.getName();
- String challenge = header.getValue();
- if (authheader.equalsIgnoreCase(AUTH.WWW_AUTH)) {
- this.proxy = false;
- } else if (authheader.equalsIgnoreCase(AUTH.PROXY_AUTH)) {
- this.proxy = true;
+ if (state == State.UNINITIATED) {
+ token = new Base64().decode(challenge.getBytes());
+ state = State.CHALLENGE_RECEIVED;
} else {
- throw new MalformedChallengeException("Unexpected header name: " + authheader);
- }
-
- if (challenge.startsWith("Negotiate")) {
- if(isComplete() == false)
- state = NEGOTIATING;
-
- if (challenge.startsWith("Negotiate ")){
- token = new Base64().decode(challenge.substring(10).getBytes());
- if (log.isDebugEnabled()) {
- log.debug("challenge = " + challenge.substring(10));
- }
- } else {
- token = new byte[0];
- }
+ log.debug("Authentication already attempted");
+ state = State.FAILED;
}
}
-
- public boolean isProxy() {
- return this.proxy;
- }
-
+
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/auth/TestNegotiateScheme.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/auth/TestNegotiateScheme.java?rev=943620&r1=943619&r2=943620&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/auth/TestNegotiateScheme.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/auth/TestNegotiateScheme.java Wed May 12 18:33:17 2010
@@ -26,17 +26,17 @@
*/
package org.apache.http.impl.auth;
-import static org.mockito.Matchers.*;
-import static org.mockito.Mockito.*;
-
import java.io.IOException;
import java.security.Principal;
+import junit.framework.Assert;
+
import org.apache.http.HttpEntity;
import org.apache.http.HttpException;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
import org.apache.http.auth.AuthScheme;
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.Credentials;
@@ -44,8 +44,6 @@ import org.apache.http.client.methods.Ht
import org.apache.http.client.params.AuthPolicy;
import org.apache.http.client.params.ClientPNames;
import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.auth.NegotiateScheme;
-import org.apache.http.impl.auth.NegotiateSchemeFactory;
import org.apache.http.impl.client.DefaultHttpClient;
import org.apache.http.localserver.BasicServerTestBase;
import org.apache.http.localserver.LocalTestServer;
@@ -59,8 +57,9 @@ import org.ietf.jgss.GSSManager;
import org.ietf.jgss.GSSName;
import org.ietf.jgss.Oid;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
+import org.mockito.Matchers;
+import org.mockito.Mockito;
/**
* Tests for {@link NegotiateScheme}.
@@ -79,13 +78,15 @@ public class TestNegotiateScheme extends
* This service will continue to ask for authentication.
*/
private static class PleaseNegotiateService implements HttpRequestHandler {
- public void handle(final HttpRequest request,
+
+ public void handle(
+ final HttpRequest request,
final HttpResponse response,
final HttpContext context) throws HttpException, IOException {
- response.setStatusCode(401);
+ response.setStatusCode(HttpStatus.SC_UNAUTHORIZED);
response.addHeader(new BasicHeader("WWW-Authenticate", "Negotiate blablabla"));
- response.setEntity(new StringEntity("auth required "));
response.addHeader(new BasicHeader("Connection", "Keep-Alive"));
+ response.setEntity(new StringEntity("auth required "));
}
}
@@ -96,29 +97,34 @@ public class TestNegotiateScheme extends
*
*/
private static class NegotiateSchemeWithMockGssManager extends NegotiateScheme {
- GSSManager manager = mock(GSSManager.class);
- GSSName name = mock(GSSName.class);
- GSSContext context = mock(GSSContext.class);
+
+ GSSManager manager = Mockito.mock(GSSManager.class);
+ GSSName name = Mockito.mock(GSSName.class);
+ GSSContext context = Mockito.mock(GSSContext.class);
NegotiateSchemeWithMockGssManager() throws Exception {
super(null, true);
-
- when(context.initSecContext(any(byte[].class), anyInt(), anyInt()))
- .thenReturn("12345678".getBytes());
- when(manager.createName(any(String.class), any(Oid.class)))
- .thenReturn(name);
- when(manager.createContext(any(GSSName.class), any(Oid.class), any(GSSCredential.class), anyInt()))
- .thenReturn(context);
-
+ Mockito.when(context.initSecContext(
+ Matchers.any(byte[].class), Matchers.anyInt(), Matchers.anyInt()))
+ .thenReturn("12345678".getBytes());
+ Mockito.when(manager.createName(
+ Matchers.any(String.class), Matchers.any(Oid.class)))
+ .thenReturn(name);
+ Mockito.when(manager.createContext(
+ Matchers.any(GSSName.class), Matchers.any(Oid.class),
+ Matchers.any(GSSCredential.class), Matchers.anyInt()))
+ .thenReturn(context);
}
@Override
protected GSSManager getManager() {
return manager;
}
+
}
private static class UseJaasCredentials implements Credentials {
+
public String getPassword() {
return null;
}
@@ -126,17 +132,22 @@ public class TestNegotiateScheme extends
public Principal getUserPrincipal() {
return null;
}
+
}
private static class NegotiateSchemeFactoryWithMockGssManager extends NegotiateSchemeFactory {
+
NegotiateSchemeWithMockGssManager scheme;
+
NegotiateSchemeFactoryWithMockGssManager() throws Exception {
scheme = new NegotiateSchemeWithMockGssManager();
}
+
@Override
public AuthScheme newInstance(HttpParams params) {
return scheme;
}
+
}
/**
@@ -144,7 +155,6 @@ public class TestNegotiateScheme extends
* the server still keep asking for a valid ticket.
*/
@Test
- @Ignore
public void testDontTryToAuthenticateEndlessly() throws Exception {
int port = this.localServer.getServiceAddress().getPort();
this.localServer.register("*", new PleaseNegotiateService());
@@ -163,25 +173,26 @@ public class TestNegotiateScheme extends
HttpGet httpget = new HttpGet(s);
HttpResponse response = client.execute(httpget);
HttpEntity e = response.getEntity();
- e.consumeContent();
+ if (e != null) {
+ e.consumeContent();
+ }
+ Assert.assertEquals(HttpStatus.SC_UNAUTHORIZED, response.getStatusLine().getStatusCode());
}
-
/**
* Javadoc specifies that {@link GSSContext#initSecContext(byte[], int, int)} can return null
* if no token is generated. Client should be able to deal with this response.
- *
*/
@Test
- @Ignore
- public void testNoTokenGeneratedGenerateAnError() throws Exception {
+ public void testNoTokenGeneratedError() throws Exception {
int port = this.localServer.getServiceAddress().getPort();
this.localServer.register("*", new PleaseNegotiateService());
HttpHost target = new HttpHost("localhost", port);
DefaultHttpClient client = new DefaultHttpClient();
NegotiateSchemeFactoryWithMockGssManager nsf = new NegotiateSchemeFactoryWithMockGssManager();
- when(nsf.scheme.context.initSecContext(any(byte[].class), anyInt(), anyInt())).thenReturn(null);
+ Mockito.when(nsf.scheme.context.initSecContext(
+ Matchers.any(byte[].class), Matchers.anyInt(), Matchers.anyInt())).thenReturn(null);
client.getAuthSchemes().register(AuthPolicy.SPNEGO, nsf);
Credentials use_jaas_creds = new UseJaasCredentials();
@@ -193,7 +204,10 @@ public class TestNegotiateScheme extends
HttpGet httpget = new HttpGet(s);
HttpResponse response = client.execute(httpget);
HttpEntity e = response.getEntity();
- e.consumeContent();
+ if (e != null) {
+ e.consumeContent();
+ }
+ Assert.assertEquals(HttpStatus.SC_UNAUTHORIZED, response.getStatusLine().getStatusCode());
}
}