blank password in default config

[andreworg]

Hi,
I have 1.6.0 in my Ubuntu system, coming from the package manager.
It features a default config file /etc/openfortivpn/config, with null values.

# config file for openfortivpn, see man openfortivpn(1)
host =
port =
username =
password =

I just compiled 1.11.0 from git and tried running it before installing.
I found out that the blank password in the default configuration file will cause openfortivpn not to ask for a password at the CLI and my connections to fail just as if I provided a wrong password.

I don't know if this is by design. With the same default config, 1.6.0 prompts for the password.

Having a default config with blank values is probably not the best of ideas - I see in the repo there is a file named /etc/openfortivpn/config.template, probably that's what gets installed in the current release - but I think that the current behaviour is at least somehow misleading. At the very least, the verbose debug output should hint at the fact that a password is being picked up from the config file. I don't think it does now.
Better still, the log could show more clearly what is being taken from the config file and what is being picked up/overridden from the CLI parameters.

[mrbaseman]

This is an intended change (although it is admittedly not a very lucky one) in order to support login with a smartcard or user certificate only.
Just comment out the password line in your config file (or remove it) - and openfortivpn 1.11.0 will ask you again for the password.

[mrbaseman]

we should comment out the line in the template to avoid this problem in the future. The template is copied to the default config location if that file doesn't exist during installation.

[mrbaseman]

a fix for this is landed on the master branch now. I would leave this ticket open until the next release, just to have it visible for other users who upgrade to the 1.11.0 release and run into the same problem

[andreworg]

I still contend that the debug output could be clearer about where the parameters are taken from. I'll propose a PR if I find the time.

[mrbaseman]

Additional information in the debug output usually helps understanding problems. In this context however, I don't really see the need anymore. In Version 1.8.0 Issue #258 was solved via #346.

Since then, the general rule is that openfortivpn has built-in default values for almost all parameters. In many cases it is sufficient to define the host to connect to and the username. The ssl port 443 is assumed by default and the password is interactively asked, if it is not specified in any way. At configure time the location of the default config file is determined depending on the installation prefix or the sysconfdir. Values in the config file are taken (unless the lines are commented out) and they have priority over the built-in default values. If a config file is supplied on the command line, that config file is used instead of the default config. Other parameters on the command line have priority over the ones in the config file. For the values there are some checks which lead to an error if insane values are supplied, e.g. an empty host name or values that are outside of the acceptable range for the particular parameter.

These are the general rules. After they are applied, a configuration is available in memory, which consists of all parameters known to openfortivpn and depending on where they are specified (default value, config file, command line), the one with the higher priority is applied.

Now we have a fixed configuration, but to understand the behavior, it is important to know, that for some parameters there are special rules for special values. If the username and the password are empty, we must have a user certificate or a smartcard (#464) to authenticate the user. If the password is not specified anywhere, the user is asked for one interactively. If there is a line for the password, but no value assigned, it is assumed that the password is empty and it shall not be sent together with the certificate (#375). If there is a user certificate, a username but no line for the password, it is still assumed that a password is required in addition to the certificate (that's how I run my Fortigate).

There are other parameters which may have special values with a special meaning, like persistent=0 or user-cert=pkcs11: but that's pretty much it and they are documented in the man page.

Anyhow, we could add some debugging output in the merge_config() function that might help to spot issues when a value is changed in an unexpected way, or maybe add a function to dump the full config that is used after the call of merge_config() in main()

[andreworg]

the password is interactively asked, if it is not specified in any way

I agree with most of what you wrote but I think the verbose log could mention whether a parameter has been picked up from the config or from the CLI - I admit this is maybe not very important but we're talking about a verbose log - but more importantly that a password has been picked up at all - be it from the config or from the CLI - as it actually happens for the other parameters.
Maybe this is just an opinion and I sure don't want to convince you or anyone - that's what forks and PRs are for anyway - but it sure would have saved me time troubleshooting my issue. I was wondering why the heck I did not get a password prompt and I did not realize that a password was being silently provided, until I realized that HTTP status code 405 could mean a wrong password and it dawned on me. Maybe I'm just dumb or maybe the verbose log could have warned me that a password was being picked up.
My 2¢

~$ sudo openfortivpn myhost:443 -u myuser -v
DEBUG:  openfortivpn 1.11.0
WARN:   Bad port in config file: "0".return code 
DEBUG:  Loaded config file "/etc/openfortivpn/config".
DEBUG:  Config host = "myhost"
DEBUG:  Config realm = ""
DEBUG:  Config port = "443"
DEBUG:  Config username = "myuser"
DEBUG:  Resolving gateway host ip
DEBUG:  Establishing ssl connection

[mrbaseman]

Ok, I see. I have prepared PR #518 for this.
Note that with -v -v -v the password would have been printed out.
We had #407 in which cleartext output of the password was considered insecure practice. Therefore, we have introduced this extra debug level.

[mrbaseman]

fix for the debug output is merged on master

[andreworg]

Terrific work. Thank you!

[andreworg]

Not sure I understand the PR in main.c at line 490-492. It logs "Disabled password due to empty entry in config file" but it does not actually get disabled, at least the program still fails to ask a password interactively if there is a blank password in the config file.

[mrbaseman]

if an empty password is in the config file, openfortivpn does not ask for a password later in main() (the password is not NULL anymore, it's the empty string) and it does not use it when attempting to make a connection in auth_log_in
The reason why the log output is made at the beginning of main() is that at a later stage the parameters from cli are meged with the values from the config file, and the cli may overwrite the ones from the config file (therefore the check for cli_cfg.password == NULL)

[DimitriPapadopoulos]

Fixed in openfortivpn 1.12.0.