getdnsapi / stubby

Stubby is the name given to a mode of using getdns which enables it to act as a local DNS Privacy stub resolver (using DNS-over-TLS).
https://dnsprivacy.org/dns_privacy_daemon_-_stubby/
BSD 3-Clause "New" or "Revised" License
1.17k stars 97 forks source link

TLS verification fails on self-signed certificates with LibreSSL 2.7, pass with OpenSSL #102

Open CaseOf opened 6 years ago

CaseOf commented 6 years ago

Hello, Stubby is able to run in strict mode with LibreSSL since 2.7 release. This version supports now OpenSSL 1.0.2 and 1.1 APIs. It works with servers that have a certificate signed by an authority. However, TLS verification fails on self-signed certificates. Here is a debug log with Lorraine Data Network server which have a self-signed certificate:

[12:04:55.862914] STUBBY: 80.67.188.188                            : Conn opened: TLS - Strict Profile
[12:04:55.918753] STUBBY: 80.67.188.188                            : Verify failed: TLS - *Failure* -  (18) "self signed certificate"
[12:04:55.918834] STUBBY: 80.67.188.188                            : Conn closed: TLS - *Failure*
[12:04:55.918845] STUBBY:    *FAILURE* no valid transports or upstreams available!
[12:04:55.918918] STUBBY: 80.67.188.188                            : Conn closed: TLS - Resps=     0, Timeouts  =     0, Curr_auth = Failed, Keepalive(ms)=     0
[12:04:55.918930] STUBBY: 80.67.188.188                            : Upstream   : TLS - Resps=     0, Timeouts  =     0, Best_auth = Failed
[12:04:55.918936] STUBBY: 80.67.188.188                            : Upstream   : TLS - Conns=     0, Conn_fails=     1, Conn_shuts=      0, Backoffs     =     0
ArchangeGabriel commented 6 years ago

This is expected and known. In strict mode self-signed certificates are considered invalid, even with a pinned key.

CaseOf commented 6 years ago

With OpenSSL, stubby consider self-signed certificate valid, as the verification pass. Maybe, is it OpenSSL that have a wrong behavior?

ArchangeGabriel commented 6 years ago

No, it does not work either with OpenSSL here.

CaseOf commented 6 years ago

Which version of OpenSSL? My tests with OpenSSL were with version 1.1.0.h on ArchLinux.

ArchangeGabriel commented 6 years ago

@CaseOf Exactly the same (I’m Arch packager btw).

ArchangeGabriel commented 6 years ago

See https://github.com/getdnsapi/getdns/issues/293#issuecomment-367652547 also.

saradickinson commented 6 years ago

Hi Folks - please see the release notes for getdns 1.4.1/stubby0.2.2 in particular: "For OpenSSL versions before 1.0.0 and for LibreSSL, self-signed certificates are no longer tolerated."

The issue for OpenSSL (and I believe LibreSSL) is that due to the way the default error handling works the verification stops when it hits the first error (e.g. a self-signed). Previously, getdns accepted the cert at this point if the SPKI pin matched, but there is no way to proceed past that first error to check for other errors (e.g. certificate is expired) so this was a security issue - some certificates were accepted when they should not have been.

The code was changed in 1.4.1 for OpenSSL 1.0.0 and later to use a different OpenSSL based error checking mechanism which can correctly asses self-signed certificates, however I don't believe this alternative checking can be implemented for LibreSSL. So for security, when the proper error checks can't be done we no longer accept self-signed certs. @wtoorop can correct me if I'm wrong.

wtoorop commented 6 years ago

Almost right, the code was changed in 1.4.0.

I've tried to make Viktor's library work for LibreSSL and asked Viktor to have a look too, but we didn't manage.

Besides fixing the Security issue, this change also enabled verification of upstreams with pinsets in installations without a Certificate Authority repository for which we had an issue at the time too.

ArchangeGabriel commented 6 years ago

But self-signed certificates do not seem to work currently with OpenSSL 1.1.0. If I use only LDN servers, I get this without setting tls_auth_name: Verify failed : TLS - *Failure* - (18) "self signed certificate".

Only when setting tls_auth_name (which is 80.67.188.188 in this case apparently, and not ns0.ldn-fai.net) did I got it working.

ArchangeGabriel commented 6 years ago

(@saradickinson The wiki should be updated about this server)

wtoorop commented 6 years ago

@ArchangeGabriel it should. It does with me:

willem@makaak:~$ getdns_query -i|grep openssl
  "openssl_build_version_number": 269488130,
  "openssl_built_on": <bindata of "built on: Thu Apr  5 12:03:07 20"...>,
  "openssl_cflags": <bindata of "compiler: gcc -fPIC -pthread -m6"...>,
  "openssl_dir": <bindata of "OPENSSLDIR: "/home/willem/local/"...>,
  "openssl_engines_dir": <bindata of "ENGINESDIR: "/home/willem/local/"...>,
  "openssl_platform": <bindata of "platform: linux-x86_64">,
  "openssl_version_number": 269488132,
  "openssl_version_string": <bindata of "OpenSSL 1.1.1-pre4 (beta) 3 Apr "...>,
willem@makaak:~$ getdns_query -sLmy 7 @2001:913::8 -K 'pin-sha256="WaG0kHUS5N/ny0labz85HZg+v+f0b/UQ73IZjFep0nM="' example.com A +return_call_reporting
[12:28:10.351709] UPSTREAM 2001:913::8                              : Conn opened: TLS - Strict Profile
[12:28:10.424501] UPSTREAM 2001:913::8                              : Verify passed : TLS
[12:28:10.444050] UPSTREAM 2001:913::8                              : Conn closed: TLS - Resps=     1, Timeouts  =     0, Curr_auth =Success, Keepalive(ms)=     0
[12:28:10.444153] UPSTREAM 2001:913::8                              : Upstream   : TLS - Resps=     1, Timeouts  =     0, Best_auth =Success
[12:28:10.444210] UPSTREAM 2001:913::8                              : Upstream   : TLS - Conns=     1, Conn_fails=     0, Conn_shuts=      0, Backoffs     =     0
{
  "answer_type": GETDNS_NAMETYPE_DNS,
  "call_reporting":
  [
    {
      "idle timeout in ms": 0,
      "query_name": <bindata for example.com.>,
      "query_to":
      {
        "address_data": <bindata for 2001:913::8>,
        "address_type": <bindata of "IPv6">
      },
      "query_type": GETDNS_RRTYPE_A,
      "resolution_type": GETDNS_RESOLUTION_STUB,
      "responses_for_this_upstream": 1,
      "responses_on_this_connection": 1,
      "run_time/ms": 19,
      "server_keepalive_received": 0,
      "timeouts_for_this_upstream": 0,
      "timeouts_on_this_connection": 0,
      "tls_auth_status": <bindata of "Success">,
      "tls_peer_cert": <bindata of 0x308205633082034ba003020102020900...>,
      "tls_version": <bindata of "TLSv1.2">,
      "transport": GETDNS_TRANSPORT_TLS
    }
  ],
  "canonical_name": <bindata for example.com.>,
  "just_address_answers":
  [
    {
      "address_data": <bindata for 93.184.216.34>,
      "address_type": <bindata of "IPv4">
    }
  ],
  "replies_full":
  [
     <bindata of 0x415d8180000100010000000107657861...>
  ],
  "replies_tree":
  [
    {
      "additional":
      [
        {
          "do": 0,
          "extended_rcode": 0,
          "rdata":
          {
            "rdata_raw": <bindata of 0x>
          },
          "type": GETDNS_RRTYPE_OPT,
          "udp_payload_size": 4096,
          "version": 0,
          "z": 0
        }
      ],
      "answer":
      [
        {
          "class": GETDNS_RRCLASS_IN,
          "name": <bindata for example.com.>,
          "rdata":
          {
            "ipv4_address": <bindata for 93.184.216.34>,
            "rdata_raw": <bindata of 0x5db8d822>
          },
          "ttl": 75237,
          "type": GETDNS_RRTYPE_A
        }
      ],
      "answer_type": GETDNS_NAMETYPE_DNS,
      "authority": [],
      "canonical_name": <bindata for example.com.>,
      "header":
      {
        "aa": 0,
        "ad": 0,
        "ancount": 1,
        "arcount": 1,
        "cd": 0,
        "id": 16733,
        "nscount": 0,
        "opcode": GETDNS_OPCODE_QUERY,
        "qdcount": 1,
        "qr": 1,
        "ra": 1,
        "rcode": GETDNS_RCODE_NOERROR,
        "rd": 1,
        "tc": 0,
        "z": 0
      },
      "question":
      {
        "qclass": GETDNS_RRCLASS_IN,
        "qname": <bindata for example.com.>,
        "qtype": GETDNS_RRTYPE_A
      }
    }
  ],
  "status": GETDNS_RESPSTATUS_GOOD
}
ArchangeGabriel commented 6 years ago

This individual command work, but not this stubby config:

resolution_type: GETDNS_RESOLUTION_STUB
dns_transport_list:
  - GETDNS_TRANSPORT_TLS
tls_authentication: GETDNS_AUTHENTICATION_REQUIRED
tls_cipher_list: "TLS13-AES-256-GCM-SHA384:TLS13-AES-128-GCM-SHA256:TLS13-CHACHA20-POLY1305-SHA256:EECDH+AESGCM:EECDH+CHACHA20"
tls_query_padding_blocksize: 256 
edns_client_subnet_private : 1
idle_timeout: 10000
listen_addresses:
  - 127.0.0.1@8053
  -  0::1@8053
upstream_recursive_servers:
  - address_data: 80.67.188.188
    tls_port: 443
    tls_pubkey_pinset:
      - digest: "sha256"
        value: WaG0kHUS5N/ny0labz85HZg+v+f0b/UQ73IZjFep0nM=
  - address_data: 2001:913::8
    tls_port: 443
    tls_pubkey_pinset:
      - digest: "sha256"
        value: WaG0kHUS5N/ny0labz85HZg+v+f0b/UQ73IZjFep0nM=
BetaRays commented 6 years ago
  • With OpenSSL versions 1.1.0 or higher, the native DANE functions are used to check pinsets
  • With OpenSSL versions between 1.0.0 and 1.1.0, is uses an (included) version of Viktor Dukhovni's danessl library to check pinsets
  • With OpenSSL versions below 1.0.0 and LibreSSL, self-signed certificates can no longer be used

Doesn't LibreSSL support the OpenSSL 1.1 API since version 2.7?

saradickinson commented 6 years ago

@ArchangeGabriel I agree with @wtoorop - it works for me using OpenSSL 1.1.1-pre5. And your config works just fine for me (once I add upstream_recursive_servers:)

BTW Any self-signed cert should fail if only the tls_auth_name is specified - I've updated the wiki to make that clearer.

ArchangeGabriel commented 6 years ago

Strange. Maybe it works only with OpenSSL 1.1.1+? (missing upstream_recursive_servers was a copy-paste error of course)

ArchangeGabriel commented 6 years ago

And my comment about tls_auth_name was that adding it in addition to tls_pubkey_pinset made LDN server work for me.

wtoorop commented 6 years ago

@ArchangeGabriel Yes I noticed that. That should not have made a difference... Your stubby is definitely using the correct libgetdns.so?

saradickinson commented 6 years ago

@ArchangeGabriel I misunderstood - that does sound odd. FWIIW...it works for me with OpenSSL 1.0.1h too (with just a pinset).

@wtoorop I do notice that for LDN servers using just a hostname fails, and using just a pinset works and using both works..... we should chat about this because the RFC8310 says that if both are provided then validation of both must pass for the authentication to pass.

wtoorop commented 6 years ago

@saradickinson Yes, that is what is happening:

wtoorop commented 6 years ago

@BetaRays getdns is actually testing if SSL_dane_enable() function is available at configure time. I haven't tested with libressl 2.7 myself. At the time I had LibreSSL 2.6.4 which did not have it available.

saradickinson commented 6 years ago

What I meant was src/tools/getdns_query -sLmy 7 @2001:913::8~80.67.188.188 fails (as it should because the spec says the hostname validation MUST use the entire certification path per [RFC5280]).

The fact the combination of hostname and pin works means the logic used to validate the hostname is effected by the presence of the pin, which seems wrong. The spec treats the two mechanisms independently and says that both methods should pass, not that hostname validation can be relaxed if the pinset matches. I know this gives the weird results that hostnames can never be used with self-signed certs but I think that is the strict reading of the spec.

ArchangeGabriel commented 6 years ago

Yes, my stubby is using the right libgetdns.so. I’m getting confused. I can perfectly reproduce Verify failed : TLS - *Failure* - (18) "self signed certificate" using stubby and my above configuration, but not with getdns_query command that all work. Even setting the port to 443 in getdns_query did not change anything. What could explain this difference between Stubby and getdns?

And especially, why does Stubby treats no hostname and just pin as wrong, but pin + hostname as correct?

wtoorop commented 6 years ago

@ArchangeGabriel I'm confused too! On my arch linux vm it works with your package. You can review: ssh root@185.49.141.9 . I've added your ssh pubkey (from github) to ~root/.ssh/authorized_keys

ArchangeGabriel commented 6 years ago

OK, so I’ve tried one by one removing any of the settings, and found the culprit. @wtoorop @saradickinson Can both of you retry after adding tls_cipher_list: "TLS13-AES-256-GCM-SHA384:TLS13-AES-128-GCM-SHA256:TLS13-CHACHA20-POLY1305-SHA256:EECDH+AESGCM:EECDH+CHACHA20" to your stubby configuration?

ArchangeGabriel commented 6 years ago

Hum wrong suspect. I thought it was this because commenting the line made it work. But apparently it was something else, because now it works even without commenting the line. I suspect this might be supporting https://github.com/getdnsapi/stubby/issues/94 somehow…

ArchangeGabriel commented 6 years ago

This is very strange. I’ve tried removing the space after the : in the above line, or adding two spaces, then going back to one space, and got tons of different errors, including the self-signed certificate one.

ArchangeGabriel commented 6 years ago

If I input 4 spaces using the TAB key, then I get Could not schedule query: The context has internal deficiencies. If I then remove two of the spaces (thus leaving two of them), it works again (only one does not). If I then remove one space, it works… or not. Randomly. If I add a random character, save the file, remove the random character, save again, it works… or fails with self-signed… Sigh.

To go further, I even checked the sha256sum of the config file in all setup were it has only 1 space at this position, and got the same one each time. However, I could get failure or not, with this same file.

So then I had the idea of just relaunching stubby without modifying the file. And got success and failures, from one run to another, without even editing the file in between. At this point, I can truely said that I’m lost.

ArchangeGabriel commented 6 years ago

Oh and finally, the tls_auth_name setting has no impact whatsoever.

ArchangeGabriel commented 6 years ago

@wtoorop I’ve copied my full config file to your server (moved the previous one as .back) and can reproduce the issue there. I’m attaching it here too in case @saradickinson want to have a look.

stubby.txt

ArchangeGabriel commented 6 years ago

@wtoorop To not let your vm without a working DNS resolver, I reverted to the normal stubby. To try mine, just systemctl stop stubby, cp /etc/stubby/stubby.yml{.bug,} and stubby -l, then in another terminal run a drill github.com command and see. If it works, kill stubby and restart it. Try again.

ArchangeGabriel commented 6 years ago

Note: this seems to be an issue with the commented lines. Removing all of them get it working all the time. Uncommenting all of upstream_recursive_servers works too. So apparently having commented entries there is not a good idea.

wtoorop commented 6 years ago

@ArchangeGabriel thanks for getting to the bottom of this! I do have a suspicion, so hold on...

wtoorop commented 6 years ago

@ArchangeGabriel Here is the patch to fix this issue:

ArchangeGabriel commented 6 years ago

@wtoorop Do you need me to test it?

Otherwise, this issue should now focus again on LibreSSL 27. ;)

@saradickinson Should I open a new issue for self-signed tls_auth_name considered valid?

wtoorop commented 6 years ago

You don't need to test. It works. I'll have another go with LibreSSL (and otherwise will close as won't fix)

saradickinson commented 6 years ago

@ArchangeGabriel @wtoorop Nice work in tracking this down! @wtoorop lets talk about the self-signed auth name behaviour tomorrow and decide if we need to change it or document it better.

Pheelbert commented 5 years ago

But self-signed certificates do not seem to work currently with OpenSSL 1.1.0. If I use only LDN servers, I get this without setting tls_auth_name: Verify failed : TLS - *Failure* - (18) "self signed certificate".

Only when setting tls_auth_name (which is 80.67.188.188 in this case apparently, and not ns0.ldn-fai.net) did I got it working.

Changing the tls_auth_name to be the same as the address_data fixed the issue for me. I was trying to get some errors by feeding in invalid certificates onto my DNS resolver set up, but I wouldn't get any using only the tls_pubkey_pinset. My specific set up is for academic purposes, but I'm sure someone will benefit from this (and yours especially) comment someday. :)

PS: The fact that self-signed is considered valid is good for my tests, but I doubt this is wanted behavior.. maybe put it as an option?

UPDATE: Actually, I've only been able to get an "certificate has expired" error, all the others are "hostname mismatch". This is probably due to the fact that self-signed certificates aren't handled when using tls_auth_name.

I've found that even if I set my configuration like so: Example in stubby.yml:

# Local DNS server
  - address_data: 192.168.0.52
    tls_port: 853
    tls_auth_name: "garbage"

I still get the expiry error for a specific self-signed certificate I have, so, it seems to show that the expiry check is done before the hostname check.

Also, if the tls_pubkey_pinset is set, self-signed certificates pass successfully even if they are expired (and many other issues).

randomstuff commented 5 years ago

Also, if the tls_pubkey_pinset is set, self-signed certificates pass successfully even if they are expired (and many other issues).

@ Pheelbert , Wgen tls_auth_name is not set that's the correct behavior: RFC831 says that, in this case, there is no PKIX validation (time-based, subectAltName); only one configured SPKI pin must matche somewhere in the certificate chain. The server should event be able to only send the raw public key (not in a cert).