From 50f93ec18be8d6f49138825356051c4c0b60dce4 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 13 Sep 2021 19:15:22 +0200 Subject: [PATCH] HTTPCLIENT-2045: BASIC auth scheme conformance to RFC 7617 --- .../http/impl/auth/AuthSchemeSupport.java | 50 ++++++++ .../client5/http/impl/auth/BasicScheme.java | 54 ++++++--- .../client5/http/impl/auth/DigestScheme.java | 10 +- .../http/impl/auth/TestBasicScheme.java | 112 ++++++++++++------ 4 files changed, 169 insertions(+), 57 deletions(-) create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java new file mode 100644 index 000000000..a65935154 --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java @@ -0,0 +1,50 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.hc.client5.http.impl.auth; + +import java.nio.charset.Charset; +import java.nio.charset.UnsupportedCharsetException; + +import org.apache.hc.client5.http.auth.AuthenticationException; +import org.apache.hc.core5.annotation.Internal; + +/** + * @since 5.2 + */ +@Internal +public class AuthSchemeSupport { + + public static Charset parseCharset( + final String charsetName, final Charset defaultCharset) throws AuthenticationException { + try { + return charsetName != null ? Charset.forName(charsetName) : defaultCharset; + } catch (final UnsupportedCharsetException ex) { + throw new AuthenticationException("Unsupported charset: " + charsetName); + } + } + +} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java index 438cc682c..682602975 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java @@ -42,13 +42,13 @@ import java.util.Map; import org.apache.commons.codec.binary.Base64; import org.apache.hc.client5.http.auth.AuthChallenge; import org.apache.hc.client5.http.auth.AuthScheme; -import org.apache.hc.client5.http.auth.StandardAuthScheme; import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.AuthStateCacheable; import org.apache.hc.client5.http.auth.AuthenticationException; import org.apache.hc.client5.http.auth.Credentials; import org.apache.hc.client5.http.auth.CredentialsProvider; import org.apache.hc.client5.http.auth.MalformedChallengeException; +import org.apache.hc.client5.http.auth.StandardAuthScheme; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.http.utils.ByteArrayBuilder; import org.apache.hc.core5.http.HttpHost; @@ -72,7 +72,7 @@ public class BasicScheme implements AuthScheme, Serializable { private static final Logger LOG = LoggerFactory.getLogger(BasicScheme.class); private final Map paramMap; - private transient Charset charset; + private transient Charset defaultCharset; private transient ByteArrayBuilder buffer; private transient Base64 base64codec; private boolean complete; @@ -85,7 +85,7 @@ public class BasicScheme implements AuthScheme, Serializable { */ public BasicScheme(final Charset charset) { this.paramMap = new HashMap<>(); - this.charset = charset != null ? charset : StandardCharsets.US_ASCII; + this.defaultCharset = charset != null ? charset : StandardCharsets.US_ASCII; this.complete = false; } @@ -93,13 +93,21 @@ public class BasicScheme implements AuthScheme, Serializable { this(StandardCharsets.US_ASCII); } + private void applyCredentials(final Credentials credentials) { + this.username = credentials.getUserPrincipal().getName(); + this.password = credentials.getPassword(); + } + + private void clearCredentials() { + this.username = null; + this.password = null; + } + public void initPreemptive(final Credentials credentials) { if (credentials != null) { - this.username = credentials.getUserPrincipal().getName(); - this.password = credentials.getPassword(); + applyCredentials(credentials); } else { - this.username = null; - this.password = null; + clearCredentials(); } } @@ -150,8 +158,7 @@ public class BasicScheme implements AuthScheme, Serializable { final Credentials credentials = credentialsProvider.getCredentials( authScope, context); if (credentials != null) { - this.username = credentials.getUserPrincipal().getName(); - this.password = credentials.getPassword(); + applyCredentials(credentials); return true; } @@ -160,8 +167,7 @@ public class BasicScheme implements AuthScheme, Serializable { final String exchangeId = clientContext.getExchangeId(); LOG.debug("{} No credentials found for auth scope [{}]", exchangeId, authScope); } - this.username = null; - this.password = null; + clearCredentials(); return false; } @@ -170,16 +176,34 @@ public class BasicScheme implements AuthScheme, Serializable { return null; } + private void validateUsername() throws AuthenticationException { + if (username == null) { + throw new AuthenticationException("User credentials not set"); + } + for (int i = 0; i < username.length(); i++) { + final char ch = username.charAt(i); + if (Character.isISOControl(ch)) { + throw new AuthenticationException("Username must not contain any control characters"); + } + if (ch == ':') { + throw new AuthenticationException("Username contains a colon character and is invalid"); + } + } + } + @Override public String generateAuthResponse( final HttpHost host, final HttpRequest request, final HttpContext context) throws AuthenticationException { + validateUsername(); if (this.buffer == null) { - this.buffer = new ByteArrayBuilder(64).charset(this.charset); + this.buffer = new ByteArrayBuilder(64); } else { this.buffer.reset(); } + final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), defaultCharset); + this.buffer.charset(charset); this.buffer.append(this.username).append(":").append(this.password); if (this.base64codec == null) { this.base64codec = new Base64(0); @@ -191,16 +215,16 @@ public class BasicScheme implements AuthScheme, Serializable { private void writeObject(final ObjectOutputStream out) throws IOException { out.defaultWriteObject(); - out.writeUTF(this.charset.name()); + out.writeUTF(this.defaultCharset.name()); } @SuppressWarnings("unchecked") private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException { in.defaultReadObject(); try { - this.charset = Charset.forName(in.readUTF()); + this.defaultCharset = Charset.forName(in.readUTF()); } catch (final UnsupportedCharsetException ex) { - this.charset = StandardCharsets.US_ASCII; + this.defaultCharset = StandardCharsets.US_ASCII; } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java index ab6eabd14..812eaeeec 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java @@ -32,7 +32,6 @@ import java.io.ObjectOutputStream; import java.io.Serializable; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.nio.charset.UnsupportedCharsetException; import java.security.MessageDigest; import java.security.Principal; import java.security.SecureRandom; @@ -275,14 +274,7 @@ public class DigestScheme implements AuthScheme, Serializable { throw new AuthenticationException("None of the qop methods is supported: " + qoplist); } - final String charsetName = this.paramMap.get("charset"); - final Charset charset; - try { - charset = charsetName != null ? Charset.forName(charsetName) : defaultCharset; - } catch (final UnsupportedCharsetException ex) { - throw new AuthenticationException("Unsupported charset: " + charsetName); - } - + final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), defaultCharset); String digAlg = algorithm; if (digAlg.equalsIgnoreCase("MD5-sess")) { digAlg = "MD5"; diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java index 9115aad65..de2f4ce83 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java @@ -36,9 +36,10 @@ import java.util.List; import org.apache.commons.codec.binary.Base64; import org.apache.hc.client5.http.auth.AuthChallenge; import org.apache.hc.client5.http.auth.AuthScheme; -import org.apache.hc.client5.http.auth.StandardAuthScheme; import org.apache.hc.client5.http.auth.AuthScope; +import org.apache.hc.client5.http.auth.AuthenticationException; import org.apache.hc.client5.http.auth.ChallengeType; +import org.apache.hc.client5.http.auth.StandardAuthScheme; import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; @@ -72,30 +73,9 @@ public class TestBasicScheme { Assert.assertNull(authscheme.getRealm()); } - @Test - public void testBasicAuthenticationWith88591Chars() throws Exception { - final int[] germanChars = { 0xE4, 0x2D, 0xF6, 0x2D, 0xFc }; - final StringBuilder buffer = new StringBuilder(); - for (final int germanChar : germanChars) { - buffer.append((char)germanChar); - } - - final HttpHost host = new HttpHost("somehost", 80); - final AuthScope authScope = new AuthScope(host, "some realm", null); - final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("dh", buffer.toString().toCharArray()); - final BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider(); - credentialsProvider.setCredentials(authScope, creds); - final BasicScheme authscheme = new BasicScheme(StandardCharsets.ISO_8859_1); - - Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null)); - final HttpRequest request = new BasicHttpRequest("GET", "/"); - final String authResponse = authscheme.generateAuthResponse(host, request, null); - Assert.assertEquals(StandardAuthScheme.BASIC + " ZGg65C32Lfw=", authResponse); - } - @Test public void testBasicAuthentication() throws Exception { - final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\""); + final AuthChallenge authChallenge = parse("Basic realm=\"test\""); final BasicScheme authscheme = new BasicScheme(); authscheme.processChallenge(authChallenge, null); @@ -110,7 +90,7 @@ public class TestBasicScheme { Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null)); final String authResponse = authscheme.generateAuthResponse(host, request, null); - final String expected = StandardAuthScheme.BASIC + " " + new String( + final String expected = "Basic " + new String( Base64.encodeBase64("testuser:testpass".getBytes(StandardCharsets.US_ASCII)), StandardCharsets.US_ASCII); Assert.assertEquals(expected, authResponse); @@ -119,27 +99,58 @@ public class TestBasicScheme { Assert.assertFalse(authscheme.isConnectionBased()); } + static final String TEST_UTF8_PASSWORD = "123\u00A3"; + @Test - public void testBasicProxyAuthentication() throws Exception { - final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\""); + public void testBasicAuthenticationDefaultCharsetASCII() throws Exception { + final HttpHost host = new HttpHost("somehost", 80); + final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray()); + final BasicScheme authscheme = new BasicScheme(StandardCharsets.US_ASCII); + final HttpRequest request = new BasicHttpRequest("GET", "/"); + authscheme.initPreemptive(creds); + final String authResponse = authscheme.generateAuthResponse(host, request, null); + Assert.assertEquals("Basic dGVzdDoxMjM/", authResponse); + } + + @Test + public void testBasicAuthenticationDefaultCharsetISO88591() throws Exception { + final HttpHost host = new HttpHost("somehost", 80); + final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray()); + final BasicScheme authscheme = new BasicScheme(StandardCharsets.ISO_8859_1); + final HttpRequest request = new BasicHttpRequest("GET", "/"); + authscheme.initPreemptive(creds); + final String authResponse = authscheme.generateAuthResponse(host, request, null); + Assert.assertEquals("Basic dGVzdDoxMjOj", authResponse); + } + + @Test + public void testBasicAuthenticationDefaultCharsetUTF8() throws Exception { + final HttpHost host = new HttpHost("somehost", 80); + final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray()); + final BasicScheme authscheme = new BasicScheme(StandardCharsets.UTF_8); + final HttpRequest request = new BasicHttpRequest("GET", "/"); + authscheme.initPreemptive(creds); + final String authResponse = authscheme.generateAuthResponse(host, request, null); + Assert.assertEquals("Basic dGVzdDoxMjPCow==", authResponse); + } + + @Test + public void testBasicAuthenticationWithCharset() throws Exception { + final AuthChallenge authChallenge = parse("Basic realm=\"test\", charset=\"utf-8\""); final BasicScheme authscheme = new BasicScheme(); authscheme.processChallenge(authChallenge, null); final HttpHost host = new HttpHost("somehost", 80); final AuthScope authScope = new AuthScope(host, "test", null); - final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("testuser", "testpass".toCharArray()); + final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray()); final BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider(); credentialsProvider.setCredentials(authScope, creds); final HttpRequest request = new BasicHttpRequest("GET", "/"); Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null)); final String authResponse = authscheme.generateAuthResponse(host, request, null); - - final String expected = StandardAuthScheme.BASIC + " " + new String( - Base64.encodeBase64("testuser:testpass".getBytes(StandardCharsets.US_ASCII)), - StandardCharsets.US_ASCII); - Assert.assertEquals(expected, authResponse); + Assert.assertEquals("Basic dGVzdDoxMjPCow==", authResponse); Assert.assertEquals("test", authscheme.getRealm()); Assert.assertTrue(authscheme.isChallengeComplete()); Assert.assertFalse(authscheme.isConnectionBased()); @@ -147,7 +158,7 @@ public class TestBasicScheme { @Test public void testSerialization() throws Exception { - final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\""); + final AuthChallenge authChallenge = parse("Basic realm=\"test\""); final BasicScheme basicScheme = new BasicScheme(); basicScheme.processChallenge(authChallenge, null); @@ -182,4 +193,39 @@ public class TestBasicScheme { Assert.assertEquals(basicScheme.isChallengeComplete(), authScheme.isChallengeComplete()); } + @Test + public void testBasicAuthenticationUserCredentialsMissing() throws Exception { + final BasicScheme authscheme = new BasicScheme(); + final HttpHost host = new HttpHost("somehost", 80); + final HttpRequest request = new BasicHttpRequest("GET", "/"); + Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null)); + } + + @Test + public void testBasicAuthenticationUsernameWithBlank() throws Exception { + final BasicScheme authscheme = new BasicScheme(); + final HttpHost host = new HttpHost("somehost", 80); + final HttpRequest request = new BasicHttpRequest("GET", "/"); + authscheme.initPreemptive(new UsernamePasswordCredentials("blah blah", null)); + authscheme.generateAuthResponse(host, request, null); + } + + @Test + public void testBasicAuthenticationUsernameWithTab() throws Exception { + final BasicScheme authscheme = new BasicScheme(); + final HttpHost host = new HttpHost("somehost", 80); + final HttpRequest request = new BasicHttpRequest("GET", "/"); + authscheme.initPreemptive(new UsernamePasswordCredentials("blah\tblah", null)); + Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null)); + } + + @Test + public void testBasicAuthenticationUsernameWithColon() throws Exception { + final BasicScheme authscheme = new BasicScheme(); + final HttpHost host = new HttpHost("somehost", 80); + final HttpRequest request = new BasicHttpRequest("GET", "/"); + authscheme.initPreemptive(new UsernamePasswordCredentials("blah:blah", null)); + Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null)); + } + }