Issues/431/dev: configuration file implementation #501

Open
wants to merge 7 commits into
from

Conversation

Projects
None yet
5 participants
@earlchew

earlchew commented Feb 9, 2014

No description provided.

earlchew added some commits Feb 9, 2014

[mosh] Add configuration file parsing
#431

Add class ConfigFile that is based on ssh_config(5) and have it parse
~/.mosh/config that will be used to store global and per-host configuration.

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
[mosh] Use --test to trigger configuration unit test
#431

Add undocumented --test option that will trigger the unit test in
the ConfigFile class.

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
[mosh] Refactor fake_proxy to simplify configuration file processing
#431

Move fake_proxy to before the main option processing to simplify
configuration file processing.

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
[mosh] Perform command line initialisation as early as possible
#431

Move command line initialisation before considering any of the command line
options to simplify subsequent configuration file operations.

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
[mosh] Initialize options only if not provided on the command line
#431

Declare option variables as undef, and only provide a default value
if options were not provided on the command line in preparation for
obtaining values from a configuration file.

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
[mosh] Use entries from the configuration file
#431

Initialize options from the configuration file if not present on the
command line.

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
@andersk

This comment has been minimized.

Show comment Hide comment
@andersk

andersk Feb 10, 2014

Member

I’m not really a fan of the idea of more than doubling the mosh wrapper script just to add configuration file parsing—surely there must be a simpler way. Also, we should stop littering HOME with new dotfiles; that’s what the XDG base directory specification is for.

Member

andersk commented Feb 10, 2014

I’m not really a fan of the idea of more than doubling the mosh wrapper script just to add configuration file parsing—surely there must be a simpler way. Also, we should stop littering HOME with new dotfiles; that’s what the XDG base directory specification is for.

@earlchew

This comment has been minimized.

Show comment Hide comment
@earlchew

earlchew Feb 10, 2014

I chose to follow the format for ssh_config(5) since mosh is already related to ssh(1). Since I already have to manipulate .ssh/config in order to use mosh(1), it seemed reasonable to follow that format for .mosh/config.

There are other ways to parse configuration files in Perl (eg Config::Tiny) which would require less code in mosh(1), but increase coupling to other Perl modules. Some of these are not commonly included in operating system distributions.

Even if XDG is desirable, I think it's best not to make XDG mandatory so that mosh can continue to support non-desktop environments, particularly those in which ssh(1) plays.

I chose to follow the format for ssh_config(5) since mosh is already related to ssh(1). Since I already have to manipulate .ssh/config in order to use mosh(1), it seemed reasonable to follow that format for .mosh/config.

There are other ways to parse configuration files in Perl (eg Config::Tiny) which would require less code in mosh(1), but increase coupling to other Perl modules. Some of these are not commonly included in operating system distributions.

Even if XDG is desirable, I think it's best not to make XDG mandatory so that mosh can continue to support non-desktop environments, particularly those in which ssh(1) plays.

@andersk

This comment has been minimized.

Show comment Hide comment
@andersk

andersk Feb 23, 2014

Member

The XDG specification has no dependence on desktop environments. (“If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.”)

Member

andersk commented Feb 23, 2014

The XDG specification has no dependence on desktop environments. (“If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.”)

[mosh] Support XDG_CONFIG_HOME
Search for configuration file in $HOME/.mosh/config, then
$XDG_CONFIG_HOME/mosh/config and finally $HOME/.config/mosh/config.

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
@earlchew

This comment has been minimized.

Show comment Hide comment
@earlchew

earlchew Feb 23, 2014

All right. I've reworked the configuration file scanning to include support for XDG_CONFIG_HOME.

All right. I've reworked the configuration file scanning to include support for XDG_CONFIG_HOME.

@cgull

This comment has been minimized.

Show comment Hide comment
@cgull

cgull May 20, 2015

Member

I'm a little uncomfortable with adding an ad-hoc config file parser in a security-sensitive program, and I'm not sure SSH is a good example to follow, perhaps YAML might be better here.

I echo @andersk in saying surely there must be a simpler way, though I don't know what it is either.

Member

cgull commented May 20, 2015

I'm a little uncomfortable with adding an ad-hoc config file parser in a security-sensitive program, and I'm not sure SSH is a good example to follow, perhaps YAML might be better here.

I echo @andersk in saying surely there must be a simpler way, though I don't know what it is either.

@glensc

This comment has been minimized.

Show comment Hide comment
@glensc

glensc May 20, 2015

testing ConfigFile package inside main mosh script? does not seem like super idea.

maybe use existing CPAN module to parse the config, and if want to avoid depending on CPAN modules in distribution (why?), could use some other module that inlines needed CPAN modules during build process. Can't give any reference which tool it is, as haven't used any myself, but there exists, maybe even several projects.

glensc commented May 20, 2015

testing ConfigFile package inside main mosh script? does not seem like super idea.

maybe use existing CPAN module to parse the config, and if want to avoid depending on CPAN modules in distribution (why?), could use some other module that inlines needed CPAN modules during build process. Can't give any reference which tool it is, as haven't used any myself, but there exists, maybe even several projects.

@cgull cgull changed the title from Issues/431/dev to Issues/431/dev: configuration file implementation Jun 8, 2015

@ekacnet

This comment has been minimized.

Show comment Hide comment
@ekacnet

ekacnet Jul 26, 2015

FYI my PR. is similar to this but:
I'm only adding ~130 lines, most of commits are cosmetics and are meant for reducing the risk of forgetting options (ie. by not having them spread all across the script).

It's really handy to have the capacity to set some options like predict or the path to the server command, it's also handy if you have to use a custom ssh command as I do when I do port knocking or use 2FA like yubikeys

ekacnet commented Jul 26, 2015

FYI my PR. is similar to this but:
I'm only adding ~130 lines, most of commits are cosmetics and are meant for reducing the risk of forgetting options (ie. by not having them spread all across the script).

It's really handy to have the capacity to set some options like predict or the path to the server command, it's also handy if you have to use a custom ssh command as I do when I do port knocking or use 2FA like yubikeys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment