Re: [Openvpn-devel] OpenVPN argument parsing of most options ignores "extra" parameters

[Samuli Seppänen]

On 17-05-15 19:24, Gert Doering wrote:

On Sun, May 17, 2015 at 03:13:21PM +0200, Steffan Karger wrote:

That sounds reasonable to me.  However, I tend to be easier in accepting
(potentially) breaking changes than other community members.  So I think
it makes sense to put this on the agenda for the next IRC meeting.
Iirc, the next one should be tomorrow at 20:00 CEST.

+1 :-)

I do not have strong feelings on whether this should be a warning or
a hard error, but it's generally good practice to let the user know about
"unexpected extra arguments" (or, turned around, totally annoying if
software just ignores stuff on the command line without at least telling
you) - so all for "doing something about it".

Quoting from Samuli's IRC meeting summary:

"Decided to make the config parser fail if erroneous extra options are
encountered."

That is, there is agreement to get this into master / 2.4.  Patches are
very welcome.

-Steffan


Hi,

I added a ticket for this:



Samuli

Re: [Openvpn-devel] OpenVPN argument parsing of most options ignores "extra" parameters

[Steffan Karger]

On 17-05-15 19:24, Gert Doering wrote:

On Sun, May 17, 2015 at 03:13:21PM +0200, Steffan Karger wrote:

That sounds reasonable to me.  However, I tend to be easier in accepting
(potentially) breaking changes than other community members.  So I think
it makes sense to put this on the agenda for the next IRC meeting.
Iirc, the next one should be tomorrow at 20:00 CEST.


+1 :-)

I do not have strong feelings on whether this should be a warning or
a hard error, but it's generally good practice to let the user know about
"unexpected extra arguments" (or, turned around, totally annoying if
software just ignores stuff on the command line without at least telling
you) - so all for "doing something about it".


Quoting from Samuli's IRC meeting summary:

"Decided to make the config parser fail if erroneous extra options are 
encountered."


That is, there is agreement to get this into master / 2.4.  Patches are 
very welcome.


-Steffan

Re: [Openvpn-devel] OpenVPN argument parsing of most options ignores "extra" parameters

[Jonathan K. Bullard]

On Mon, May 4, 2015 at 9:26 AM, Jonathan K. Bullard wrote:
> If I have a
> configuration that has worked for many years I might be more likely to
> not notice one warning among all the output in a typical log at the
> default "verb 3" setting.

Correction: the default setting is "verb 1", not "verb 3".

However, almost all of the configurations I see from people
troubleshooting Tunnelblick include "verb 3", and eight of the ten
sample configuration files in OpenVPN 2.3.6 include "verb 3". So I
think my conclusions are still valid: a typical log includes a lot of
information and warnings are easily overlooked in a configuration that
was worked for years.

Re: [Openvpn-devel] OpenVPN argument parsing of most options ignores "extra" parameters

[Gert Doering]

Hi,

On Sun, May 17, 2015 at 03:13:21PM +0200, Steffan Karger wrote:
> That sounds reasonable to me.  However, I tend to be easier in accepting 
> (potentially) breaking changes than other community members.  So I think 
> it makes sense to put this on the agenda for the next IRC meeting. 
> Iirc, the next one should be tomorrow at 20:00 CEST.

+1 :-)

I do not have strong feelings on whether this should be a warning or 
a hard error, but it's generally good practice to let the user know about
"unexpected extra arguments" (or, turned around, totally annoying if 
software just ignores stuff on the command line without at least telling 
you) - so all for "doing something about it".

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgp3gZuRxhdhu.pgp
Description: PGP signature

Re: [Openvpn-devel] OpenVPN argument parsing of most options ignores "extra" parameters

[Steffan Karger]

On 04-05-15 15:26, Jonathan K. Bullard wrote:

Perhaps it could go into OpenVPN 2.4 but not 2.3? As I understand it,
2.3 is gets security and bug fixes, so many people probably don't test
it as thoroughly as a new release; some probably won't test it at all
-- those are the ones that you are presumably worried about. When 2.4
is released, most people will test it at least cursorily before
deploying it. If extra parameters cause a failure, it will be
immediately obvious and can be fixed easily.

Although usually ignoring extra parameters would not cause security
problems, to the extent they do, the concept of OpenVPN being "secure
by default" is jeopardized by not causing an error. Something like
ignoring a "--redirect-gateway def1" -- which would cause traffic to
be "leaked" outside of the VPN -- could be considered a security risk.



That sounds reasonable to me.  However, I tend to be easier in accepting 
(potentially) breaking changes than other community members.  So I think 
it makes sense to put this on the agenda for the next IRC meeting. 
Iirc, the next one should be tomorrow at 20:00 CEST.


-Steffan

Re: [Openvpn-devel] OpenVPN argument parsing of most options ignores "extra" parameters

[Jonathan K. Bullard]

On Sun, May 3, 2015 at 12:33 PM, Steffan Karger  wrote:
> On 17-04-15 11:28, Jonathan K. Bullard wrote:
> > I would like to propose a patch which complains if OpenVPN options
> > include parameters that are not expected.
>
> I agree that silently ignoring extra parameters is not nice. However, I
> think that breaking configs after they have worked for many years might
> result in too many unpleasant surprises for our users. How would you
> feel about just issuing a warning for ignored extra parameters?

Thanks for your comment. It's a difficult balance.

In my opinion a warning is not sufficient: if a configuration has an
extra parameter, the user probably **thinks** that the parameter is
doing something. In that situation, I would personally would rather
have an unpleasant surprise than continue in ignorance. If I have a
configuration that has worked for many years I might be more likely to
not notice one warning among all the output in a typical log at the
default "verb 3" setting.

The "fix" if a config fails is very simple: remove the extra parameter
or insert a line break if one is missing. You can then connect, and
OpenVPN's behavior will not have changed except that if a line break
was inserted then the previously ignored option will be used. If the
parameter was supposed to do something important, then more thought
might be required, but in that case, it is probably even **more**
important that the configuration breaks.

Perhaps it could go into OpenVPN 2.4 but not 2.3? As I understand it,
2.3 is gets security and bug fixes, so many people probably don't test
it as thoroughly as a new release; some probably won't test it at all
-- those are the ones that you are presumably worried about. When 2.4
is released, most people will test it at least cursorily before
deploying it. If extra parameters cause a failure, it will be
immediately obvious and can be fixed easily.

Although usually ignoring extra parameters would not cause security
problems, to the extent they do, the concept of OpenVPN being "secure
by default" is jeopardized by not causing an error. Something like
ignoring a "--redirect-gateway def1" -- which would cause traffic to
be "leaked" outside of the VPN -- could be considered a security risk.

Re: [Openvpn-devel] OpenVPN argument parsing of most options ignores "extra" parameters

[Steffan Karger]

Hi Jonathan,

On 17-04-15 11:28, Jonathan K. Bullard wrote:

I would like to propose a patch which complains if OpenVPN options
include parameters that are not expected.

If possible, I would like to get a "feature ACK" consensus before I
create the patch. (If I get a "feature NAK" then I won't create the
patch.)

The patch would be to reject options that are followed by extra parameters.

The error message would change from
  Options error: Unrecognized option or missing parameter(s)
to
  Options error: Unrecognized option or missing or unexpected parameter(s)

Perhaps the current behavior of ignoring "extra" parameters is
purposeful, to allow options to have parameters that are ignored by
"old" versions of OpenVPN but accepted and acted on by "new" versions
of OpenVPN. (I think doing that is not a good idea, but maybe that's
the way the community wants it.)

The patch would break any configurations that have such "extra"
parameters. I think that's good, because the configurations are, well,
wrong. But there could be a lot of such configurations being used with
current versions of OpenVPN -- who knows?


**EXAMPLE**

[...]

Thoughts?


Always ;-)

I agree that silently ignoring extra parameters is not nice. However, I 
think that breaking configs after they have worked for many years might 
result in too many unpleasant surprises for our users. How would you 
feel about just issuing a warning for ignored extra parameters?


-Steffan