michael-o commented on code in PR #615: URL: https://github.com/apache/httpcomponents-client/pull/615#discussion_r1975196166
########## 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: > we could never work with Squid (and I expect that that there are many more similarly broken servers.) I have never worked with Squid. If Squid does not complete the auth loop one has to raise this as a bug, can you do that? Just because it is broken does not mean it should not be fixed. > I lean towards not making mutual authentication optional, as the user has no way to check the status, and if it is optional, then why even bother in the first place ? Correct, request it always, it does not cause any harm. libself and Cyrus SASL do the same. > On the other hand, I lean towards adding a strict flag to the config, so that we require that the GSS negotiation completes, so that we have a standards-compliant mode. This of course is implied when mutual Auth is requested. Non strict in terms of the RFC for broken servers if you cannot fix them. == I agree on your citation of "If a flag ...'", no need to fail if mutual isn't available on the context after completion even if we set it always. Set and forget. On the second citation: Yes, if the acceptor fails to resond with a token we must fail with an expcetion here. We can relax this with an explicit flag, but I would issue a warning then this happens. Clients must know that their acceptor is broken. Compare py-requests `verify` flag set to `False`. It will bug you that certificate verification is disabled, same shall we. Do you want to check the code again for this or does it already comply from your PoV? Note: I checked libserf and Cyrus SASL's GSSAPI and GS2 mechs. Libserf always requests mutual with fire and forget, GSSAPI mech has a FIXME to check return flags on the context, GS2 mech enforces all requested flags from the completed context: * https://github.com/apache/serf/blob/d4dc7a996d1a2a34dc3aa504cf44c0658120d666/auth/auth_spnego_gss.c#L204 * https://github.com/apache/serf/blob/d4dc7a996d1a2a34dc3aa504cf44c0658120d666/auth/auth_spnego_gss.c#L210 * https://github.com/cyrusimap/cyrus-sasl/blob/b8302a8c99d7079d1b68acf18802fce25c8c8243/plugins/gs2.c#L827-L838 * https://github.com/cyrusimap/cyrus-sasl/blob/b8302a8c99d7079d1b68acf18802fce25c8c8243/plugins/gssapi.c#L2294 -- 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
