Differences

This shows you the differences between two versions of the page.

Link to this comparison view

irc:1489186800 [2017/05/27 13:44] (current)
Line 1: Line 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