danfruehauf / NetworkManager-ssh

SSH VPN integration for NetworkManager
Other
253 stars 40 forks source link

Extra SSH options parser too simplistic (Cannot parse spaces successfully) #90

Closed danfruehauf closed 3 years ago

danfruehauf commented 5 years ago

Description of problem: Adding complex extra arguments (advanced --> extra SSH options), with spaces, results in error.

Version-Release number of selected component (if applicable): NetworkManager-ssh-1.2.7-5.fc29.x86_64

How reproducible: Always

Steps to Reproduce:

  1. Configure, with NM, a VPN using SSH
  2. Go to "advanced --> extra SSH options" and add something like "ProxyCommand=arg1 arg2 arg3 ..." (space separated list of parameters)
  3. Try to connect

Actual results: It does not work

Expected results: It should work

Additional info: The error found is that "ssh" does not understand the options.

Considering the code at: https://github.com/danfruehauf/NetworkManager-ssh/blob/master/src/nm-ssh-service.c

Specifically the function:

static void
add_ssh_extra_opts (GPtrArray *args, const char *extra_opts)
{
    gchar      **extra_opts_split;
    gchar      **iter;

    /* Needs to separate arguements nicely */
    extra_opts_split = g_strsplit (extra_opts, " ", 256);
    iter = extra_opts_split;

    /* Ensure it's a valid DNS name or IP address */
    while (*iter) {
        g_message("%s", *iter);
        add_ssh_arg (args, *iter);
        iter++;
    }
    g_strfreev (extra_opts_split);
}

It shows that, apparently, the option separator is simply a space, but, as in the example of ProxyCommand=, there could be options which parameters are a series of elements separated by spaces.

One possibility would be to force to use a different separator, like ":", to this extra options menu, but this would create problems with already working configurations.

An other possibility, would be to introduce a second "space" separator, like "+" (maybe not) or %20, to be processed during the assignment of the extra options to the "ssh" command. Unless "ssh" (not tested yet) will parse "%20" by its own.

This could be taken from (check license, please):

https://github.com/stsquad/easytag/blob/master/src/scan.c

Using the function:

/*
 * Function to replace %20 by a space
 * No need to reallocate
 */
void Scan_Convert_P20_Into_Space (gchar *string)
{
    gchar *tmp, *tmp1;

    while ((tmp=strstr(string,"%20"))!=NULL)
    {
        tmp1 = tmp + 3;
        *(tmp++) = ' ';
        while (*tmp1)
            *(tmp++) = *(tmp1++);
        *tmp = '\0';
    }
}

This code should be run just after the "while (*iter) {" entry of the loop.

Of course, if "ssh" understands %20 this is of no use. Otherwise, please consider to add the second function to parse %20.

ChristofKaufmann commented 3 years ago

I think this can be closed, since extra ssh options have been removed in #98.