stoty commented on code in PR #615: URL: https://github.com/apache/httpcomponents-client/pull/615#discussion_r1975386169
########## httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/gss/GssSchemeBase.java: ########## @@ -0,0 +1,366 @@ +/* + * ==================================================================== + * 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 + * <http://www.apache.org/>. + * + */ +package org.apache.hc.client5.http.impl.auth.gss; + +import java.net.UnknownHostException; +import java.security.Principal; + +import org.apache.hc.client5.http.DnsResolver; +import org.apache.hc.client5.http.SystemDefaultDnsResolver; +import org.apache.hc.client5.http.auth.AuthChallenge; +import org.apache.hc.client5.http.auth.AuthScheme; +import org.apache.hc.client5.http.auth.AuthScope; +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.InvalidCredentialsException; +import org.apache.hc.client5.http.auth.StandardAuthScheme; +import org.apache.hc.client5.http.auth.gss.GssConfig; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.http.utils.Base64; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.Args; +import org.ietf.jgss.GSSContext; +import org.ietf.jgss.GSSCredential; +import org.ietf.jgss.GSSException; +import org.ietf.jgss.GSSManager; +import org.ietf.jgss.GSSName; +import org.ietf.jgss.Oid; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Common behaviour for the new mutual authentication capable {@code GSS} based authentication + * schemes. + * + * This class is derived from the old {@link org.apache.hc.client5.http.impl.auth.GGSSchemeBase} + * class, which was deprecated in 5.3. + * + * @since 5.5 + * + * @see GGSSchemeBase + */ +public abstract class GssSchemeBase implements AuthScheme { + + enum State { + UNINITIATED, + TOKEN_READY, + TOKEN_SENT, + SUCCEEDED, + FAILED, + } + + private static final Logger LOG = LoggerFactory.getLogger(GssSchemeBase.class); + private static final String NO_TOKEN = ""; + private static final String KERBEROS_SCHEME = "HTTP"; + + // 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 GssConfig 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[] queuedToken = new byte[0]; + + GssSchemeBase(final GssConfig config, final DnsResolver dnsResolver) { + super(); + this.config = config != null ? config : GssConfig.DEFAULT; + this.dnsResolver = dnsResolver != null ? dnsResolver : SystemDefaultDnsResolver.INSTANCE; + this.mutualAuth = config.isRequestMutualAuth(); + this.state = State.UNINITIATED; + } + + GssSchemeBase(final GssConfig config) { + this(config, SystemDefaultDnsResolver.INSTANCE); + } + + GssSchemeBase() { + this(GssConfig.DEFAULT, SystemDefaultDnsResolver.INSTANCE); + } + + @Override + public String getRealm() { + return null; + } + + // Required by AuthScheme for backwards compatibility + @Override + public void processChallenge(final AuthChallenge authChallenge, + final HttpContext context ) { + // If this gets called, then AuthScheme was changed in an incompatible way + throw new UnsupportedOperationException(); + } + + // 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 boolean challenged, + final AuthChallenge authChallenge, + final HttpContext context + ) throws AuthenticationException { + + if (challengesLeft-- <= 0 ) { + if (LOG.isDebugEnabled()) { + final HttpClientContext clientContext = HttpClientContext.cast(context); + final String exchangeId = clientContext.getExchangeId(); + 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 gssHostname; + String hostname = host.getHostName(); + if (config.isUseCanonicalHostname()) { + try { + hostname = dnsResolver.resolveCanonicalHostname(host.getHostName()); + } catch (final UnknownHostException ignore) { + } + } + if (config.isAddPort()) { + gssHostname = hostname + ":" + host.getPort(); + } else { + gssHostname = hostname; + } + + if (LOG.isDebugEnabled()) { + final HttpClientContext clientContext = HttpClientContext.cast(context); + final String exchangeId = clientContext.getExchangeId(); + LOG.debug("{} GSS init {}", exchangeId, gssHostname); + } + try { + setGssCredential(HttpClientContext.cast(context).getCredentialsProvider(), host, context); + queuedToken = generateToken(challengeToken, KERBEROS_SCHEME, gssHostname); + switch (state) { + case UNINITIATED: + if (challenge != NO_TOKEN) { + if (LOG.isDebugEnabled()) { + final HttpClientContext clientContext = HttpClientContext.cast(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) { Review Comment: Thank you. Yes, the current code should comply with that. "ignoreMissingToken" is the non-strict flag (defaults to off), but I am open to a better name. I am not sure about the warning, that can be a really high volume, and the user has already disabled it explicitly. I am currently logging a DEBUG message for this. Currently we enforce mutual auth if requested. Requesting it unconditionally, and enforcing based on a flag also sounds good to me, I will make the change. ########## httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/gss/GssSchemeBase.java: ########## @@ -183,35 +184,54 @@ public void processChallenge( 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 (challengeToken == null) { + if (!challenged && ignoreMissingToken) { + // Got a 200 without a challenge. Old non RFC compliant server. Review Comment: Fixing -- 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
