nng icon indicating copy to clipboard operation
nng copied to clipboard

TLS listener/dialer should handshake

Open gdamore opened this issue 6 years ago • 7 comments

The new (not yet merged) TLS listener and dialer code should perform the TLS handshake before returning the connection. This way, we can be certain that we are only receiving real connections that are able to perform message exchange, and not ones that are not suitable due to being invalid.

Probably also we should consider an option to not return the validation errors back to the client.

Also one can consider an async I/O function, nni_tls_handshake() that takes an nng_tls object and an AIO, and signals only when the handshake is complete.

gdamore avatar Jan 06 '19 20:01 gdamore

Hello and thank you for your benevolent dictatorship.

I can say that validation errors would be helpful. When trying to create a custom dialer to use TLS I was getting the "nng_dialer_start: Cryptographic error" message. I had to drop down to mbedtls to create an SSL connection directly to find out that it was failing on x509 peer verification. So if you are able forward the mbedtls error output up the chain to the client I think that would be really helpful. I also think the documentation could me more clear that the NNG_OPT_TLS_CONFIG is the specifier that should be used to pass the TLS config object to the custom dialer when using nng_dialer_setopt_ptr().

However, after getting around that error and clearly getting past SSL handshake and peer verification now, I can't seem to find a socket protocol that will work with the custom dialer and the TLS config object set. I always get the following error when starting the custom dialer:

nng_dialer_start: Protocol error

Snippet:

//create a NNG socket
if ((rv = nng_req0_open(&sock)) != 0) {
    fatal("nng_socket", rv);
    return -1;
}
//create a custom dialer to set TLS config object as a dialer option
nng_dialer tlsDialer;         //nng client must dealloc this memory?
if ((rv = nng_dialer_create(&tlsDialer, sock, rdb_conn->addr)) != 0) {
    fatal("nng_dialer_create", rv);
    return -1;
}
//set the custom dialer options, such as the NNG TLS config object
if ((rv =  nng_dialer_setopt_ptr(tlsDialer, NNG_OPT_TLS_CONFIG, nngTLS)) != 0) {
    fatal("nng_dialer_setopt_ptr", rv);
    return -1;
}
//start the dialer
int dialerFlags = 0;
if ((rv =  nng_dialer_start(tlsDialer,dialerFlags)) != 0) {
    fatal("nng_dialer_start", rv);
    return -1;
}

joemoulton avatar May 19 '19 02:05 joemoulton

Ok, I found your newer nni_tls_dialer api based on nng_stream and I've got that communicating with my db service. So I suppose you can ignore my comment.

  1. I'm kind of unsure whether the TLS listener and dialer you are referring to in this issue is the nni_tls_dialer or not. I guess I am not seeing the connection being returned before the handshake from what I am observing.
  2. Supplemental/tls/tls_api.h is not yet getting included in the includes dir, but the tls_common.c is fortunately getting built into the lib.
  3. Have protocols been deprecated completely in favor of byte streams?

joemoulton avatar May 19 '19 05:05 joemoulton

Oh, one more item. Async read of the aio returned from the server is of course lightning fast so synchronous performance is not a huge deal for me considering, but if I wait for the aio associated with the nng_stream_recv it seems to take several seconds to return. How does the aio completion notification get scheduled back to the calling thread?

joemoulton avatar May 19 '19 05:05 joemoulton

The nng_stream APIs are separate, and offer lower level capabilities than the upper layer message oriented APIs. Only use the stream APIs if you know you need them, otherwise you should use the protocols.

gdamore avatar May 19 '19 22:05 gdamore

The tls/tls_api.h header is "internal". Don't use it directly in your application program. There's a reason why it isn't included. You should instead interface with the related functions via abstract nng_stream APIs, or via the nng_listen / nng_dial family of functions (if using the upper layer protocols.)

gdamore avatar May 19 '19 22:05 gdamore

(Generally, symbols starting with nni_* are private (internal use only). You should only use nng_* symbols.

gdamore avatar May 19 '19 22:05 gdamore

Ok, understood. My first comment indicates that I was getting protocol error after SSL handshake when using nng_dialer_start. It is unclear how I should proceed to debug that error or why the protocol was invalid. That's why I went looking for the tls_api.h based on your comments related to it in other issues and this was my first encounter with the stream api.

joemoulton avatar May 19 '19 22:05 joemoulton