stoty commented on code in PR #577:
URL:
https://github.com/apache/httpcomponents-client/pull/577#discussion_r1747053464
##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/GGSSchemeBase.java:
##########
@@ -60,69 +60,167 @@
*
* @since 4.2
*
- * @deprecated Do not use. The GGS based experimental authentication schemes
are no longer
- * supported. Consider using Basic or Bearer authentication with TLS instead.
*/
-@Deprecated
-public abstract class GGSSchemeBase implements AuthScheme {
+// FIXME The class name looks like a Typo. Rename in 6.0 ?
+public abstract class GGSSchemeBase implements AuthSchemeV2 {
enum State {
UNINITIATED,
- CHALLENGE_RECEIVED,
- TOKEN_GENERATED,
+ TOKEN_READY,
+ TOKEN_SENT,
+ SUCCEEDED,
FAILED,
}
private static final Logger LOG =
LoggerFactory.getLogger(GGSSchemeBase.class);
private static final String NO_TOKEN = "";
private static final String KERBEROS_SCHEME = "HTTP";
- private final org.apache.hc.client5.http.auth.KerberosConfig config;
+
+ // The GSS spec does not specify how long the conversation can be. This
should be plenty.
+ // Realistically, we get one initial token, then one maybe one more for
mutual authentication.
+ private static final int MAX_GSS_CHALLENGES = 3;
+ private final KerberosConfig config;
private final DnsResolver dnsResolver;
+ private final boolean mutualAuth;
+ private int challengesLeft = MAX_GSS_CHALLENGES;
/** Authentication process state */
private State state;
private GSSCredential gssCredential;
+ private GSSContext gssContext;
private String challenge;
- private byte[] token;
+ private byte[] queuedToken = new byte[0];
- GGSSchemeBase(final org.apache.hc.client5.http.auth.KerberosConfig config,
final DnsResolver dnsResolver) {
+ GGSSchemeBase(final KerberosConfig config, final DnsResolver dnsResolver) {
super();
- this.config = config != null ? config :
org.apache.hc.client5.http.auth.KerberosConfig.DEFAULT;
+ this.config = config != null ? config : KerberosConfig.DEFAULT;
this.dnsResolver = dnsResolver != null ? dnsResolver :
SystemDefaultDnsResolver.INSTANCE;
+ this.mutualAuth = config.getRequestMutualAuth() ==
KerberosConfig.Option.ENABLE;
this.state = State.UNINITIATED;
}
- GGSSchemeBase(final org.apache.hc.client5.http.auth.KerberosConfig config)
{
+ GGSSchemeBase(final KerberosConfig config) {
this(config, SystemDefaultDnsResolver.INSTANCE);
}
GGSSchemeBase() {
- this(org.apache.hc.client5.http.auth.KerberosConfig.DEFAULT,
SystemDefaultDnsResolver.INSTANCE);
+ this(KerberosConfig.DEFAULT, SystemDefaultDnsResolver.INSTANCE);
}
@Override
public String getRealm() {
return null;
}
+ // The AuthScheme API maps awkwardly to GSSAPI, where proccessChallange
and generateAuthResponse
+ // map to the same single method call. Hence the generated token is only
stored in this method.
@Override
public void processChallenge(
+ final HttpHost host,
final AuthChallenge authChallenge,
- final HttpContext context) throws MalformedChallengeException {
- Args.notNull(authChallenge, "AuthChallenge");
-
- this.challenge = authChallenge.getValue() != null ?
authChallenge.getValue() : NO_TOKEN;
+ final HttpContext context,
+ final boolean challenged) throws AuthenticationException {
- if (state == State.UNINITIATED) {
- token = Base64.decodeBase64(challenge.getBytes());
- state = State.CHALLENGE_RECEIVED;
- } else {
+ if (challengesLeft-- <= 0 ) {
if (LOG.isDebugEnabled()) {
final HttpClientContext clientContext =
HttpClientContext.cast(context);
final String exchangeId = clientContext.getExchangeId();
- LOG.debug("{} Authentication already attempted", exchangeId);
+ LOG.debug("{} GSS error: too many challenges received.
Infinite loop ?", exchangeId);
}
+ // TODO: Should we throw an exception ? There is a test for this
behaviour.
state = State.FAILED;
+ return;
+ }
+
+ final byte[] challengeToken = Base64.decodeBase64(authChallenge== null
? null : authChallenge.getValue());
+
+ final String authServer;
+ String hostname = host.getHostName();
+ if (config.getUseCanonicalHostname() != KerberosConfig.Option.DISABLE){
+ try {
+ hostname =
dnsResolver.resolveCanonicalHostname(host.getHostName());
+ } catch (final UnknownHostException ignore){
+ }
+ }
+ if (config.getStripPort() != KerberosConfig.Option.DISABLE) {
+ authServer = hostname;
+ } else {
+ authServer = hostname + ":" + host.getPort();
+ }
+
+ if (LOG.isDebugEnabled()) {
+ final HttpClientContext clientContext =
HttpClientContext.adapt(context);
+ final String exchangeId = clientContext.getExchangeId();
+ LOG.debug("{} GSS init {}", exchangeId, authServer);
+ }
+ try {
+ queuedToken = generateToken(challengeToken, KERBEROS_SCHEME,
authServer);
+ switch (state) {
+ case UNINITIATED:
+ if (challenge != NO_TOKEN) {
+ if (LOG.isDebugEnabled()) {
+ final HttpClientContext clientContext =
HttpClientContext.adapt(context);
+ final String exchangeId =
clientContext.getExchangeId();
+ LOG.debug("{} Internal GSS error: token received when
none was sent yet: {}", exchangeId, challengeToken);
+ }
+ // TODO Should we fail ? That would break existing tests
that send a token
+ // in the first response, which is against the RFC.
+ }
+ state = State.TOKEN_READY;
+ break;
+ case TOKEN_SENT:
+ if (challenged) {
+ state = State.TOKEN_READY;
+ } else if (mutualAuth){
+ // We should have received a valid mutualAuth token
+ if (!gssContext.isEstablished()) {
+ if (LOG.isDebugEnabled()) {
+ final HttpClientContext clientContext =
+ HttpClientContext.adapt(context);
+ final String exchangeId =
clientContext.getExchangeId();
+ LOG.debug("{} GSSContext is not established ",
exchangeId);
+ }
+ state = State.FAILED;
+ // TODO should we have specific exception(s) for these
?
+ throw new AuthenticationException(
+ "requireMutualAuth is set but GSSContext is
not established");
+ } else if (!gssContext.getMutualAuthState()) {
+ if (LOG.isDebugEnabled()) {
+ final HttpClientContext clientContext =
+ HttpClientContext.adapt(context);
+ final String exchangeId =
clientContext.getExchangeId();
+ LOG.debug("{} requireMutualAuth is set but
GSSAUthContext does not have"
+ + " mutualAuthState set", exchangeId);
+ }
+ state = State.FAILED;
+ throw new AuthenticationException(
+ "requireMutualAuth is set but GSSContext
mutualAuthState is not set");
+ } else {
+ state = State.SUCCEEDED;
+ }
+ }
+ break;
+ default:
+ state = State.FAILED;
+ throw new IllegalStateException("Illegal state: " + state);
+
+ }
+ } catch (final GSSException gsse) {
+ state = State.FAILED;
+ if (gsse.getMajor() == GSSException.DEFECTIVE_CREDENTIAL
+ || gsse.getMajor() == GSSException.CREDENTIALS_EXPIRED) {
+ throw new InvalidCredentialsException(gsse.getMessage(), gsse);
+ }
+ if (gsse.getMajor() == GSSException.NO_CRED) {
+ throw new InvalidCredentialsException(gsse.getMessage(), gsse);
+ }
+ if (gsse.getMajor() == GSSException.DEFECTIVE_TOKEN
+ || gsse.getMajor() == GSSException.DUPLICATE_TOKEN
+ || gsse.getMajor() == GSSException.OLD_TOKEN) {
+ throw new AuthenticationException(gsse.getMessage(), gsse);
+ }
+ // other error
+ throw new AuthenticationException(gsse.getMessage(), gsse);
Review Comment:
Not really.
But the Auth. execption would have the same message as the cause.
We can write some new message for the Auth. exception, I just really hate
when a library swallows the original exception instead of wrapping it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
