trzsz / trzsz-ssh

trzsz-ssh ( tssh ) is an ssh client designed as a drop-in replacement for the openssh client. It aims to provide complete compatibility with openssh, mirroring all its features, while also offering additional useful features. Such as login prompt, batch login, remember password, automated interaction, trzsz, zmodem(rz/sz), udp mode like mosh, etc.
https://trzsz.github.io/ssh
MIT License
1.74k stars 102 forks source link

hash in ssh_config value cause truncate #92

Closed warjiang closed 7 months ago

warjiang commented 8 months ago

the ssh config as follow:

Host wap1 # there is a proxy available for this one...
    UserKnownHostsFile /dev/null
    StrictHostKeyChecking no
    Port 22
    User user_name
    IdentityFile path_to_private_key
    ProxyCommand ncat --proxy-type socks5 --proxy-auth 'user_name:pwd@#123_^&*' --proxy proxy_ip:proxy_port %h %p

value after '#' in line of ProxyCommand will be truncate. more detail can refer following links: https://github.com/trzsz/ssh_config/pull/2

warjiang commented 8 months ago

@lonnywong plz help

lonnywong commented 8 months ago

@warjiang Does openssh work properly?

warjiang commented 8 months ago

@warjiang Does openssh work properly?

Yeah, it works

lonnywong commented 8 months ago

openssh supports # without ' and ":

Host test
    LocalCommand echo Hello# World
    PermitLocalCommand yes

Need to study how openssh does it. But I've been busy recently.

warjiang commented 8 months ago

@lonnywong

First i should admit that i have little knowledge of openssh, even bunch of knowledge came from daily usage 🤣 .

May be it's not right time to talk further, you can recheck this when you are free. Feel free to review it 😄 . What i want to express is that, the ssh_config pkg cannot parse # char in side string literal.

The test code as follow:

func main() {
    configData := `Host test
    LocalCommand echo Hello'#' World
    PermitLocalCommand yes`
    cfg, err := ssh_config.Decode(strings.NewReader(configData))
    if err != nil {
        panic(err)
    }
    v, err := cfg.Get("test", "LocalCommand")
    if err != nil {
        panic(err)
    }
    debug("value is %s", v)
}

before fixed: image

after fixed: image

Apart from that, i've tested in openssh client, i mock a ssh config file like following:

Host wap1
    LocalCommand echo Hello'#' World
    PermitLocalCommand yes

image

it's equla to command echo Hello'#' World no echo Hello'

warjiang commented 8 months ago

what i do is that handel the # within string literal, it's not enough, but it can work currentlly. After a brief view of openssh: https://github.com/openssh/openssh-portable/blob/master/readconf.c, image openssh lib also handle the ssh config with ast like tokenizer, lexer and so on. But the token is much more than ssh_config 😭 May we can talk it further in the futhre when free.

lonnywong commented 8 months ago

@warjiang How about we make some changes like this:

Host comment
    Key1 = Value1 # This is part of the value, not a comment
    Key2   Value2 # This is a comment, not part of the value

The value of Key1 is Value1 # This is part of the value, not a comment. And the value of Key2 is Value2.

warjiang commented 8 months ago

@warjiang How about we make some changes like this:

Host comment
    Key1 = Value1 # This is part of the value, not a comment
    Key2   Value2 # This is a comment, not part of the value

The value of Key1 is Value1 # This is part of the value, not a comment. And the value of Key2 is Value2.

we should distinguish scene of #

so @lonnywong should change code as following, if you want # as part of value

Host comment
    Key1 = 'Value1 # This is part of the value, not a comment'
    Key2   Value2 # This is a comment, not part of the value
lonnywong commented 8 months ago

It's not clear whether the value 'Value1 # This is part of the value, not a comment' contains single quotes or not.

I think = means there is no comment at the end, which is simple and easy to distinguish. It's not exactly the same as openssh, but it's worth it.

warjiang commented 8 months ago

i've made serval test cases it in my local enviroment

Host wap
    LocalCommand = echo Value1 # This is part of the value, not a comment
    PermitLocalCommand yes

image

Host wap
    LocalCommand echo Value1 # This is part of the value, not a comment
    PermitLocalCommand yes

image

Conculsion: whether use equal sign, it turns out the same effect.

if wrap # with single quote as follow:

Host wap
    LocalCommand echo 'Value1 # This is part of the value, not a comment'
    PermitLocalCommand yes

image

lonnywong commented 8 months ago

I mean we change the code to do that.

warjiang commented 8 months ago

It's not clear whether the value 'Value1 # This is part of the value, not a comment' contains single quotes or not.

I think = means there is no comment at the end, which is simple and easy to distinguish. It's not exactly the same as openssh, but it's worth it.

maybe you are right, but it will casued some extra knowledge of another version of ssh config (like config of tssh) 😆 .

warjiang commented 8 months ago

I mean we change the code to do that.

i can try it, but maybe we can use option to let user to choose whehter enable this feature.

warjiang commented 8 months ago

how about another im tools like wechat or tg? may be we can talk it for further. the work under trzsz is very useful for me, maybe i can take part in these project in deep. 🤔

lonnywong commented 8 months ago

i can try it, but maybe we can use option to let user to choose whehter enable this feature.

I think most people will not use = in ssh config, even fewer use = and add comments at the end of the line.

We just need to document the new features of = in the new version.

how about another im tools like wechat or tg? may be we can talk it for further. the work under trzsz is very useful for me, maybe i can take part in these project in deep. 🤔

There's a QQ group at the bottom of README.

warjiang commented 8 months ago

I think most people will not use = in ssh config, even fewer use = and add comments at the end of the line.

We just need to document the new features of = in the new version.

that's true~ i'll try it later 🔧

warjiang commented 8 months ago

I've implemented this feature by add lexRReservedvalue state. After detect equal, it will go to state of checking lexRReservedvalue. image

warjiang commented 8 months ago

I've implemented this feature by add lexRReservedvalue state. After detect equal, it will go to state of checking lexRReservedvalue. image

for the ssh config as following:

Host comment
    Key1 = Value1 # This is part of the value, not a comment
    Key2   Value2 # This is a comment, not part of the value

the test case like following: image

lonnywong commented 8 months ago

I will write the documentation and update it this weekend.

lonnywong commented 7 months ago

done https://github.com/trzsz/trzsz-ssh/commit/a6c64a61aefa33c74d5ec7bdd6b211ec2713ce81