[Vm-dev] halp.

Tue Jun 13 22:57:43 UTC 2017

> On 14.06.2017, at 00:13, Levente Uzonyi <leves at caesar.elte.hu> wrote:
> 
> On Tue, 13 Jun 2017, Tobias Pape wrote:
> 
>> Dear all
>> 
>> I'm calling from the ssl-basement, and lo and behold, it's all a mess.
>> 
>> First, the "fun" parts:
>> - OSX SSL does not support ALPN (necessary for good HTTP/2, go figure)
>> - while we are at picking, OSX SSL, when using via secure transport, does not give you more hints than
>>  * trustworthy (+ trustworthy cause you said me so via keychain)
>>  * _maybe_ trustworthy
>>  * not trustworthy.
>>  Here's the gem from their code:
>> 	/* ... */
>> 				status = errSSLXCertChainInvalid;
>>           }
>>           /* Do we really need to return things like:
>>                  errSSLNoRootCert
>>                  errSSLUnknownRootCert
>>                  errSSLCertExpired
>>                  errSSLCertNotYetValid
>>                  errSSLHostNameMismatch
>>              for our client to see what went wrong, or should we just always
>>              return
>>                  errSSLXCertChainInvalid
>>              when something is wrong? */
>>           break;
>> 
>> 	/* ... */
>>    Really, that's the current code. Color me surprised (to put it politely)
>> 
>> 
>> 
>> Now the really un-fun part: peer-name validation.
>> -------------------------------------------------
>> 
>> It's half-way ok for osx, quite ok for windows but not right and absent for openssl/unix/linux.
>> 
>> Levente set out to make that better two years ago but I stopped him (sorry for blocking this, btw :( )
>> Just now I had to close his PR (https://github.com/squeak-smalltalk/squeakssl/pull/3), and I think I'm stuck.
>> 
>> Just HOW can we achieve the following without _massive_ engineering effort, essentially duplicating large parts of libcurl?
>> 
>> **What do we want?**
>> --------------------
>> Check that we connected to the right peer with the right certificate.
>> 
>> Up to now the idea _was_ to extract the "peer name" from the `CommonName` of whom we contacted.
>> 
>> However, this is completely insufficient because it need things (a) beyond checking the commonName (serverAlternativeName) and (b) sAN has some priorities as per RFT 2818, 3.1 Server Idenitity et al.
>> 
>> Also: What if a cert has multiple names in the CN (yes that is possible) or the sAN (yes, that is even common)?
>> 
>> OK, we hence _must_ ditch the idea to have the "peer name" handed to the image, because we cannot know _which_ one of multiple the image wants… except when we were given a `serverName` previously.
>> 
>> But even then, this is _really_ complex:
>> 
>> 1. If a certificaet has sAN of dNSName type it MUST be used (even if cn is present).
> 
> Sounds easy: skip CN when SAN/DNSName is there.
> 
>> 2. Certificates not having sAN (and cn only) is DEPRECATED.
> 
> Just because it's deprecated, we should still support it, because most certificates are probably still like that.

well, Google has removed support for CN in Chrome 58 (released in April)

https://groups.google.com/a/chromium.org/forum/#!msg/security-dev/IGT2fLJrAeo/csf_1Rh1AwAJ 

> 
>> 3. If a certificate has multiple sAN/dNSName, any of them is a potential match.
> 
> Just iterate over them.
> 
>> 4. Wildcards (*) SHALL only appear as whole name parts, and have other restrictions.
>>  *  Example:
>> 		- `*.example.com` is valid, but
>> 		- `*x.example.com` is technically valid but practically invalid
>> 		- `*example.com` is  technically valid but practically invalid
>> 		- `test.*.example.com` is invalid (as per RFC 6125)
>> 		- `*.*.example.com` is invalid
>> 		- `*`, `*.`, `*.*`, etc. is invalid
>>   * Other matching examples:
>> 		- `*.example.com` matches `a.example.com` BUT does NOT match `a.b.example.com`
>> 		- `example.com` (in the certificate) matches `example.com.` (hostname) (note trailing period) BUT
>> 		- `example.com.` (in the certificate) does NOT match `example.com.` (hostname) (note trailing period)
>> 		- `example.com` (in the certificate) matches `EXAMPLE.com` (hostname)
> 
> This sounds really complicated, but it's just a case-insensitive suffix comparison + the remaining part must end with a dot and must not contain any other dots. Perhaps the 63 character limit can be enforced, but any other checks are probably unnecessary.

I don't think so. for example, curl disables wildcards in idn-names (punycode), which sounds somehow reasonable, and
Apple has these 20+ distinct cases they test: https://github.com/CamJN/Security/blob/master/SecurityTests/ssl-policy-certs/TestDescriptions.txt

> The whole thing should take at most 100 lines of platform independent C code.

yes, but since even strndup is not portable, I'd like to question that.

Also, in curl, this stuff for OpenSSL is around 300 lines, PLUS ca 70 lines only for the actual hostname-checking algo.

See here: https://github.com/curl/curl/blob/master/lib/hostcheck.c#L63
	And curl does not detect invalid things like '*.co.uk'
	And curl has a strcasecompare somewhere, which would add some more lines

This does not consider OSX/SecureTransport and Win32/SChannel.
Those two libraries actually _do_ check the hostname in the default case, but since we _disable_ the verification there,
we would have to duplicate the extraction/precedence logic etc, which is completely different c styles with completely different concepts on how to retrieve a certificate.
(as you say below)
> 
> Perhaps I'm just not seeing something, but this part doesn't sound complicated at all. IIRC extracting the necessary data (CN/SAN) took a lot more effort.

What I just wanted to say is all those things do not make for nice code and (except for unix) could have been done by the os…

I don't know. Maybe it's too late here already for me to form a coherent thought :/

Best
	-Tobias

> 
> Levente
> 
>> 5. IP Addresses MUT be of type sAN/iPAddress, NOT sAN/dNSName, they MUST match exactly.
>> 
>> Due to 4., name checking is CANNOT as simple as
>> ```Smalltalk
>> certificateName match: peerName
>> ```
>> on the image side.
>> 
>> Systems
>> -------
>> - On openssl/linux/unix, we have to care for _all_ of the above manually.
>> - On osx, we have the funny trust thing from above and hence would have to do the checking on our own (which we would get for free if we wouldn't break the connection right before the internal trust evaluation and do that manually)
>> - On win32, the trust evaluation is very good, verifying for us the hostname, too, but extracting it for image side hasn't been done.
>> 
>> ===============================================================
>> 
>> To all poor souls still reading, What Shall We Do?
>> 	-Tobias