jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.82k stars 1.91k forks source link

SNI missing in TLS handshake #11534

Open souravgupta6nov1992 opened 5 months ago

souravgupta6nov1992 commented 5 months ago

Jetty version(s) 11.0.17 Java version/vendor (use: java -version) 17.0.8 Spring boot version(s) 2.7.18 OS type/version Linux

We are using jetty secure client over http/2 We had a requirement where we have the ips for the fqdn we want to make connections to already resolved with us but that information is very dynamic in nature and can change at any time But in the uri in request, we can only pass the fqdn e.g. fqdn1 resolves to ip1 and ip2 This information is very dynamic in nature and we do not want the DNS resolution to happen We were able to successfully achieve this by overriding jetty httpclient with some custom logic:

    this.httpClient = new HttpClient(transport) {
        @Override
        public Origin createOrigin(HttpRequest request, Origin.Protocol protocol)
        {
                String scheme = request.getScheme();
                if (!HttpScheme.HTTP.is(scheme) && !HttpScheme.HTTPS.is(scheme) &&
                        !HttpScheme.WS.is(scheme) && !HttpScheme.WSS.is(scheme))
                    throw new IllegalArgumentException("Invalid protocol " + scheme);
                scheme = scheme.toLowerCase(Locale.ENGLISH);
                String host = request.getHost();
                host = host.toLowerCase(Locale.ENGLISH);
                /**
                 * overriding the implementation from jetty client- start
                 * host will be overriden with ip from cookies if available
                 * we set the ip in cookies while sending the request
                 */
                List<HttpCookie> cookies = request.getCookies();
                logger.debug("cookies found in request: {}",cookies);
                String ip = getIpFromCookies(cookies);
                if(StringUtils.isNotBlank(ip)) {
                    host = ip;
                }
                /**
                 * Overriding the implementation from jetty client- end
                 */
                int port = request.getPort();
                port = normalizePort(scheme, port);
                logger.debug("SSL Client: Origin formed on host: {} port: {}", host, port);
                return new Origin(scheme, host, port, request.getTag(), protocol);
        }

    };

please refer to an already closed ticket:https://github.com/jetty/jetty.project/issues/9049

But now we have moved to https communication over TLS Every thing else works fine but we observe that during the client Hello in TLS handshake, SNI does not contain our fqdn Without this overriding logic, SNI comes with a proper fqdn

Can you please help out on what more can be done so that SNI is picked up from the provided fqdn in request

joakime commented 5 months ago

This seems to be a duplicate of #11532

sbordet commented 5 months ago

The SNI is missing because it's an IP address, and as explained in #11532, this is not allowed.

What you can do is the following:

The last bullet is the key, the value of REMOTE_SOCKET_ADDRESS_CONTEXT_KEY will be used when creating the SSLEngine, and therefore control the SNI.

The first 2 bullets will allow you to subclass only one class.

Please report back if it worked.

souravgupta6nov1992 commented 5 months ago

We were able to achieve our requirement following the direction of your suggestion on using REMOTE_SOCKET_ADDRESS_CONTEXT_KEY to pass our fqdn. We have updated the Origin class adding one more field for keeping our fqdn. We extract the fqdn from this field inside newConnection() method of HttpClient class and form an InetSocketAddress which we use as value of REMOTE_SOCKET_ADDRESS_CONTEXT_KEY PFB the code for this. We have surrounded our code with comments. Please have a look at it and suggest if you find some gap.

Httpclient

    protected void newConnection(HttpDestination destination, Promise<Connection> promise)
    {
        // Multiple threads may access the map, especially with DEBUG logging enabled.
        Map<String, Object> context = new ConcurrentHashMap<>();
        context.put(ClientConnectionFactory.CLIENT_CONTEXT_KEY, HttpClient.this);
        context.put(HttpClientTransport.HTTP_DESTINATION_CONTEXT_KEY, destination);
        //our custom origin
        Origin origin = destination.getOrigin();
        /**
        *****************our changes Start
        */
        //our custom field
        if (null != origin.getOriginalHost()) {
            logger.debug("newConnection(). going to updated host in context map host: {}", origin.getOriginalHost());
            InetSocketAddress address = InetSocketAddress.createUnresolved(origin.getOriginalHost(), origin.getAddress().getPort());
            context.put(ClientConnector.REMOTE_SOCKET_ADDRESS_CONTEXT_KEY, address);
        }

        /**
        ******************our changes End
        */

        Origin.Protocol protocol = origin.getProtocol();
        List<String> protocols = protocol != null ? protocol.getProtocols() : List.of("http/1n.1");
        context.put(ClientConnector.APPLICATION_PROTOCOLS_CONTEXT_KEY, protocols);

        Origin.Address address = destination.getConnectAddress();
        resolver.resolve(address.getHost(), address.getPort(), new Promise<>()
        {
            @Override
            public void succeeded(List<InetSocketAddress> socketAddresses)
            {
                connect(socketAddresses, 0, context);
            }

            @Override
            public void failed(Throwable x)
            {
                promise.failed(x);
            }

            private void connect(List<InetSocketAddress> socketAddresses, int index, Map<String, Object> context)
            {
                context.put(HttpClientTransport.HTTP_CONNECTION_PROMISE_CONTEXT_KEY, new Promise.Wrapper<>(promise)
                {
                    @Override
                    public void failed(Throwable x)
                    {
                        int nextIndex = index + 1;
                        if (nextIndex == socketAddresses.size())
                            super.failed(x);
                        else
                            connect(socketAddresses, nextIndex, context);
                    }
                });
                transport.connect((SocketAddress)socketAddresses.get(index), context);
            }
        });
    }
public class Origin
{
    private final String scheme;
    private final Address address;
    private final Object tag;
    private final Protocol protocol;
    /**
    ***********our own field
    */
    private final String originalHost;

    public Origin(String scheme, String host, int port)
    {
        this(scheme, host, port, null);
    }

    public Origin(String scheme, String host, int port, Object tag)
    {
        this(scheme, new Address(host, port), tag);
    }

    public Origin(String scheme, String host, int port, Object tag, Protocol protocol)
    {
        this(scheme, new Address(host, port), tag, protocol);
    }

    /**
    *our changes Start
    */
    public Origin(String scheme, String host, int port, Object tag, Protocol protocol, String originalHost)
    {
        this(scheme, new Address(host, port), tag, protocol, originalHost);

    }

    /**
    *our changes End
    */

    public Origin(String scheme, Address address)
    {
        this(scheme, address, null);
    }

    public Origin(String scheme, Address address, Object tag)
    {
        this(scheme, address, tag, null);
    }

    public Origin(String scheme, Address address, Object tag, Protocol protocol)
    {
        this.scheme = Objects.requireNonNull(scheme);
        this.address = address;
        this.tag = tag;
        this.protocol = protocol;
        this.originalHost = null;
    }

    public Origin(String scheme, Address address, Object tag, Protocol protocol, String originalHost)
    {
        this.scheme = Objects.requireNonNull(scheme);
        this.address = address;
        this.tag = tag;
        this.protocol = protocol;
        this.originalHost = originalHost;
    }

    public String getScheme()
    {
        return scheme;
    }

    public Address getAddress()
    {
        return address;
    }

    public Object getTag()
    {
        return tag;
    }

    public Protocol getProtocol()
    {
        return protocol;
    }
    public String getOriginalHost() {
        return originalHost;
    }

    @Override
    public boolean equals(Object obj)
    {
        if (this == obj)
            return true;
        if (obj == null || getClass() != obj.getClass())
            return false;
        Origin that = (Origin)obj;
        return scheme.equals(that.scheme) &&
                address.equals(that.address) &&
                Objects.equals(tag, that.tag) &&
                Objects.equals(protocol, that.protocol);
    }

    @Override
    public int hashCode()
    {
        return Objects.hash(scheme, address, tag, protocol);
    }

    public String asString()
    {
        StringBuilder result = new StringBuilder();
        URIUtil.appendSchemeHostPort(result, scheme, address.host, address.port);
        return result.toString();
    }

    @Override
    public String toString()
    {
        return String.format("%s@%x[%s,tag=%s,protocol=%s]",
                getClass().getSimpleName(),
                hashCode(),
                asString(),
                getTag(),
                getProtocol());
    }

}
sbordet commented 5 months ago

@souravgupta6nov1992 you do not need to change the Origin class. It has a tag field that can be a string with your fqdnHost, if you want to store it, but it is not strictly necessary.

You only need to override HttpClientTransport like I suggested, build the Origin like you were doing a couple of comments ago, and replace REMOTE_SOCKET_ADDRESS_CONTEXT_KEY like I suggested.

souravgupta6nov1992 commented 5 months ago

if we do the overriding in HttpClientTransportOverHttp2 instead of HttpClient, we cannot add the REMOTE_SOCKET_ADDRESS_CONTEXT_KEY key-value there inside newOrigin() method as context map would not be initialized there and hence not avaialable. As we could see in HttpClient that context map is initialized in newConnection() method. This is the reason we chose to override httpclient instead where context map is initialized. which seems to be getting called after newOrigin() method. Can you clarify on in which class method you want us to add the key-value mapping for address.

sbordet commented 5 months ago

@souravgupta6nov1992 read again: https://github.com/jetty/jetty.project/issues/11534#issuecomment-2010395568