[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