Internet.nl icon indicating copy to clipboard operation
Internet.nl copied to clipboard

Making visible if TLS 1.3 and/or TLS 1.2 is used

Open baknu opened this issue 5 years ago • 13 comments

Currently we only show in the tech table of the "TLS version" subtest what TLS versions are PHASE OUT or INSUFFICIENT (if we detect these). It would be helpfull if we also display GOOD and SUFFICIENT versions. This way users can check if their provider support the latest (and best) TLS version 1.3. This is also in line with the latest TLS guidelines from NCSC-NL which aims to motivate users to ask their provider to support TLS1.3.

This would mean that naming of the technical table should be changed as follows.

From: Mail server (MX) | Affected TLS versions | Status

to : Mail server (MX) | Detected TLS version | Status

-- This could probably also be done for the subtest "Ciphers (Algorithm selections)".

baknu avatar Jan 31 '21 17:01 baknu

This is something that could not be easily implemented with the current approach of internet.nl. For the TLS tests there is effort to find the bad configurations and report them. No effort is made to check what/how good configurations are used.

If this changes and all TLS versions are reported (they need to be, otherwise if something is not "detected" it means it is not there), as a user I would also expect to see all the ciphers that are supported. This (connecting with all available ciphers and versions) is something that internet.nl tries to avoid. Especially for the mail test.

BTW, TLS1.3 adoption could be motivated by having 1.2 as phase out in the future.

gthess avatar Feb 05 '21 17:02 gthess

What's the expected arrival time for a "1.2 is phase-out" NCSC-NL directive? Could be quite a long time... Perhaps an addition to the description or FAQ saying something like "to check all supported ciphers, use Peter Mosmans excellent tool at https://github.com/PeterMosmans/testssl.sh " would be helpful (oblig note: I did not check to see if we already do that)

sinteur avatar Feb 05 '21 18:02 sinteur

Ok, thanks for feedback! I can see that the "making visible what is supported approach" has way more impact on the cipher test than on the TLS version test. Therefore I suggest to go for a step-by-step approach. First step could be TLS versions and (maybe) later also ciphers.

  1. TLS versions: There is quite some interest from our users in making it visible if TLS 1.3 is supported (and subsequently also if TLS 1.2 is supported). This has grown since NCSC-NL lowered TLS 1.2 from GOOD to SUFFICIENT. (I can't tell when NCSC-NL changes this to PHASE-OUT. This probably mainly depends on possible future vulnerabilities in TLS 1.2.)

    • Would we need more connections to make it possible if TLS 1.3 and/or TLS 1.2 is supported? If so, how many more?

    • Note that in the API we already provide for an extra field for TLS 1.3 support. Here is the explanation of that extra field:

TLS 1.3 Support: Derives TLS1.3 support through the 0-RTT test. Explicitly testing for TLS1.3 support is not part of the compliance tool. However, TLS1.3 support could be derived from the 0-RTT test as the function is only available starting from TLS1.3. As there is no explicit TLS1.3 connection during testing, the test assumes that the server chose TLS1.3 when given the opportunity to do so."

  1. Ciphers:
  • This heavily impacts performance (mainly test duration and also the risk of running into rate-limiting), because we need much more connections, right? Could you give an indication on how much more connections?
  • First step could be to refer to another test or test library in the further testing box, test explanation and/or FAQ (like John suggested).
  • If we want to implement this (and I am not sure yet), we need to think on an approach that has the least impact on performance. I have the following first thoughts: - After the first test that only detects INSUFFICIENT and PHASE-OUT ciphers, we offer a link to further test for all supported ciphers (i.e. SUFFICIENT and GOOD). So after a normal test a user has to click and activate an additional test to get these extra results. - We test domains that have been tested before automatically on a daily basis (and on a slow pace) for all supported ciphers and show cached results for the all supported ciphers part.

baknu avatar Feb 08 '21 09:02 baknu

I'll check if "show highest tls version" per IP address is simple enough to do. If that satisfies a significant part of this wish, it may indeed be interesting to do this in steps.

sinteur avatar Feb 08 '21 09:02 sinteur

@sinteur: ok @gthess: do you have any additional ideas on this? Thanks!

baknu avatar Feb 08 '21 09:02 baknu

Looks possible. I've just put the TLS version instead of "None" when the results are "sufficient" or better. Like this:

Screenshot 2021-02-08 at 13 54 36

Of course, layout and exact text to be decided, just proof of concept here.

sinteur avatar Feb 08 '21 12:02 sinteur

Yes, nice. Some remarks:

  • It seems now only the highest supported TLS version is shown. So for example it does only show TLS 1.3, when both TLS 1.2 and 1.3 are supported by a tested web server.
  • When we detect SSL 1.0, 2.0, 3.0 (INSUFFICIENT) or TLS 1.0 or TLS 1.1 (PHASE-OUT) we show a status column (you can use tls-v1-0.badssl.com as an example test domain). I believe this status column should also be there when TLS 1.2 and/or 1.3 are detected. They have the respective statuses SUFFICIENT and GOOD.

baknu avatar Feb 08 '21 14:02 baknu

Like this:

Screenshot 2021-02-09 at 12 28 51

sinteur avatar Feb 09 '21 11:02 sinteur

(I used "secure" instead of "good" - tell me if you disagree)

sinteur avatar Feb 09 '21 11:02 sinteur

I agree that doing the versions is much more easier. The extra connections would be either 0 or 1. My argument is that by doing that, user demand will easily shift to do the same for the ciphers as well.

For the API output, as explained in the documentation of the field, this is a derived result. There is not explicit check for TLS1.3.

For the ciphers I have the following concerns:

  1. Testing for all the ciphers we would need 265 connections in total for the worst case scenario (server supports everything)
  2. I don't like the link idea. If ratelimiting would happen, by clicking the link (why wouldn't you?) you will make this domain suffer for future ratelimiting. As a user I also don't see why this is not the default behavior.
  3. I don't like the idea of the cached results and working constantly on the background. This has the potential for really broken results if the domain is undergoing config changes. How would you know to discard cached results after a while (Except for a hard coded time limit value)? Internet.nl is not a constant monitoring service, we try to give fast results about compliance back to normal users.
  4. Even if we do this only for the webtest, there are cases that we have hit ratelimiting on webservers. This is admittedly very rare (I only remember discussing this once) but worth noticing.

gthess avatar Feb 09 '21 11:02 gthess

(I used "secure" instead of "good" - tell me if you disagree)

I suggest (mainly for consistency reasons) to adhere to the terminlogy of the NCSC guidelines (i.e. Good / Sufficient / Phase out / Insufficent).

baknu avatar Feb 11 '21 11:02 baknu

I agree that doing the versions is much more easier. The extra connections would be either 0 or 1. My argument is that by doing that, user demand will easily shift to do the same for the ciphers as well.

Thanks! Until now we received quite some requests to explicitly test for TLS 1.2 and TLS 1.3. Your helpful remarks show that it is far from trivial to add similar testing for ciphers. Furthermore in general this seems to be a 'nice to have' and not a 'must have'. Let's therefore start with TLS versions now, and wait and see if we get (more) requests to add this for ciphers as well. If the latter happens we could add a remark in the test explanation on why we do not detect and show all ciphers, and we could also refer to other tools like testssl.sh.

baknu avatar Feb 11 '21 11:02 baknu

Replaced "secure" with "good"

See https://github.com/NLnetLabs/Internet.nl/pull/517

sinteur avatar Feb 11 '21 12:02 sinteur