eggheads
botlink ssl cert hostname verification bugs
There are 2 bugs in eggdrop when it comes to ssl cert verification.
It fails, when eggdrop cant match the hostname it expects against the hostname (common name) in the cert.
BUG 1 looks like eggdrop is comparing a resolved hostname ("localhost") against the common name ("127.0.0.1") in the cert.
BUG 2 looks like a bug in the share module. Eggdrop will compare an empty string to that common name in the cert, which will always fail.
Debugging is not easy with current debug loggin in eggdrop, so for the following 2 tests i added a line between
https://github.com/eggheads/eggdrop/blob/fd1dff31d798618d75be550506444010d196eec6/src/tls.c#L478
and
https://github.com/eggheads/eggdrop/blob/fd1dff31d798618d75be550506444010d196eec6/src/tls.c#L479
that is:
debug4("######## ip %i cn %s data->host %s match %i", ip, cn, data->host, match);
Both tests were done with current eggdrop version ddcf4d0bb029bcbab5cad95626a5fcd10e02224a (20200814)
======== BUG #1 ========
Create certs with common name "127.0.0.1":
$ openssl req -new -x509 -nodes -keyout BotA.key -out BotA.crt -config ~/projects/eggdrop/ssl.conf
[...]
Common Name (typically your domain name) []:127.0.0.1
[...]
$ openssl req -new -x509 -nodes -keyout BotB.key -out BotB.crt -config ~/projects/eggdrop/ssl.conf
[...]
Common Name (typically your domain name) []:127.0.0.1
[...]
Enable cert verification in BotA.conf and BotB.conf, in my case its 3, because i also allow self signed certs (pls dont allow this in real life ;):
set ssl-verify-dcc 3
set ssl-verify-bots 3
set ssl-verify-clients 3
BotA:
.chaddr BotB 127.0.0.1 +3363
BotB:
.chaddr BotA 127.0.0.1 +3343
I expect eggdrop to match the "127.0.0.1" in the cert to the "127.0.0.1" in the botaddr in the userfile.
BotA:
.link BotB
[...]
[01:33:24] Linking to BotB at 127.0.0.1:3363 ...
[01:33:24] net: open_telnet_raw(): idx 3 host 127.0.0.1 port 3363 ssl 1
[01:33:25] TLS: attempting SSL negotiation...
[...]
[01:33:25] ######## ip -382630416 cn 127.0.0.1 data->host 127.0.0.1 match 1
[...]
[01:33:25] TLS: handshake successful. Secure connection established.
[...]
*** [BotA] Couldn't link to BotB.
[01:33:25] Failed link to BotB.
So far so good.
BotB:
[01:33:24] net: connect! sock 6
[...]
[01:33:24] DNS resolved 127.0.0.1 to localhost
[01:33:24] Telnet connection: localhost/34959
[01:33:24] TLS: attempting SSL negotiation...
[...]
[01:33:25] ######## ip 0 cn 127.0.0.1 data->host localhost match 0
[01:33:25] TLS: certificate validation failed. Certificate subject does not match peer.
[...]
[01:33:25] Lost telnet connection to telnet@localhost/34959
Not good! eggdrop compares "127.0.0.1" to "localhost" and the result is that eggdrops cert verification fails and then bots do not link.
======== BUG 2 ========
I think the problem with BUG 1 is, that it sometimes compares the hostname in the cert not against the botaddr but against the dns lookup result of that botaddr. So i exchanged every "127.0.0.1" with "localhost", and tried again. this time i expect eggdrop to match "localhost" against "localhost".
Create those certs again, this time with "localhost" instead of "127.0.0.1":
$ openssl req -new -x509 -nodes -keyout BotA.key -out BotA.crt -config ~/projects/eggdrop/ssl.conf
[...]
Common Name (typically your domain name) []:localhost
[...]
$ openssl req -new -x509 -nodes -keyout BotB.key -out BotB.crt -config ~/projects/eggdrop/ssl.conf
[...]
Common Name (typically your domain name) []:localhost
[...]
And set the botaddrs accordingly:
BotA:
.chaddr BotB localhost +3363
BotB:
.chaddr BotA localhost +3343
Try to link again:
BotA:
.link BotB
[...]
[01:56:28] Linking to BotB at localhost:3363 ...
[01:56:28] net: open_telnet_raw(): idx 3 host localhost port 3363 ssl 1
[01:56:29] TLS: attempting SSL negotiation...
[...]
[01:56:29] ######## ip 0 cn localhost data->host localhost match 1
[...]
[01:56:29] TLS: handshake successful. Secure connection established.
[...]
[01:56:29] Received challenge from BotB... sending response ...
[01:56:29] Linked to BotB.
[01:56:29] Creating resync buffer for BotB
[01:56:29] Sending user file send request to BotB
[01:56:29] net: connect! sock 10
[01:56:29] TLS: attempting SSL negotiation...
[...]
[01:56:30] Disconnected BotB (aborted userfile transfer).
[01:56:30] Received close notify warning during write
[01:56:30] (Userlist transmit aborted.)
So far so good. Linked to BotB. means, the bots are linked. Yeah. But, i have share-bots and i expect the sharing of the userfile should work as it did before i switched the cert verification on.
BotB:
[01:56:28] net: connect! sock 6
[...]
[01:56:28] DNS resolved 127.0.0.1 to localhost
[01:56:28] Telnet connection: localhost/53835
[01:56:28] TLS: attempting SSL negotiation...
[...]
[01:56:29] ######## ip 0 cn localhost data->host localhost match 1
[...]
[01:56:29] Challenging BotA...
[01:56:29] Linked to BotA.
[01:56:29] Downloading user file from BotA
[01:56:30] TLS: attempting SSL negotiation...
[...]
[01:56:30] ######## ip 0 cn localhost data->host match 0
[01:56:30] TLS: certificate validation failed. Certificate subject does not match peer.
[...]
[01:56:30] Failed connection; aborted userfile transfer.
[01:56:30] Disconnected from: BotA. No reason (lost 1 bot and 1 user).
Not good! The bots linked, but userfile sharing failed. Eggdrop compared the common name in the cert, which is "localhost", to an empty string. I looked into eggdrops code, and the share module simply doesnt set the hostname to compare to. So share bots with cert verification ON, is not possible currently.
My guess was an error in share_ufsend(), which should copy dcc[idx].host, which is something like "telnet@host" to dcc[i].host, but probably only the host part, not the telnet@ part. See https://github.com/eggheads/eggdrop/blob/ddcf4d0bb029bcbab5cad95626a5fcd10e02224a/src/mod/share.mod/share.c#L1200 So i tried to copy the hostname there. While this solved the hostname matching problem on BotB, there is still one mismatch on BotA comming up. It then compares "localhost" to "BotB". Then i found out, hat dcc[i].host is in some case "misused" by the share functionality to store/load the nick of the bot. So its a problem with the design regarding this field dcc[i].host here. It cant be used both for storing a nick and for matching a hostname during cert validation.
Note: Why is fixing this bug so important? There is only one way to secure botlinks, and that is by enabling ssl certification (and dont use self signed certificates).
