async-io icon indicating copy to clipboard operation
async-io copied to clipboard

Option to ignore/suppress TLS issues

Open tadman opened this issue 4 years ago • 4 comments

Some badly behaved clients fumble TLS negotiation which ends up causing a lot of stack dump activity in the Async logs. Example:

    4m: Async::Task
      |   OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 state=error: tlsv1 alert unknown ca
      |   → /.../gems/3.0.0/gems/async-io-1.30.2/lib/async/io/generic.rb:216 in `accept_nonblock'
      |     /.../gems/3.0.0/gems/async-io-1.30.2/lib/async/io/generic.rb:216 in `async_send'
      |     /.../gems/3.0.0/gems/async-io-1.30.2/lib/async/io/generic.rb:62 in `block in wrap_blocking_method'
      |     /.../gems/3.0.0/gems/async-io-1.30.2/lib/async/io/ssl_socket.rb:145 in `block in accept'
      |     /.../gems/3.0.0/gems/async-1.29.0/lib/async/task.rb:263 in `block in make_fiber'

It's unavoidable that some clients are going to be crappy or fail to negotiate. Is it possible to disable and/or ignore these instead of letting them bubble up? There doesn't seem to be a place in the stack to intercept these, it's too low-level.

tadman avatar May 03 '21 19:05 tadman

Due to how these are being delegated by a "Wrapper" which encapsulates them in an Async::Task there's no way to rescue them before the task exception reporter has already raised the alarm.

tadman avatar May 03 '21 19:05 tadman

I don't think we can make a determination about whether it constitutes an error or not. It may well do. Just because in some cases it's a "fault of the client" doesn't mean it's not an error that should be reported. Yes, it may be noisy because many bad clients. This should probably be a cause for concern. I agree that from some point of view, it's much more noisy than a system which simply ignores/fails to log those issues.

ioquatix avatar May 04 '21 01:05 ioquatix

I agree it's not the place of the library to decide what is or isn't important, but there should be a way to rescue these somehow, and presently there isn't due to the structure where it's embedded in an Async::Task you can't control.

Is there a way to wait on this during the accept process? This test code doesn't help:

tls_socket = Async::IO::SSLSocket.new(io, tls_context)
tls_socket.accept
tls_socket.wait # Doesn't actually capture exceptions, internally just waits for read/write status changes

Like what I'd hope to see is something like:

tls_socket = Async::IO::SSLSocket.new(io, tls_context)

begin
  tls_socket.accept.wait
rescue OpenSSL::SSL::SSLError
  # ...deal with it...
end

As implemented the accept method doesn't return something that can do that.

tadman avatar May 04 '21 15:05 tadman

Hopefully this helps:

require 'async'
require 'async/io'
require 'localhost/authority'

# Get the self-signed authority for localhost:
authority = Localhost::Authority.fetch

endpoint = Async::IO::Endpoint.tcp("localhost", 4040)
server_endpoint = Async::IO::SSLEndpoint.new(endpoint, ssl_context: authority.server_context)
client_endpoint = Async::IO::SSLEndpoint.new(endpoint, ssl_context: authority.client_context)

Async do |parent|
  server = parent.async do
    server_endpoint.bind do |socket|
      socket.listen(Socket::SOMAXCONN)
      
      begin
        peer, address = socket.accept
        peer.accept
        peer.write("Hello World!")
        peer.close
      rescue OpenSSL::SSL::SSLError
        # Ignore.
      end
    end
  end
  
  client = client_endpoint.connect
  Console.logger.info(client, client.read)
  
  client.close
  server.stop
end

ioquatix avatar May 06 '21 12:05 ioquatix