Several SSH (RFC 4253 / RFC 4252 / RFC 4254) Compliance Issues in tinyssh

[Zhaodl1]

Hi maintainers, While reviewing tinyssh's SSH Transport Layer, Authentication, and Connection protocol implementation against RFC 4253, RFC 4252, and RFC 4254, we noticed several places where the current behavior appears to differ from the specification. Each item below cites the relevant RFC text alongside the corresponding source code for your reference. We hope this is helpful for improving RFC conformance.

---

1. none Authentication Accepts a Non-Existent User via getpwuid(geteuid())

RFC Reference: RFC 4252 Section 5

"If the requested 'user name' does not exist, the server MAY disconnect, or MAY send a bogus list of acceptable authentication 'method name' values, but never accept any. This makes it possible for the server to avoid disclosing information on which accounts exist. In any case, if the 'user name' does not exist, the authentication request MUST NOT be accepted."

Analysis:
When the daemon is invoked as tinysshnoneauthd (flagnoneauth = 1, set in main_tinysshd.c:172), the none-method branch in packet_auth() at packet_auth.c:82-94 does not verify whether the requested user name exists. Instead of calling getpwnam() on the requested name (as the publickey path does via subprocess_auth()), the code calls getpwuid(geteuid()) at line 89, overwrites packet.name with the process owner's user name at line 91, and jumps directly to the authorized label at line 94. As a result, SSH_MSG_USERAUTH_SUCCESS is sent at line 196 regardless of whether the originally requested user name exists on the system, which differs from the RFC requirement that an authentication request for a non-existent user MUST NOT be accepted.

Source Code Evidence (packet_auth.c):

// packet_auth.c:82-94  (none-auth branch ignores requested user name)
        if (str_equaln((char *) b->buf + pos - len, len, "none")) {
            /*
            if auth. none is enabled get the user from UID
            */
            if (flagnoneauth) {
                struct passwd *pw;
                pkname = "none";
                pw = getpwuid(geteuid());          // process owner, not requested user
                if (!pw) bug();
                str_copyn(packet.name, sizeof packet.name, pw->pw_name);
                b->len = 0;
                b->buf[0] = 0;
                goto authorized;                   // skips user-existence verification
            }
        }

// packet_auth.c:191-198  (USERAUTH_SUCCESS sent unconditionally after goto)
authorized:
    log_i7("auth: ", packet.name, ": ", pkname, " ", (char *) b->buf,
           " accepted");
    buf_purge(b);
    buf_putnum8(b, SSH_MSG_USERAUTH_SUCCESS);
    packet_put(b);
    if (!packet_sendall()) return 0;

---

2. Data Accepted After Receiving SSH_MSG_DISCONNECT

RFC Reference: RFC 4253 Section 11.1

"The sender MUST NOT send or receive any data after this message, and the recipient MUST NOT accept any data after receiving this message."

Analysis:
When an SSH_MSG_DISCONNECT is received, packet_get() returns 0 with errno = 0 at packet_get.c:90-92, which causes the inner packet-processing loop in main_tinysshd.c:380-384 to break. However, the outer for (;;) main loop at main_tinysshd.c:263 continues unconditionally — there is no flag (for example a flagdisconnectreceived) or state transition that marks the connection as disconnected. The termination check at lines 264-268 only fires when channel_iseof() is true, packet.sendbuf is empty, packet.flagchanneleofreceived is set, and packet.flagclosesent is set — none of which is triggered by receiving SSH_MSG_DISCONNECT. The server therefore returns to polling on fd 0, reads subsequent packets via packet_recv(), and dispatches them normally through the switch (b1.buf[0]) at main_tinysshd.c:386. A subsequent SSH_MSG_CHANNEL_DATA is accepted at line 395 and delivered to the child process via packet_channel_recv_data(), which differs from the RFC requirement that the recipient MUST NOT accept any data after receiving a DISCONNECT message.

Source Code Evidence (packet_get.c, main_tinysshd.c):

// packet_get.c:89-92  (DISCONNECT only breaks the inner loop; no state flag set)
    switch (b->buf[0]) {
        case SSH_MSG_DISCONNECT:
            errno = 0;
            return 0;

// main_tinysshd.c:380-384  (inner loop breaks, outer loop continues)
            if (!packet_get(&b1, 0)) {
                if (!errno) break;       // only breaks inner for(;;)
                die_fatal("unable to get packets from network", 0, 0);
            }
            if (b1.len < 1) break; /* XXX */

// main_tinysshd.c:263-268  (outer loop termination does not check for DISCONNECT)
    for (;;) {
        if (channel_iseof())
            if (!packet.sendbuf.len)
                if (packet.flagchanneleofreceived)
                    if (packet.flagclosesent)
                        break;

// main_tinysshd.c:395-397  (CHANNEL_DATA still accepted after DISCONNECT)
                case SSH_MSG_CHANNEL_DATA:
                    if (!packet_channel_recv_data(&b1))
                        die_fatal("unable to handle channel-data", 0, 0);
                    break;

---

3. Null Byte in Identification String Silently Converted to Newline

RFC Reference: RFC 4253 Section 4.2

"The null character MUST NOT be sent."

Analysis:
The server reads the client's identification string one byte at a time via getln() in getln.c. At getln.c:63, a null byte (ch == 0) is silently rewritten to '\n', which then terminates the line at getln.c:65. The truncated string (for example SSH-2.0-evil from a raw SSH-2.0-evil\x00rest\r\n) is returned to packet_hello_receive(), where it passes the length ≥ 6 check and the SSH- prefix check, so packet_hello_receive() returns success and the server proceeds to packet_kex_send() to begin key exchange. No detection, rejection, or log message is produced for the presence of a null character, which differs from the RFC requirement that the null character MUST NOT be sent (the implementation neither enforces this on reception nor signals a protocol error).

Source Code Evidence (getln.c):

// getln.c:54-67  (null byte converted to '\n', line truncated, no rejection)
    xlen = 0;
    for (;;) {
        if (xlen >= xmax - 1) {
            x[xmax - 1] = 0;
            errno = ENOMEM;
            return -1;
        }
        r = getch(fd, &ch);
        if (r != 1) break;
        if (ch == 0) ch = '\n';   // null byte silently rewritten
        x[xlen++] = ch;
        if (ch == '\n') break;    // line terminates at the converted null
    }
    x[xlen] = 0;
    return r;

---

4. Binary Packet Padding Length < 4 Not Validated on Receive

RFC Reference: RFC 4253 Section 6

"There MUST be at least four bytes of padding. The padding SHOULD consist of random bytes. The maximum amount of padding is 255 bytes."

Analysis:
The plaintext receive path packet_get_plain_() in packet_get.c reads the padding_length byte at recvbuf->buf[PACKET_ZEROBYTES + 4] and computes the payload length as packet_length - padding_length - 1 at line 48. The only padding-related validation at line 49 is if (len <= 0) bug_proto(), which rejects only the case where padding consumes the entire packet (padding_length >= packet_length). There is no check that padding_length >= 4. A binary packet with padding_length of 0, 1, 2, or 3 bytes is therefore parsed, its payload is copied into the output buffer at line 50, and it is dispatched to the message-type handler in main_tinysshd.c. The same pattern is present in the encrypted ChaCha20-Poly1305 receive path. This differs from the RFC requirement that there MUST be at least four bytes of padding.

Source Code Evidence (packet_get.c):

// packet_get.c:46-50  (only payload>0 is checked; padding_length>=4 is not)
    /* we have full packet */
    len = packet_length;
    len -= recvbuf->buf[PACKET_ZEROBYTES + 4] + 1;   // subtract padding_length+1
    if (len <= 0) bug_proto();                        // rejects only padding>=packet_length
    buf_put(b, recvbuf->buf + PACKET_ZEROBYTES + 5, len);

---

5. SSH_MSG_CHANNEL_OPEN_FAILURE Uses ADMINISTRATIVELY_PROHIBITED for Unknown Channel Types

RFC Reference: RFC 4254 Section 5.1

"Naturally, if the server does not understand the proposed 'channel type', even if it is a locally defined 'channel type', then the 'reason code' MUST be 0x00000003, as described above, if the 'reason code' is sent."

Analysis:
packet_channel_open() in packet_channel_open.c only accepts the "session" channel type (line 41). For any other channel type, execution falls through to the rejection block at lines 78-94, which constructs an SSH_MSG_CHANNEL_OPEN_FAILURE message. The reason code is hardcoded to SSH_OPEN_ADMINISTRATIVELY_PROHIBITED (value 1) at line 85, rather than SSH_OPEN_UNKNOWN_CHANNEL_TYPE (value 3). The constant SSH_OPEN_UNKNOWN_CHANNEL_TYPE is defined in ssh.h:71 but is not referenced anywhere in the source. The current behavior therefore reports an administratively prohibited condition for an unknown channel type, which differs from the RFC requirement that the reason code MUST be 0x00000003 when the server does not understand the proposed channel type.

Source Code Evidence (packet_channel_open.c, ssh.h):

// packet_channel_open.c:78-91  (rejection path hardcodes reason code 1)
    /* reject channel */
    buf_purge(b2);
    buf_putnum8(
        b2, SSH_MSG_CHANNEL_OPEN_FAILURE); /* byte SSH_MSG_CHANNEL_OPEN_FAILURE */
    buf_putnum32(b2, id);                  /* uint32    recipient channel */
    buf_putnum32(
        b2, SSH_OPEN_ADMINISTRATIVELY_PROHIBITED); /* uint32    reason code */
    buf_putstring(
        b2, "only one 'session' channel allowed");
    buf_putstring(b2, "");
    packet_put(b2);

// ssh.h:69-72  (UNKNOWN_CHANNEL_TYPE defined but never used)
#define SSH_OPEN_ADMINISTRATIVELY_PROHIBITED 1
#define SSH_OPEN_CONNECT_FAILED 2
#define SSH_OPEN_UNKNOWN_CHANNEL_TYPE 3
#define SSH_OPEN_RESOURCE_SHORTAGE 4

---

6. No SSH_MSG_DISCONNECT Sent on Unsupported Service Request or Pre-Auth High-Numbered Message

RFC References: RFC 4253 Section 7.1, RFC 4252 Section 6

"If the server rejects the service request, it SHOULD send an appropriate SSH_MSG_DISCONNECT message and MUST disconnect." (RFC 4253 Section 7.1)

"Message numbers of 80 and higher are reserved for protocols running after this authentication protocol, so receiving one of them before authentication is complete is an error, to which the server MUST respond by disconnecting, preferably with a proper disconnect message." (RFC 4252 Section 6)

Analysis:
Both trigger conditions are detected by tinyssh, but in neither case is an SSH_MSG_DISCONNECT message constructed or sent. When the service name in SSH_MSG_SERVICE_REQUEST is not "ssh-userauth", packet_auth.c:49-50 calls bug_proto(), which routes through bug.h to global_die(111) and _exit(111) without emitting any SSH protocol message. When a high-numbered message (type ≥ 80) arrives during authentication, packet_get() falls through to the default case at packet_get.c:108-115, detects the type mismatch against the expected SSH_MSG_SERVICE_REQUEST/SSH_MSG_USERAUTH_REQUEST, and likewise calls global_die(111). An exhaustive search of the production source shows that the constant SSH_MSG_DISCONNECT (value 1, defined in ssh.h:7) appears only as a receive-side case label in packet_get.c:90; no outgoing-message construction code ever assembles or transmits an SSH_MSG_DISCONNECT, and all SSH_DISCONNECT_* reason codes defined in ssh.h:53-67 are unused. The connection is always terminated via global_die() → global_purge() → _exit(), which differs from the RFC guidance that a proper disconnect message SHOULD be sent (RFC 4253 §7.1) and that the server MUST respond by disconnecting, preferably with a proper disconnect message (RFC 4252 §6).

Source Code Evidence (packet_auth.c, packet_get.c, global.c):

// packet_auth.c:43-50  (unsupported service name -> bug_proto -> global_die, no DISCONNECT)
    if (!packet_getall(b, SSH_MSG_SERVICE_REQUEST)) return 0;
    pos = packetparser_uint8(b->buf, b->len, pos, &ch);
    if (ch != SSH_MSG_SERVICE_REQUEST) bug_proto();
    pos = packetparser_uint32(b->buf, b->len, pos, &len);
    pos = packetparser_skip(b->buf, b->len, pos, len);
    if (!str_equaln((char *) b->buf + pos - len, len, "ssh-userauth"))
        bug_proto();

// packet_get.c:108-116  (unexpected high-numbered message -> global_die, no DISCONNECT)
        default:
            if (x && x != b->buf[0]) {
                char buf1[NUMTOSTR_LEN];
                char buf2[NUMTOSTR_LEN];
                errno = EPROTO;
                log_f4("expected packet type ", numtostr(buf1, x), ", got ",
                       numtostr(buf2, b->buf[0]));
                global_die(111);
            }
            break;

// ssh.h:7,53-67  (SSH_MSG_DISCONNECT and SSH_DISCONNECT_* reason codes defined but never sent)
#define SSH_MSG_DISCONNECT 1                 /* 0x1 */
...
#define SSH_DISCONNECT_SERVICE_NOT_AVAILABLE 7
#define SSH_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED 8
...
#define SSH_DISCONNECT_ILLEGAL_USER_NAME 15

[janmojzis]

Thank you for the detailed report!

• (1) fixed in e6d57ed.
• (2) fixed in 0f07005.
• (3) fixed in a16f3c3.
• (4) fixed in 9206ed0.
• (5) fixed in 7c36be2.
• (6) has not been changed yet

[janmojzis]

(6) fixed in 3ed285b