Critical security flaws in Nagios NRPE client/server crypto

in CC to: grok full disclosure, bugtraq

TL;DR - DO NOT USE NRPE'S SSL IMPLEMENTATION!

Dear Nagios developers,

It's been a couple of years since I've had a look at NRPE, the remote
monitoring agent distributed with Nagios. Back then we've exclusively
used NRPE on unrouted dedicated monitoring vLANS.

I've recently been implementing monitoring with Icinga2 and been looking
up NRPE again. So I read through your source code and "documentation".
Here's my impression of your work.

• Cryptography -
  README.SSL:
  ```
  The Encryption is done using a set encryption routine of
  AES-256 Bit Encryption using SHA and Anon-DH. This encrypts
  all traffic using the NRPE sockets from the client to the server.
  ```

This is simply not true.

src/nrpe.c L259 and src/client_check.c L168:
```
SSL_CTX_set_cipher_list(ctx,"ADH");
```

Setting the cipherstring to "ADH" allows for a multitude of possible
cipherstrings, depending on the OpenSSL configuration on the system and
the configuration shipped by the operating system distribution.
Furthermore, a quick peek into the OpenSSL wiki [0] or any textbook [1]
on the subject would have shown you that anonymous diffie hellman does
not provide any kind of authentication, and is thus, vulnerable to
(easily mounted) man-in-the-middle attacks.

src/nrpe.c L256 and src/client_check.c L145:
```
SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
```

You're excluding SSLv2 and SSLv3, still leaving two broken protocols in
there: TLSv1.0 and TLSv1.1.

README.SSL
```
Since we are using Anon-DH this allows for an encrypted
SSL/TLS Connection without using pre-generated keys or
certificates. The key generation information used by the
program to dynaically create keys on daemon startup can be found
in the dh.h file in the nrpe src directory. This file was created
using the command:

openssl dhparam -C 512
```

512bit DH has been broken in_the_real_world for a couple of years.
Current best practices recommend 3k+ [2]. Further more regenerating
diffie-hellman parameters with autotools may not really improve security.

As to the note on the developer not being sure if there would be
restrictions on the export: Yes. If you live in oppressive regimes there
are restrictions. i.e. for the US of A put a cryptography export notice
there.

The aforementioned implementation of "cryptography" does not provide any
security other than security by obscurity. It's completely absurd and
system administrators without proper knowledge might actually deploy
this, without any warning on the security implications caused by a
fisher-price-my-first-crypto implementation. The first lesson in any
course on cryptography is always “do not implement yourself!”.

• Code quality -
  As with the Nagios core, the overall code quality is just horrendous.
  I've not had time to check thoroughly but from just scrolling though I
  see a wide variety of format string vulnerabilities and bad coding
  practices.

• Bashing doesn't help, after all it's FOSS, right? -
  I've patched some parts of the code to explicitly exclude anything else
  than TLSv1.2 and use a sane default cipher string loaded from a
  configuration file. But there's still much work to do to enable
  something with low overhead like ECDHE-ECDSA - because, yes, in this
  case, for good security you'll need certificate handling and proper
  implemented PKI. I'm not sure on how to proceed, I see the following
  three options:

.) Do nothing and ignore security completely
.) Completely remove the mentioned SSL code parts and only point to stunnel
.) Implement proper PKI with current cryptography and update the project
accordingly (I’d help with that - but I’m not sure if thats even reasonable)

Sincerely,
Aaron Zauner

[0] - http://wiki.openssl.org/index.php/Diffie_Hellman
[1] - https://www.schneier.com/book-ce.html
[1] - https://www.cl.cam.ac.uk/~rja14/book.html
[2] - http://www.keylength.com