Fix telnet regression of 1104c649aea4a56d59a010c9bfe10131eba68058

[michaelortmann]

Found by: Geo
Patch by: michaelortmann
Fixes:

One-line summary:
Fix telnet regression of 1104c64

Additional description (if needed):

Test cases demonstrating functionality (if applicable):
eggdrop.conf:

set ident-timeout 0
set protect-telnet 1

Before:

[02:01:43] net: connect! sock 4
[02:01:43] dcc_telnet_hostresolved()
[02:01:43] Telnet connection: localhost/36680
[02:01:43] Denied telnet: *, No Access

After:
[02:01:00] net: connect! sock 4
[02:01:00] dcc_telnet_hostresolved()
[02:01:00] Telnet connection: localhost/54118
[02:01:15] pbkdf2 method SHA256 rounds 16000, user 2.209ms sys 0.224ms
[02:01:15] Logged in: testuser (telnet@localhost/54118)
*** testuser joined the party line.

[thommey]

AI review follows

Summary

The PR addresses a real regression introduced by the WebUI commit (1104c649) where dcc_telnet_hostresolved2 was passing dcc[idx].host (the listener's host) to dcc_telnet_got_ident instead of the user's "telnet@" form, denying every telnet user. The author's test case (with ident-timeout 0) is fixed, but the fix breaks the default ident-enabled path, which is currently silently masked because identtimeout is normally >0. Request changes.

Security & defensive coding

• Blocker — src/dcc.c:1452 (and :1439) overwrite dcc[i].host with userhost, breaking the ident-enabled path.
  dcc[i].host is now the "telnet@<host>" string instead of the bare hostname before entering dcc_telnet_hostresolved2. changeover_dcc preserves it, and dcc_telnet_hostresolved2:1368 then does strcpy(dcc[j].host, dcc[i].host) so the IDENT slot also carries "telnet@<host>". When ident completes or times out:

  • dcc_ident:2304 builds snprintf(buf1, …, "%s@%s", uid, dcc[idx].host) → "uid@telnet@<host>".
  • eof_timeout_dcc_ident:2321 builds snprintf(buf, …, "telnet@%s", dcc[idx].host) → "telnet@telnet@<host>".

  Both malformed strings are then passed back into dcc_telnet_got_ident:2374, which sets dcc[i].host and matches "-telnet!" + dcc[i].host against the userfile (get_user_by_host). It will not match any sane -telnet!*@host userfile record, so users hit DCC_NOACCESS with the default ident-timeout 5 / protect-telnet 1 configuration — i.e. the PR fixes one config and regresses the default config.

  Minimal fix: leave dcc[i].host as the bare hostname and build the "telnet@…" form inline for the skip-ident call only, e.g. in dcc_telnet_hostresolved2:

  if (identtimeout <= 0) {
      char uh[7 + UHOSTLEN];
      dcc[i].u.ident_sock = dcc[idx].sock;
      snprintf(uh, sizeof uh, "telnet@%s", dcc[i].host);
      dcc_telnet_got_ident(i, uh);
      return;
  }

  Apply the same idea for the script-listener short-circuit (line 1439) instead of overwriting dcc[i].host.

• Should-fix — src/dcc.c:1362 is the same regression in the j < 0 fallback (not touched by this PR).
  When new_dcc(&DCC_IDENT, …) fails, the code still passes the listener's host (dcc[idx].host) to dcc_telnet_got_ident. Pre-WebUI this was userhost. Rare path, but worth fixing in the same PR since it's the same class of bug.

Behavior risk

• The script-listener path at :1439 now puts "telnet@<host>" into dcc[i].host before check_tcl_listen runs, where previously (pre-WebUI) the bare hostname was in dcc[i].host at that point — dcc_telnet_got_ident only overwrote it after the script branch returned. Tcl listen handlers that read dcc[i].host may see a different value than before. Worth confirming via the listen Tcl docs/expectations.

Readability

• The added userhost variable and the two strlcpy(dcc[i].host, userhost, UHOSTLEN) calls hide a contract change (dcc[i].host semantics shift from "resolved hostname" to "user@host") that's not commented anywhere. The cleaner alternative above avoids this contract change entirely.

Agreement with prior reviewers

No prior reviews or inline comments on this PR yet.

Questions for the author

1. Was the ident-enabled path (ident-timeout 5, default) tested before/after? The test case in the PR description only exercises ident-timeout 0.
2. Any reason to mutate dcc[i].host here rather than computing the "telnet@" form only at the call site that needs it?

[thommey]

see comment above

[michaelortmann]

set protect-telnet 1

all tests with non localhost telnet to the bot, e.g. over ethernet via telnet 192.168.16.3 3333

eggdrop 1.10.1

test with set ident-timeout 5

[21:25:05] net: connect! sock 4
[21:25:05] Telnet connection: isis.home.arpa/57988
[21:25:05] net: eof!(read) socket 10
[21:25:05] EOF ident connection
[21:25:08] pbkdf2 method SHA256 rounds 16000, user 2.235ms sys 0.186ms
[21:25:08] Logged in: testuser (telnet@isis.home.arpa/57988)
*** testuser joined the party line.

test with set ident-timeout 0

[21:28:24] net: connect! sock 4
[21:28:24] Telnet connection: isis.home.arpa/33460
[21:28:27] pbkdf2 method SHA256 rounds 16000, user 2.344ms sys 0.000ms
[21:28:27] Logged in: testuser (telnet@isis.home.arpa/33460)
*** testuser joined the party line.

eggdrop #1905

test with set ident-timeout 5

[21:32:50] net: connect! sock 4
[21:32:50] dcc_telnet_hostresolved()
[21:32:50] Telnet connection: isis.home.arpa/37662
[21:32:50] net: eof!(read) socket 10
[21:32:50] EOF ident connection
[21:32:54] pbkdf2 method SHA256 rounds 16000, user 2.409ms sys 0.278ms
[21:32:54] Logged in: testuser (telnet@telnet@isis.home.arpa/37662)
*** testuser joined the party line.

test with set ident-timeout 0

[21:30:16] net: connect! sock 4
[21:30:16] dcc_telnet_hostresolved()
[21:30:16] Telnet connection: isis.home.arpa/59810
[21:30:19] pbkdf2 method SHA256 rounds 16000, user 2.634ms sys 0.000ms
[21:30:19] Logged in: testuser (telnet@isis.home.arpa/59810)
*** testuser joined the party line.

[thommey]

no ident running, telnet localhost 3333

[21:36:15] Telnet connection: localhost/46450
[21:36:16] EOF ident connection
[21:36:16] Denied telnet: telnet@telnet@localhost, No Access

[thommey]

Superseded by #1905

[vanosg]

But really, superceded by #1908 ;)