SSH server comparison the channel id from INT_MAX for a signed value. #786

asingh1508 · 2025-03-06T06:37:45Z

The problem is related to the checking of what is called a sender channel ID (which is set to a unsigned INT random number by the originator of the connection). However, in the versions mentioned above the daemon is making a comparison using INT_MAX for a signed value. This means that valid channel numbers are being rejected and the connection is not established.

norrisjeremy · 2025-03-06T20:02:22Z

Hi @asingh1508,

Can you provide more details as to what problem this is solving?
Is this an issue with some specific SSH server implementation that fails with channel id values that exceed INT_MAX?

Thanks,
Jeremy

asingh1508 · 2025-03-07T05:32:14Z

@norrisjeremy

We're describing a bug in OpenSSH where the OpenSSH daemon (sshd) incorrectly handles sender channel IDs due to an issue with signed vs. unsigned integer comparisons.

Understanding the Issue:

In SSH, each side of a connection assigns a sender channel ID, which is an unsigned integer (typically a random number).
OpenSSH daemon (sshd) checks this channel ID when establishing a connection.

However, in the affected versions, sshd mistakenly compares the sender channel ID using INT_MAX (which is for signed integers).
This means that if the sender channel ID is greater than INT_MAX (i.e., exceeds 2147483647 on a 32-bit system), the check fails.
As a result, valid connections are being rejected.

Impact:
This bug can prevent SSH connections from being established in certain cases.
It might affect systems with high activity or specific configurations where channel IDs can exceed the signed integer limit.

OpenSSH server code:
/* Parse a channel ID from the current packet */
static int
channel_parse_id(struct ssh *ssh, const char *where, const char *what)
{
u_int32_t id;
int r;

if ((r = sshpkt_get_u32(ssh, &id)) != 0) {
        error("%s: parse id: %s", where, ssh_err(r));
        ssh_packet_disconnect(ssh, "Invalid %s message", what);
}
**if (id > INT_MAX) {**
        error("%s: bad channel id %u: %s", where, id, ssh_err(r));
        ssh_packet_disconnect(ssh, "Invalid %s channel id", what);
}
return (int)id;

}

asingh1508 · 2025-03-07T05:47:26Z

Hi @asingh1508,

Can you provide more details as to what problem this is solving? Is this an issue with some specific SSH server implementation that fails with channel id values that exceed INT_MAX?

Thanks, Jeremy

Is this an issue with some specific SSH server implementation that fails with channel id values that exceed INT_MAX?

Thanks,

Its not specific SSH server. This issue will happend all SSH to servers that are running OpenSSH daemon.

norrisjeremy

Hi @asingh1508,

Thank you for providing information on how OpenSSH rejects channels with an id that exceeds INT_MAX.

I think a cleaner fix would be something like this: can you confirm and if so, adjust this PR?

diff --git a/src/main/java/com/jcraft/jsch/Channel.java b/src/main/java/com/jcraft/jsch/Channel.java
index 6fb79a7..467d591 100644
--- a/src/main/java/com/jcraft/jsch/Channel.java
+++ b/src/main/java/com/jcraft/jsch/Channel.java
@@ -139,6 +139,8 @@ public abstract class Channel {
   Channel() {
     synchronized (pool) {
       id = index++;
+      // OpenSSH rejects channels with an id that exceeds INT_MAX
+      index &= Integer.MAX_VALUE;
       pool.addElement(this);
     }
   }

Thanks,
Jeremy

src/main/java/com/jcraft/jsch/Channel.java

norrisjeremy

LGTM, thanks!

asingh1508 · 2025-03-07T16:46:52Z

LGTM, thanks!

@norrisjeremy Who will this pull request merged in master branch ?

norrisjeremy · 2025-03-07T16:47:48Z

LGTM, thanks!

@norrisjeremy Who will this pull request merged in master branch ?

We will need to wait for @mwiede.

asingh1508 · 2025-03-07T16:49:43Z

We will need to wait for @mwiede.

Thanks

norrisjeremy

Actually, I am reviewing OpenSSH code more closely and more changes may be required.
I will respond back once I have completed a more thorough review.

asingh1508 · 2025-03-07T16:57:16Z

Actually, I am reviewing OpenSSH code more closely and more changes may be required. I will respond back once I have completed a more thorough review.

sure

norrisjeremy · 2025-03-07T17:12:31Z

Hi @asingh1508,

Actually, after reviewing further, I'm not sure if I believe any changes are actually needed.
I believe the code you link is related to handling of the channel id value that the OpenSSH server assigns to the channel, and not the id that the JSch client assigns to the channel.

All SSH channels consist of two channel id values: one assigned by the client and one assigned by the server (see RFC-4254 Section 5.1).

The code you cited in OpenSSH appears to be related to handling of it's locally assigned id value that it associates with the channel and not the id value that JSch associates with the channel.

Have you actually experienced a failure that can somehow be traced back to JSch channel id values exceeding 0x7fffffff (INT_MAX), or did you submit this based purely on code inspection?

Thanks,
Jeremy

asingh1508 · 2025-03-07T17:33:07Z

Have you actually experienced a failure that can somehow be traced back to JSch channel id values exceeding 0x7fffffff (INT_MAX),

I had experienced a failure in customer machine where JSch channel id values exceeding 0x7fffffff (INT_MAX),

you submit this based purely on code inspection

@norrisjeremy
I had experienced ssh connection failure in customer machine where JSch channel id values exceeding 0x7fffffff INT_MAX),.
This was reason i inspectioned the code and summit this pr.

norrisjeremy · 2025-03-07T17:36:39Z

Have you actually experienced a failure that can somehow be traced back to JSch channel id values exceeding 0x7fffffff (INT_MAX),

I had experienced a failure in customer machine where JSch channel id values exceeding 0x7fffffff (INT_MAX),

you submit this based purely on code inspection

@norrisjeremy I had experienced ssh connection failure in customer machine where JSch channel id values exceeding 0x7fffffff INT_MAX),. This was reason i inspectioned the code and summit this pr.

Can you share more details as to how you arrived at the conclusion that the failure you experienced is related to a JSch channel id exceeding 0x7fffffff (INT_MAX)?
Because I'm not convinced by my further examination of the OpenSSH code that it should result in a failure.

asingh1508 · 2025-03-07T17:48:09Z

Have you actually experienced a failure that can somehow be traced back to JSch channel id values exceeding 0x7fffffff (INT_MAX),

I had experienced a failure in customer machine where JSch channel id values exceeding 0x7fffffff (INT_MAX),

you submit this based purely on code inspection

@norrisjeremy I had experienced ssh connection failure in customer machine where JSch channel id values exceeding 0x7fffffff INT_MAX),. This was reason i inspectioned the code and summit this pr.

Can you share more details as to how you arrived at the conclusion that the failure you experienced is related to a JSch channel id exceeding 0x7fffffff (INT_MAX)? Because I'm not convinced by my further examination of the OpenSSH code that it should result in a failure.

@norrisjeremy
public void connect(int connectTimeout) throws JSchException{
Session _session=getSession();
if(!_session.isConnected()){
throw new JSchException("session is down");
}
this.connectTimeout=connectTimeout;
try{
Buffer buf=new Buffer(100);
Packet packet=new Packet(buf);
// send
// byte SSH_MSG_CHANNEL_OPEN(90)
// string channel type //
// uint32 sender channel // 0
// uint32 initial window size // 0x100000(65536)
// uint32 maxmum packet size // 0x4000(16384)
packet.reset();
buf.putByte((byte)90);
buf.putString(this.type);
buf.putInt(this.id)
buf.putInt(this.lwsize);
buf.putInt(this.lmpsize);
_session.write(packet);

@norrisjeremy Jsch library is sending the channel id (signed value) In packet SSH_MSG_CHANNEL_OPEN to openssh server.

Server code where channel id parsed from packet (get unsigend value of channed id) and comapre from INT_MAX.

/* -- protocol input */

/* Parse a channel ID from the current packet */
static int
channel_parse_id(struct ssh *ssh, const char *where, const char *what)
{
u_int32_t id;
int r;

if ((r = sshpkt_get_u32(ssh, &id)) != 0) {
        error("%s: parse id: %s", where, ssh_err(r));
        ssh_packet_disconnect(ssh, "Invalid %s message", what);
}
if (id > INT_MAX) {
        error("%s: bad channel id %u: %s", where, id, ssh_err(r));
        ssh_packet_disconnect(ssh, "Invalid %s channel id", what);
}
return (int)id;

norrisjeremy · 2025-03-07T18:35:16Z

Hi @asingh1508,

But I do not believe the code you cited is actually the code that is used to parse the SSH2_MSG_CHANNEL_OPEN that JSch sends.
I urge you to more carefully read and review the OpenSSH code, in particular the code in the server_input_channel_open() function present in serverloop.c, that is responsible for parsing and handling the remote channel id from the sender (stored in the rchan variable).
Additionally, I have reviewed a couple other SSH implementations and none them appear to clamp the id value they use in the SSH2_MSG_CHANNEL_OPEN to ensure that it doesn't exceed INT_MAX, so I have a difficult time believing this is actually an issue.

Thanks,
Jeremy

asingh1508 · 2025-03-07T19:18:25Z

server_input_channel_open

static int
server_input_channel_open(int type, u_int32_t seq, struct ssh *ssh)
{
Channel *c = NULL;
char *ctype = NULL;
const char *errmsg = NULL;
int r, reason = SSH2_OPEN_CONNECT_FAILED;
u_int rchan = 0, rmaxpack = 0, rwindow = 0;

    if ((r = sshpkt_get_cstring(ssh, &ctype, NULL)) != 0 ||
        (r = sshpkt_get_u32(ssh, &rchan)) != 0 ||
        (r = sshpkt_get_u32(ssh, &rwindow)) != 0 ||
        (r = sshpkt_get_u32(ssh, &rmaxpack)) != 0)
            sshpkt_fatal(ssh, r, "%s: parse packet", __func__);
    debug("%s: ctype %s rchan %u win %u max %u", __func__,
        ctype, rchan, rwindow, rmaxpack);

    **if (rchan > INT_MAX) {
            error("%s: invalid remote channel ID", __func__);**
    } else if (strcmp(ctype, "session") == 0) {
            c = server_request_session(ssh);
    } else if (strcmp(ctype, "direct-tcpip") == 0) {
            c = server_request_direct_tcpip(ssh, &reason, &errmsg);
    } else if (strcmp(ctype, "[email protected]") == 0) {
            c = server_request_direct_streamlocal(ssh);
    } else if (strcmp(ctype, "[email protected]") == 0) {
            c = server_request_tun(ssh);
    }
    if (c != NULL) {
            debug("%s: confirm %s", __func__, ctype);
            c->remote_id = (int)rchan;
            c->have_remote_id = 1;
            c->remote_window = rwindow;
            c->remote_maxpacket = rmaxpack;

@norrisjeremy rchan is u_int type variable but it is comparing with if (rchan > INT_MAX).
@norrisjeremy Can you please check for openssh-8.1p1 and openssh-8.0p1 version ?

norrisjeremy

Hi @asingh1508,

Ok, I now see that this was a bug that existed until OpenSSH 8.2, when it was fixed in openssh/openssh-portable@0ecd20b.

I would encourage you in the future when opening issues or PRs with us, to include as many details as possible, to help us understand the conditions in which you were encountering as issue.

For example, pinpointing the fact that this impacted older OpenSSH releases was crucial in me understanding why this workaround is needed.

Thanks,
Jeremy

src/main/java/com/jcraft/jsch/Channel.java

sonarqubecloud · 2025-03-07T20:08:02Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

asingh1508 · 2025-03-07T20:28:32Z

@norrisjeremy Buid with all tests passed succesfully. Please verify and approved..

norrisjeremy

Thanks for you patience.
I think this looks good now.

asingh1508 added 6 commits March 6, 2025 12:02

Update Channel.java

9bf954b

Update Channel.java

a1c1365

Update Channel.java

0e2b2ce

Update Channel.java

36e8e9b

Update Channel.java

6f71192

Update Channel.java

2184ebc

asingh1508 added 14 commits March 7, 2025 11:32

Update Channel.java

2f9540a

Update Channel.java

5a48ba2

Update Channel.java

e3deae3

Update Channel.java

70c7de0

Update Channel.java

cabf8d5

Update Channel.java

c229459

Update Channel.java

99b76ee

Update Channel.java

2a7567f

Update Channel.java

8d8dfa3

Update Channel.java

3eae5b2

Update Channel.java

baec439

Update Channel.java

e0e0c03

Update Channel.java

521edee

Update Channel.java

d1b080a

norrisjeremy suggested changes Mar 7, 2025

View reviewed changes

Update Channel.java

ab55c8f

norrisjeremy suggested changes Mar 7, 2025

View reviewed changes

src/main/java/com/jcraft/jsch/Channel.java Show resolved Hide resolved

Update Channel.java

34621f9

norrisjeremy suggested changes Mar 7, 2025

View reviewed changes

src/main/java/com/jcraft/jsch/Channel.java Outdated Show resolved Hide resolved

Update Channel.java

27eff64

norrisjeremy approved these changes Mar 7, 2025

View reviewed changes

norrisjeremy suggested changes Mar 7, 2025

View reviewed changes

asingh1508 closed this Mar 7, 2025

asingh1508 reopened this Mar 7, 2025

norrisjeremy approved these changes Mar 7, 2025

View reviewed changes

norrisjeremy suggested changes Mar 7, 2025

View reviewed changes

src/main/java/com/jcraft/jsch/Channel.java Outdated Show resolved Hide resolved

asingh1508 added 3 commits March 8, 2025 01:13

Update Channel.java

c2e12ef

Update Channel.java

33c26eb

Update Channel.java

214369d

norrisjeremy approved these changes Mar 7, 2025

View reviewed changes

src/main/java/com/jcraft/jsch/Channel.java Show resolved Hide resolved

asingh1508 added 3 commits March 8, 2025 01:21

Update Channel.java

47b7fdf

Update Channel.java

398408a

Update Channel.java

c7ccf28

norrisjeremy suggested changes Mar 7, 2025

View reviewed changes

src/main/java/com/jcraft/jsch/Channel.java Show resolved Hide resolved

asingh1508 added 3 commits March 8, 2025 01:31

Update Channel.java

7e00fa2

Update Channel.java

46c66fe

Update Channel.java

29bcba6

norrisjeremy approved these changes Mar 7, 2025

View reviewed changes

mwiede merged commit 793753a into mwiede:master Mar 8, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SSH server comparison the channel id from INT_MAX for a signed value. #786

SSH server comparison the channel id from INT_MAX for a signed value. #786

asingh1508 commented Mar 6, 2025

norrisjeremy commented Mar 6, 2025

asingh1508 commented Mar 7, 2025

asingh1508 commented Mar 7, 2025 •

edited

Loading

norrisjeremy left a comment

norrisjeremy left a comment

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025

norrisjeremy left a comment

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025 •

edited

Loading

norrisjeremy left a comment

sonarqubecloud bot commented Mar 7, 2025

asingh1508 commented Mar 7, 2025

norrisjeremy left a comment

SSH server comparison the channel id from INT_MAX for a signed value. #786

SSH server comparison the channel id from INT_MAX for a signed value. #786

Conversation

asingh1508 commented Mar 6, 2025

norrisjeremy commented Mar 6, 2025

asingh1508 commented Mar 7, 2025

asingh1508 commented Mar 7, 2025 • edited Loading

norrisjeremy left a comment

Choose a reason for hiding this comment

norrisjeremy left a comment

Choose a reason for hiding this comment

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025

norrisjeremy left a comment

Choose a reason for hiding this comment

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025

norrisjeremy commented Mar 7, 2025

asingh1508 commented Mar 7, 2025 • edited Loading

norrisjeremy left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 7, 2025

Quality Gate passed

asingh1508 commented Mar 7, 2025

norrisjeremy left a comment

Choose a reason for hiding this comment

asingh1508 commented Mar 7, 2025 •

edited

Loading

asingh1508 commented Mar 7, 2025 •

edited

Loading