Approvals: 0/1
[21:19:31] *** ChanServ sets mode: +o temporal_
[22:09:33] <xkr47> temporal_, have a minute regarding #1477 (SNI support)?
[22:09:41] <temporal_> xkr47 hi
[22:09:43] <temporal_> yes
[22:09:51] <xkr47> temporal_, thanks!
[22:10:05] <xkr47> temporal_, you suggested providing a custom KeyManagerFactory in KeyCertOptions
[22:10:11] <xkr47> but I must be missing something..
[22:10:52] <xkr47> KeyManagerFactory as it is in Java8 has all methods marked final
[22:11:56] <xkr47> and one key method getKeyManagers() is hardwired to call the KeyManagerFactorySpi.engineGetKeymanagers()
[22:12:28] <xkr47> oh wait!
[22:12:33] <xkr47> oh hehe .. KeyManagerFactoryImpl is not public
[22:12:46] <temporal_> there is an SPI
[22:12:49] <xkr47> yes
[22:12:56] <xkr47> which is public
[22:13:07] <xkr47> so I should tell java somehow to use my own spi?
[22:13:52] <xkr47> and then just wrap the default SPI..
[22:15:18] <temporal_> yes
[22:15:32] <xkr47> ok great thanks I'll see if I can get it from here
[22:15:41] <temporal_> have a look at VertxTrustManagerFactory
[22:15:44] <temporal_> it does that
[22:15:59] <xkr47> where's that?
[22:16:10] <xkr47> new in 3.4?
[22:16:36] <xkr47> still at 3.3
[22:16:48] <xkr47> (will upgrade soon :)
[22:17:27] <xkr47> aha it's nonpublic
[22:18:00] <temporal_> ah I just said to see as an example
[22:18:05] <xkr47> :)
[22:18:19] <xkr47> so is the PROVIDER static field somehow the trigger
[22:18:27] <xkr47> or do you need to specify some system property?
[22:19:43] <xkr47> nope :)
[22:19:56] <xkr47> sorry for being a bit trigger-happy on the question side :)
[22:20:47] <xkr47> but hey, this is what it feels like when you read a good book.. you don't know the murderer yet but you feel you are getting the picture :)
[22:21:04] <xkr47> ⇒ I don't know what I'd rather do this saturday evening :)
[22:21:13] <xkr47> thanks temporal_ for helping out
[22:21:55] <temporal_> n/p
[22:22:02] <temporal_> by the way
[22:22:12] <temporal_> we do have an unfinished PR with SNI support
[22:22:14] <temporal_> in vertx core
[22:22:19] <temporal_> perhaps you want to contribute
[22:22:26] <temporal_> there is not much to finish actually
[22:22:28] <temporal_> 90% is done
[22:22:36] <xkr47> can you fork PRs ? :D
[22:23:33] <xkr47> you mean 1698 or ?
[22:23:38] <xkr47> isn't that client-only?
[22:23:50] <xkr47> but I haven't really looked at it
[22:23:56] <temporal_> I think it does server too as far as I remember
[22:24:25] <temporal_> ah right
[22:24:32] <xkr47> “Is there any chance to get this feature into 3.4.1?”
[22:24:37] <temporal_> none
[22:24:38] <xkr47> seems people are eagerly waiting :)
[22:25:03] <temporal_> well I could try to finish it actually
[22:25:11] <temporal_> that would break my uber rule “bug fixes only”
[22:25:17] <xkr47> :O
[22:25:20] <temporal_> but since I'm fixing things for 2 days
[22:25:28] <xkr47> there goes my murderer story :P
[22:25:43] <xkr47> :)
[22:26:20] <xkr47> hmm, setSniServerName in HttpClientOptions…
[22:26:36] <xkr47> doesn't that then require recreating the client to use different sni names in different requests?
[22:27:38] <temporal_> wdym ?
[22:27:55] <temporal_> I think I asked something different in the PR
[22:28:07] <temporal_> to add
[22:28:08] <temporal_> setVirtualHost(String)
[22:28:11] <temporal_> on HttpClientRequest
[22:28:18] <temporal_> instead
[22:28:29] <temporal_> it won't be in 3.4.1
[22:28:33] <temporal_> I think
[22:28:56] <xkr47> setSniServerName → setVirtualHost ?
[22:30:10] <temporal_> going to look at it
[22:30:13] <temporal_> yes
[22:31:28] <temporal_> acutally lot of code changed in NetServerImpl since the PR was done
[22:31:37] <temporal_> so the code should be rebased properly
[22:31:51] <xkr47> :P
[22:32:09] <temporal_> it's too bad it could not be finished
[22:32:12] <temporal_> it's a good feature
[22:32:19] <xkr47> yeah
[22:32:28] <xkr47> review in progress
[22:32:29] <temporal_> I think it's more useful on client than on server
[22:32:36] <temporal_> on server it can still be handled by a proxy
[22:32:41] <xkr47> so he picked the route where you give multiple KeyCertOptions
[22:32:50] <temporal_> on the client
[22:33:08] <temporal_> I'm not sure it should use this route actually
[22:33:51] <xkr47> I was myself thinking adding multi-domain support to the KeyCertOptions instead
[22:34:00] <xkr47> but not very deep thinking behind it really
[22:35:25] <temporal_> xkr47 agreed it would perhaps be more convenient
[22:35:51] <temporal_> this feature needs to be properly done
[22:36:42] <xkr47> hmm
[22:37:00] <xkr47> is it really necessary to have separate SSLContexts for each domain?
[22:37:32] <xkr47> also it might make sense to have domain-specific trust as well? if it's possible…
[22:37:57] <temporal_> I think it's a netty feature
[22:37:58] <xkr47> I do understand that you maybe have enough questions in your head without me helping out… :|
[22:38:18] <temporal_> I should have a closer look at SNI and how it is wired in Netty
[22:38:33] <temporal_> perhaps it could be a vertx extension
[22:38:36] <temporal_> and not be in core
[22:38:38] <temporal_> perhaps not
[22:38:45] <xkr47> this implementation here gave me the feeling it's not required.. but I may be wrong … https://github.com/grahamedgecombe/netty-sni-example/blob/master/src/main/java/SniKeyManager.java
[22:39:46] <xkr47> \_ that just loads a keystore to the KeyManagerFactory that contains all the certs & private keys
[22:40:37] <temporal_> so I think there are two ways to do it
[22:40:40] <temporal_> this way indeed
[22:40:46] <temporal_> and in netty there is also an official sni class
[22:40:52] <xkr47> at _least_ two
[22:40:52] <temporal_> from where the PR has been borrowed
[22:42:53] <temporal_> I'm wondering why SNI on client requires to have several trust store
[22:43:31] <xkr47> me too
[22:43:56] <temporal_> and on client, can't SNI be mapped on a keystore on alias ?
[22:43:58] <xkr47> so where's that “Tls SNI … the good parts” book I apparently need to read :)
[22:44:05] <temporal_> lol
[22:44:11] <temporal_> yes SNI for dummies
[22:45:24] <temporal_> ah I know
[22:45:33] <xkr47> to me SNI on the client side is like the Host HTTP header.. it just tells where we want to go
[22:45:40] <temporal_> we should look at how SNI is handled in Node and/or other java server supporting it
[22:45:44] <temporal_> xkr47 yes
[22:45:53] <temporal_> hence the setVirtualHost on HttpClientRequest
[22:46:04] <temporal_> and the addition on NetClient.connect
[22:46:22] <xkr47> temporal_, could you even just use the setHost() you have today?
[22:46:38] <xkr47> after all they're probably supposed to have the same value?
[22:46:48] <temporal_> and now in 3.4.0 we ahve a new
[22:46:50] <temporal_> RequestOptions
[22:46:52] <temporal_> that has
[22:46:58] <temporal_> host/port/ssl/uri
[22:47:10] <temporal_> it seems that we could add the sniName here
[22:47:34] <temporal_> xkr47 I think yes but I need to check if there are exceptions
[22:47:44] <xkr47> MindBlownException
[22:47:53] <xkr47> there you go
[22:47:54] <temporal_> today in vertx http client
[22:48:14] <temporal_> when you don't set an Host header, the header is set from the connect address
[22:48:21] <xkr47> yes
[22:48:26] <temporal_> it seems that if sni is supported, it would do the same
[22:48:36] <temporal_> and there would be an option to override it
[22:48:42] <temporal_> so something like sniEnabled in HttpClientOptions
[22:48:46] <temporal_> to activate it
[22:48:56] <xkr47> what I don't know if there is harm in enabling sni by default
[22:49:09] <temporal_> it's an extension
[22:49:13] <xkr47> do browsers always send the SNI Hostname option ?
[22:49:18] <temporal_> so a server not implementing it will ignore it
[22:49:26] <temporal_> I think most browser do today
[22:49:36] <xkr47> yeah, just like a HTTP/1.0 server might ignore the Host header..
[22:50:18] <temporal_> I think the main difference is that
[22:50:37] <temporal_> you might want to not set SNI if you want to have the proxy certificate
[22:50:47] <temporal_> and not the virtual host certificate
[22:50:55] <temporal_> but it's not certain it's useful
[22:51:34] <temporal_> https://nodejs.org/api/tls.html
[22:51:41] <temporal_> The tlsSocket.servername property is a string containing the server name requested via SNI.
[22:51:58] <temporal_> it seems similar to the ConnectOptions I wanted to introduce in NetSocket
[22:52:04] <xkr47> well the client at least oughta know if it's talking to a proxy :)
[22:52:05] <temporal_> actually in HttpClient#connect
[22:52:11] <temporal_> true
[22:52:37] <temporal_> on server there is
[22:52:38] <temporal_> server.addContext(hostname, context)
[22:52:46] <temporal_> I don't know if context is like SSLContext in java
[22:53:39] <xkr47> hmm
[22:54:07] <xkr47> anyway
[22:54:33] <temporal_> sni is important for cloud
[22:54:37] <xkr47> yes
[22:54:42] <xkr47> and my home server ;)
[22:54:58] <xkr47> but
[22:55:04] <xkr47> if I may
[22:55:16] <xkr47> I really would like if the whole thing was dynamic
[22:55:33] <xkr47> and not “set up server with this fixed set of sni-certificate bindings”
[22:56:04] <xkr47> then I could implement letsencrypt tls-sni without shutting down the server socket in between
[22:56:23] <temporal_> http://stackoverflow.com/questions/20807408/handling-multiple-certificates-in-nettys-ssl-handler-used-in-play-framework-1-2
[22:56:37] <temporal_> this guy at least wants to map sni to keystore alias
[22:57:10] <temporal_> on the server you mean ?
[22:57:18] <xkr47> yes
[22:58:11] <xkr47> so basically when asking to do the “tls-sni-01” or “tls-sni-02” challenge, the letsencrypt (or ACME) server asks to create a temporary certificate with a specific fake hostname that it then will request using SNI
[22:58:51] <temporal_> yes but you can do that using a tool as far as I remember
[22:59:01] <xkr47> so in order not to have to shut down my production traffic to do this, I would like if I could just implement the certificate selection myself
[22:59:55] <xkr47> letsencrypt also supports http-01 challenge which just serves a static file at a predetermined url
[23:00:04] <xkr47> this is what the tool does afaik
[23:00:31] <xkr47> but but I like challenges so I'd like to try to implement the TLS version.. with vertx..
[23:01:35] <temporal_> I think the current KeyCertOptions allows you to do that as you can provide a factory
[23:04:14] <temporal_> currently the SSLHelper manages the creation of SSLContext from options
[23:04:14] <xkr47> .. yyees.. but now I feel like the duck in duck hunt
[23:04:22] <xkr47> ;)
[23:04:53] <temporal_> we will see all of that after 3.4.1
[23:05:48] <xkr47> what's your timeline for 3.4.1?
[23:06:12] <temporal_> monday
[23:06:16] <xkr47> wow
[23:06:22] <xkr47> drop this, now
[23:06:22] <xkr47> :D
[23:07:50] <xkr47> I'm sorry
[23:08:41] <xkr47> in my defense, you sucked yourself into the discussion when with your “yes” answer an hour ago
[23:09:52] <temporal_> n/p
[23:10:00] <temporal_> I'm uploading stuff
[23:11:39] <xkr47> it's a bit funny that SSLHelper gets the keyCertOptions as a separate parameter “public SSLHelper(HttpClientOptions options, KeyCertOptions keyCertOptions, TrustOptions trustOptions) {” but then “this.sniKeyCertOptions = options.getSniKeyCertOptions();”
[23:11:49] <xkr47> in the 1698 PR that is
[23:11:57] <xkr47> but this is internal anyway
[23:22:41] <xkr47> I find it funny that the ssl handler is replaced in VertxSniHandler
[23:27:14] <xkr47> hmm
[23:28:16] <xkr47> in the client case, aren't the KeyCertOptions used to provide client certificates, only?
[23:29:33] <xkr47> or are they also used to provide trusted CA certs (not in the default trusted CA list from the JVM)?
[23:29:52] <xkr47> s/not in/that are not in/
[23:35:41] <temporal_> in the client case KeyCertOptions are used if the client wants to authenticate
[23:35:44] <temporal_> and it's not related to SNI
[23:35:58] <temporal_> TrustOptions are related to validating server certficiates
[23:36:03] <temporal_> I mean they do that
[23:36:24] <xkr47> yes.. so the addSniKeyCertOption() is a bit funny to me
[23:36:51] <xkr47> wouldn't it just be addDomainKeyCertOption(), unrelated to SNI, if you wanted to have domain-specific authentication certs?
[23:37:14] <xkr47> (trustoptions - ok yeah)
[23:38:00] <xkr47> but then again you would think that's not really needed since doesn't the client pick the correct cert depending on the server domain anyway?
[23:38:48] <xkr47> but I'm acknowledging my average knowledge of TLS in general
[23:57:19] <xkr47> OH CRAP, SunX509KeyManagerImpl iterates through the certs & keys in the KeyStore at construction time
[23:58:06] <xkr47> welp that means the instance needs to be recreated every time one wants to change the list dynamically