Bug 3288 - Ignoring comments at end of config file lines broke ProxyCommand with #-sign in script
Summary: Ignoring comments at end of config file lines broke ProxyCommand with #-sign ...
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 8.5p1
Hardware: amd64 Linux
: P5 normal
Assignee: Damien Miller
URL:
Keywords:
: 3309 (view as bug list)
Depends on:
Blocks: V_8_7
  Show dependency treegraph
 
Reported: 2021-03-26 03:01 AEDT by Tomi Salminen
Modified: 2022-02-25 13:59 AEDT (History)
2 users (show)

See Also:


Attachments
Fix proposal. (1.54 KB, patch)
2021-03-26 08:27 AEDT, Tomi Salminen
no flags Details | Diff
Use a better tokeniser for ssh/sshd_config parsing (58.64 KB, patch)
2021-06-04 13:52 AEST, Damien Miller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomi Salminen 2021-03-26 03:01:18 AEDT
I wondered why my favorite ProxyCommand stopped working after upgrade to 8.5:

Host ??_*         
  User root                                                                                                
  SendEnv TERM=xterm              
  CheckHostIP no          
  ControlPath ~/.ssh/cms/%r@%h:%p
  ControlMaster auto      
  ControlPersist 1m 
  ProxyCommand bash -c 'ssh root@172.16.249.$((1 + ${0%%_*})) nc ${0#[0-9]*_} $1 -q 0' %h %p
  StrictHostKeyChecking accept-new

Checking with verbose the command is clipped:

debug1: Executing proxy command: exec bash -c 'ssh root@172.16.249.$((1 + ${0%_*})) nc ${0

Checking again with strace to make sure the command isn't just clipped by the printer:

execve("/bin/zsh", ["/bin/zsh", "-c", "exec bash -c 'ssh root@172.16.249.$((1 + ${0%_*})) nc ${0"], 0x556526e9b320 /* 47 vars */) = 0

The command is clipped at the # sign. after digging I found this commit to be the culprit:

tree d9cd1cc34e9b0f2b36080069b0bcaa39dd0152e3                                                                                                                                                                                                                                                                
parent b755264e7d3cdf1de34e18df1af4efaa76a3c015                                                                                                                                                                                                                                                              
author dtucker@openbsd.org <dtucker@openbsd.org> Mon Nov 30 05:36:39 2020 +0000                                                                                                                                                                                                                              
committer Damien Miller <djm@mindrot.org> Fri Dec 4 13:42:38 2020 +1100                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                                                             
upstream: Ignore comments at the end of config lines in ssh_config,                                                                                                                                                                                                                                          
                                                                                                                                                                                                                                                                                                             
similar to what we already do for sshd_config.  bz#2320, with & ok djm@                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                                                             
OpenBSD-Commit-ID: bdbf9fc5bc72b1a14266f5f61723ed57307a6db4                                                                                                                                                                                                                                                  
                                                                                                                                                                                                                                                                                                             
diff --git a/readconf.c b/readconf.c                                                                                                                                                                                                                                                                         
index 09b5e086..d60eeacf 100644                                                                                                                                                                                                                                                                              
--- a/readconf.c                                                                                                                                                                                                                                                                                             
+++ b/readconf.c                                                                                                                                                                                                                                                                                             
@@ -1,4 +1,4 @@                                                                                                                                                                                                                                                                                              
-/* $OpenBSD: readconf.c,v 1.342 2020/11/15 22:34:58 djm Exp $ */                                                                                                                                                                                                                                            
+/* $OpenBSD: readconf.c,v 1.343 2020/11/30 05:36:39 dtucker Exp $ */                                                                                                                                                                                                                                        
 /*                                                                                                                                                                                                                                                                                                          
  * Author: Tatu Ylonen <ylo@cs.hut.fi>                                                                                                                                                                                                                                                                      
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland                                                                                                                                                                                                                                           
@@ -1899,7 +1899,7 @@ read_config_file_depth(const char *filename, struct passwd *pw,                                                                                                                                                                                                                        
     int flags, int *activep, int *want_final_pass, int depth)                                                                                                                                                                                                                                               
 {                                                                                                                                                                                                                                                                                                           
        FILE *f;                                                                                                                                                                                                                                                                                             
-       char *line = NULL;                                                                                                                                                                                                                                                                                   
+       char *cp, *line = NULL;                                                                                                                                                                                                                                                                              
        size_t linesize = 0;                                                                                                                                                                                                                                                                                 
        int linenum;                                                                                                                                                                                                                                                                                         
        int bad_options = 0;                                                                                                                                                                                                                                                                                 
@@ -1930,6 +1930,13 @@ read_config_file_depth(const char *filename, struct passwd *pw,                                                                                                                                                                                                                       
        while (getline(&line, &linesize, f) != -1) {                                                                                                                                                                                                                                                         
                /* Update line number counter. */                                                                                                                                                                                                                                                            
                linenum++;                                                                                                                                                                                                                                                                                   
+               /*                                                                                                                                                                                                                                                                                           
+                * Trim out comments and strip whitespace.                                                                                                                                                                                                                                                   
+                * NB - preserve newlines, they are needed to reproduce                                                                                                                                                                                                                                      
+                * line numbers later for error messages.                                                                                                                                                                                                                                                    
+                */                                                                                                                                                                                                                                                                                          
+               if ((cp = strchr(line, '#')) != NULL)                                                                                                                                                                                                                                                        
+                       *cp = '\0';                                                                                                                                                                                                                                                                          
                if (process_config_line_depth(options, pw, host, original_host,                                                                                                                                                                                                                              
                    line, filename, linenum, activep, flags, want_final_pass,                                                                                                                                                                                                                                
                    depth) != 0)

To fix it one would have to keep tabs on when it is inside a parameter and only add the null-termination when it is outside of a parameter.
Comment 1 Tomi Salminen 2021-03-26 08:27:27 AEDT
Created attachment 3489 [details]
Fix proposal.
Comment 2 Tomi Salminen 2021-03-26 08:37:21 AEDT
Pull request https://github.com/openssh/openssh-portable/pull/237
Comment 3 Damien Miller 2021-04-10 19:34:45 AEST
I'm not sure this fix is correct either - it solves your particular case, but doesn't deal with # characters in quoted strings.

Maybe we should just revert the original commit until a comprehensive fix is ready.
Comment 4 Damien Miller 2021-06-04 13:46:14 AEST
*** Bug 3309 has been marked as a duplicate of this bug. ***
Comment 5 Damien Miller 2021-06-04 13:52:47 AEST
Created attachment 3528 [details]
Use a better tokeniser for ssh/sshd_config parsing

I plan to commit this soon - it switches ssh_config and sshd_config parsing to the argv_split() tokeniser, and gives this tokeniser the ability to terminate when it encounters an unquoted '#' character.

This should fix this bug, but also improve quote handling in configuration files generally. Note that the tokeniser is not used for command-line arguments (e.g. ProxyCommand), so there should be no behaviour change there other than fixing the '#' truncation regression.
Comment 6 Damien Miller 2021-06-08 17:18:43 AEST
this diff, with a few bugfixes has been committed and will be in openssh-8.7
Comment 7 Damien Miller 2022-02-25 13:59:08 AEDT
closing bugs resolved before openssh-8.9