Skip to content
This repository has been archived by the owner on Aug 29, 2024. It is now read-only.

Option to ignore/suppress TLS issues #48

Open
tadman opened this issue May 3, 2021 · 4 comments
Open

Option to ignore/suppress TLS issues #48

tadman opened this issue May 3, 2021 · 4 comments

Comments

@tadman
Copy link

tadman commented May 3, 2021

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
Copy link
Author

tadman commented May 3, 2021

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.

@ioquatix
Copy link
Member

ioquatix commented May 4, 2021

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.

@tadman
Copy link
Author

tadman commented May 4, 2021

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.

@ioquatix
Copy link
Member

ioquatix commented May 6, 2021

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants