Re: [Openvpn-devel] +++ route.c 2011-05-18 09:15:17.000000000 +0200

[Peter Stuge]

David Sommerseth wrote:
> Please do a git commit -s to your local git tree (you have all rights to do
> so, its all on your own computer) then do 'git format-patch HEAD~1' and
> send the 0001-whatever-comes-here.patch via git send-email.

git send-email HEAD~1 # is shorthand for that


//Peter

Re: [Openvpn-devel] [PATCH] Improve the mysprintf() issue in openvpnserv.c

[Peter Stuge]

David Sommerseth wrote:
> Signed-off-by: David Sommerseth <d...@users.sourceforge.net>

Acked-by: Peter Stuge <pe...@stuge.se>

Re: [Openvpn-devel] [PATCH v2] Change the default --tmp-dir path to a more suitable path

[Peter Stuge]

David Sommerseth wrote:
> In commit 4e1cc5f6dda22e9 the create_temp_filename() function was
> reviewed and hardened, which in the end renamed this function to
> create_temp_file() in commit 495e3cec5d156.
> 
> With these changes it became more evident that OpenVPN needs a directory
> where it can create temporary files.  The create_temp_file() will create
> such files f.ex. if --client-connect or --plugin which makes use of
> the OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY hook, such as openvpn-auth-pam.so.
> 
> When this happens, OpenVPN will normally create these files in the directory
> OpenVPN was started.  In many cases, this will fail due to restricted access.
> By using --tmp-dir and pointing it to a directory writeable to the user
> running OpenVPN, it works again.
> 
> This patch makes OpenVPN use a more suitable temproary directory by default,
> instead of the current working directory.  On non-Windows platforms this
> default value is set to '/tmp', but can be modified at compile-time by
> running ./configure --with-tmp-dir-path=.  On Windows, it
> will use GetTempPath() to find temporary paths recommended by the OS.  If
> this fails, it will fallback to the old behaviour, using the directory
> where OpenVPN was started.
> 
> In any cases, this default value can be overridden in the configuration
> file by using the --tmp-dir option, as before.
> 
> To check what the default is at runime, you can see this easily by doing
> this:
> 
>   $ ./openvpn --verb 4 --dev tun | grep tmp_dir
> 
> Signed-off-by: David Sommerseth 
> Tested-by: Jan Just Keijser 

The above commit message doesn't really fit the patch anymore. :)


> diff --git a/options.c b/options.c
> index 36e8393..7303cb4 100644
> --- a/options.c
> +++ b/options.c
> @@ -766,11 +766,23 @@ init_options (struct options *o, const bool init_gc)
>  #ifdef ENABLE_X509ALTUSERNAME
>o->x509_username_field = X509_USERNAME_FIELD_DEFAULT;
>  #endif
> -#endif
> -#endif
> +#endif /* USE_SSL */
> +#endif /* USE_CRYPTO */
>  #ifdef ENABLE_PKCS11
>o->pkcs11_pin_cache_period = -1;
>  #endif   /* ENABLE_PKCS11 */

The above hunk is not really related, right? Looks fine otherwise!


//Peter

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Federico Heinz wrote:
>   * some other people agree that there is a use case, but propose
> different ways of approaching the problem through various
> mechanisms to resolve the interface name to an IP address before
> passing it on to OpenVPN. The disagreement here seems to be in
> how such a use case should be handled. Sure, those approaches
> work, and I tried them myself before diving into the source
> code. The problem is I don't think that a supported use case
> should need such involved procedures,

This is exactly the point for me. Until OpenVPN can listen on all IP
addresses on the given interface, it must not try to use interface
terms at all. So IMO the feature depends on multilisten, without
which listening on an interface is not yet supported.

What *is* supported is to listen to a specific IP. It's a rudimentary
feature for all IP daemons. Better solutions are possible, but for
now this is what is there. This is still useful when taking advantage
of hooks in the configuration event source.

When multilisten is in place and listen-on-interface is proposed
again, I think we must discuss whether OpenVPN should automatically
detect reconfiguration of an interface. I tend to think that yes it
should do that. But on the other hand that code will be a bit
involved since no two systems work the same way. The alternative is
to have external helpers which hook into reconfiguration event
sources, but that's not so elegant.


> and while these may be
> more or less complicated, none of them is as simple as being
> able to specify "if:pppo" in the 'local' directive. As a matter
> of fact, the last suggestion from Peter has more code in it than
> the code portion of my suggested patches.

On the other hand it has the significant benefit of connecting the
*actual* configuration event source directly with OpenVPN. When
dealing with dynamic configurations it's critical not to create a
race condition, so anything that does not connect and sync with
actual event source should be out of the question.


> As to the second group of people, I guess it's a matter of drawing
> a line:
> at one end of the spectrum, we could do away with OpenVPN altogether
> and implement it in shell code,

Sorry, but this makes no sense. Feel free to rewrite the (re)config
event handler in C if you prefer. That can certainly have benefits
over the shell script. I showed you something that works that I use.

See above for my feature-nak rationale. It can certainly change to
ack in the future when OpenVPN has better infrastructure to support
the feature.


> we could turn OpenVPN into an HTTP server just touching config files.

Yes please! :)


> I think the proposed change would be a useful addition to the
> project, but I respect your judgement if you disagree.

I disagree at the moment, because of lack of multilisten and
reconfiguration tracking.

The only other programs I can think of that deal in terms of
interfaces are low level network monitoring tools. tcpdump, ethtool
etc, specifically because they operate below IP level. What you
propose is simply not thorough enough for me to like it, if OpenVPN
is to climb one step down in the layers and also deal in interfaces.
I'm not saying I never want it to, but that it needs to be better
prepared if it is going to try.


//Peter


pgpj9RvF_9pxP.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Gert Doering wrote:
> On Sun, Mar 13, 2011 at 05:04:21PM +0100, Peter Stuge wrote:
> > Was this for PPP? Sorry then, I completely overlooked that! I'm
> > fortunate to not have to deal with many PPP links now, but I have,
> > and pppd of course /etc/ppp/ip-up and -down where the same info about
> > (de/re)config is available as in the udhcpc script.
> 
> Which is actually somewhat underlining the point :-) - if you absolutely
> need to do this outside of OpenVPN, it can be done, but how to do that
> depends on interface technology used, specific distribution used (what
> if your distribution doesn't *have* udhcp?), etc.

Sure, but in any case it remains quite trivial.


> - or try to understand why this is needed in the first place :-) - I still
>   don't understand why not binding to anything + --multihome isn't working

Third time: Multiple openvpn daemons on different interfaces and same
port.


> As for including the patch, in either form, I'd rather wait for the
> to-be-implemented-and-cleaned-up "multi-listen" infrastructure that
> someone will hopefully implement for 2.3...

+1


//Peter


pgpOi552oS4qT.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Gert Doering wrote:
> > Changing startup scripts or wrapping openvpn is one way. But I would
> > probably drive everything from the DHCP client instead.
> 
> udhcp can notify components if the IP address of a PPP(!) interface
> changes?
> 
> "No DHCP involved on PPP links".

Was this for PPP? Sorry then, I completely overlooked that! I'm
fortunate to not have to deal with many PPP links now, but I have,
and pppd of course /etc/ppp/ip-up and -down where the same info about
(de/re)config is available as in the udhcpc script.


//Peter


pgp_N0dV13Twl.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Peter Stuge wrote:
> Changing startup scripts or wrapping openvpn is one way. But I
> would probably drive everything from the DHCP client instead.
> 
> As I wrote, udhcpc is very very easy to deal with.

Your /usr/share/udhcpc/default.script to accomplish this would be:

#!/bin/sh
test "$1" = bound || exit 0
sed -i "/^local /s/.*/local $ip/" /etc/openvpn/something/local.conf
/etc/init.d/openvpn.something restart


This covers initial config and reconfig. Zero distro changes. How
much simpler do you need?


//Peter

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Federico Heinz wrote:
> > > Because I don't know it at configuration time.
> > 
> > You said that you already have a solution in place for dealing
> > with interface reconfiguration.
> 
> I said I have a solution in place to restart OpenVPN when the
> configuration changes.

I understand. So there is no solution for dealing with the
reconfiguration.


> Sure, I could change a few system scripts, and include something
> like Alon suggested (/sbin/ip addr show dev ppp0 | grep inet | sed
> 's#.*inet \(.*\)/.*#\1#'), to figure out the IP address, change the
> configuration files and only then restart OpenVPN.

Would suggest wrapping openvpn instead of changing system scripts.


> What I fail to see is how that is "simpler" or easier to understand
> or maintain than writing "local if:ppp0" in the daemon's config file.

It remains clear that openvpn binds to only one IP.


> > Until OpenVPN has multilisten there's no point in OpenVPN trying
> > to do this, because it will only do a poor job.
> 
> This is simply not true. It will do a good job in most cases, and a
> well-specified, if incomplete, job in the rather unlikely case that
> someone deliberately attempts to use this feature together with
> multiple IP addresses on a single interface, and even then it will
> emit a warning.
> 
> You are arguing that because it cannot do the job perfectly in an
> unlikely edge case, it shouldn't even attempt to solve a
> different, well-defined use case.
> 
> I'm sorry, that doesn't make sense to me.

I like do it right or don't do it at all. Only working in some cases
is not good behavior for a feature.


> > The domain name is irrelevant. Go closer to the authoritative
> > source, not further away.
> 
> Agreed. I'm going as close to it as I can: the interface itself,
> from the viewpoint of the program that is using it.

No, the source is the system component which configures the
interface, not the interface itself, which is more like a data
store in this case.


> > > which in some cases involves messing around with the way
> > > your distro usually does things,
> > 
> > Why would it?
> 
> It means modifying the distro's startup scripts for OpenVPN, to get
> them to figure out the IP address of the interface for *some*
> instances of the server but not necessarily for all.

Changing startup scripts or wrapping openvpn is one way. But I would
probably drive everything from the DHCP client instead.

As I wrote, udhcpc is very very easy to deal with.


David Sommerseth wrote:
> As Davide Brini already mentioned, I really wonder if this issue is
> due to lack of --multihome in your configuration, combined with
> listen on 0.0.0.0.

He wants different openvpn servers on different interfaces using same
port.


//Peter

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Federico Heinz wrote:
> On 12/03/2011, Peter Stuge wrote:
> > Federico Heinz wrote:
> > > What I'm trying to solve here is a much simpler (and, in my case,
> > > frequent) use case: I'm starting several instances of OpenVPN,
> > > and I need each of them to listen on specific interfaces, but
> > > their dyndns addresses may not be up-to date yet, so I can
> > > specify neither an IP nor a domain name in the "local" directive.
> > Why can't you specify the IP address?
> 
> Because I don't know it at configuration time.

You said that you already have a solution in place for dealing with
interface reconfiguration.

Simply use the same mechanism for initial configuration.


> The "local" directive will currently accept either a fixed IP
> address (which I don't know, because it will be assigned in the
> future, and may change every time that particular interface is
> brought up)

Yes. Third time, tap the configuration information from the
authoritative source. Until OpenVPN has multilisten there's no point
in OpenVPN trying to do this, because it will only do a poor job.


> or a domain name.

The domain name is irrelevant. Go closer to the authoritative source,
not further away.


> Alon mentioned earlier that this could be dealt with at run-time by
> modifying the startup scripts to determine the IP address and
> specify it through the command-line, but it's an unnecessarily
> complex solution,

I actually think this is an extremely simple solution.


> which in some cases involves messing around with the way
> your distro usually does things,

Why would it?


//Peter

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Federico Heinz wrote:
> What I'm trying to solve here is a much simpler (and, in my case,
> frequent) use case: I'm starting several instances of OpenVPN, and I
> need each of them to listen on specific interfaces, but their dyndns
> addresses may not be up-to date yet, so I can specify neither an IP
> nor a domain name in the "local" directive.

Why can't you specify the IP address?


//Peter

Re: [Openvpn-devel] [PATCH] Bind only to specified interface

[Peter Stuge]

Federico Heinz wrote:
> The reason I looked into this in the first place was that, unlike
> those TCP-based protocols, I couldn't get OpenVPN to work on a
> firewall with two external IP addresses without running two deamons,
> each one bound to one interface only. It is then that I stumbled upon
> the difficulty of setting up the server that must listen on an
> interface that has a dynamically-assigned address.

If this is the problem you want to solve I have to say I think you're
doing it the wrong way.

There are components in your system which *will* know when your
address is reconfigured. Please just configure them to reconfigure
OpenVPN. This would seem to be a good use for the management
interface in OpenVPN.

I like to use udhcpd for systems like this. It is fast and
no-nonsense. It runs a script that you have to write.

It makes no sense trying to work around the requirement of knowing
your configuration. That is always neccessary, unless you are
prepared to listen on 0.0.0.0, which I would guess already works
without special OpenVPN options or code.


//Peter

Re: [Openvpn-devel] [PATCH 1/1] Fix warning: format not a string literal and no format arguments

[Peter Stuge]

Gilles Espinasse wrote:
> Seen with gcc-4.4.5 and -Wformat -Wformat-security
> 
> Signed-off-by: Gilles Espinasse <g@free.fr>

Acked-by: Peter Stuge <pe...@stuge.se>

Re: [Openvpn-devel] [PATCH] Changed snprintf to _snprintf in service-win32/openvpnserv.c

[Peter Stuge]

Samuli Seppänen wrote:
> +++ b/service-win32/openvpnserv.c
> @@ -86,7 +86,7 @@ static HANDLE exit_event = NULL;
>  /* snprintf with guaranteed null termination */
>  #define mysnprintf(out, ...) \
>  { \
> -   snprintf (out, sizeof(out), __VA_ARGS__); \
> +   _snprintf (out, sizeof(out), __VA_ARGS__); \
> out [sizeof (out) - 1] = '\0'; \
>  }

> Does this change affect the automake/gcc-based Windows builds?

My MinGW compiles both snprintf() and _snprintf() fine.


//Peter

Re: [Openvpn-devel] Can *plugin* kill specific ovpn tunnel?...

[Peter Stuge]

Vineet Kumar wrote:
> You mean closing the telnet session after every use so that whoever
> the next guy is gets served?

No I mean creating an intermediary that will know about your use case
and provide an interface that works.


> say, an interactive telnet session

Are they a requirement in your use case?


//Peter

Re: [Openvpn-devel] Can *plugin* kill specific ovpn tunnel?...

[Peter Stuge]

Vineet Kumar wrote:
> Are there ways around that : making telnet accept multiple sessions?

You could serialize your administrative requests somehow and only
ever have one entity use the admin interface.


//Peter

Re: [Openvpn-devel] [Patch] OpenVPN PolarSSL Support Preview [7/8]

[Peter Stuge]

Matthias Andree wrote:
> > Having the branch in git allows iterating over the commits with
> > great ease.
> 
> let's not waste time discussing this over and over

I'm sorry I wasted your time.


//Peter

Re: [Openvpn-devel] [Patch] OpenVPN PolarSSL Support Preview [7/8]

[Peter Stuge]

Adriaan de Jong wrote:
> > I'd use git for the patches right away anyway,
> 
> I'm afraid that we started off development based on checked out
> sources (quite a long time ago). I'll start rebasing to git as soon
> as I've heard that the git tree is stable.

By git I don't mean the very latest contents of the tree, but the
tool. I would branch off the commit that corresponds to your starting
state, apply the patches, and then rebase that branch forward at some
later time.

Having the branch in git allows iterating over the commits with great
ease.

Thanks for these contributions! I think it's a significant improvement!

(I'd like to be able to reuse some of your work or at least learn
from it, for the purpose of libssh2.)


//Peter

Re: [Openvpn-devel] [Patch] OpenVPN PolarSSL Support Preview [7/8]

[Peter Stuge]

Adriaan de Jong wrote:
> Thanks, I'll change the configure options when I rebase the patches
> to 2.2 . I'm a little more familiar with cmake, thanks for pointing
> that out.

Great.


> About the whitespace: it's a tad difficult to split the patches now.

Simple with git. Check out how to do an interactive rebase.

git rebase --help /INTERACTIVE


//Peter

Re: [Openvpn-devel] [Patch] OpenVPN PolarSSL Support Preview [7/8]

[Peter Stuge]

Adriaan de Jong wrote:
Content-Description: 07_polarssl_addition.patch.gz
> Add PolarSSL support to OpenVPN

> +++ b/configure.acWed Dec 08 12:35:42 2010 +0100
> @@ -273,14 +273,16 @@
>  )
>  
>  AC_ARG_WITH([ssl-type],
> -   [  --with-ssl-type=TYPE  Build with the given SSL library, TYPE = openssl 
> ],
> +   [  --with-ssl-type=TYPE  Build with the given SSL library, TYPE = openssl 
> or polarssl ],
> [case "${withval}" in 
>  openssl) SSL_LIB=openssl ;;
> +polarssl) SSL_LIB=polarssl ;;
>  *) AC_MSG_ERROR([bad value ${withval} for --with-ssl-type]) ;;
> esac],
> [SSL_LIB="openssl"]
>  )
>  AM_CONDITIONAL([USE_OPENSSL], [test x$SSL_LIB = xopenssl])
> +AM_CONDITIONAL([USE_POLARSSL], [test x$SSL_LIB = xpolarssl])

Again, I think --with-polarssl may be more common.


//Peter

Re: [Openvpn-devel] how to limite the bandwidth of every client ?

[Peter Stuge]

hmy wrote:
> I use openvpn in server mode. more than 100 clients to connect it
> . I want limit every client's bandwidth ? how to do it ?

Suggest on networking layer in the operating system.


//Peter

Re: [Openvpn-devel] [PATCH] Removed hardcoded signtool dependency from win/build_all.py

[Peter Stuge]

Samuli Seppänen wrote:
> here's the next version of the patch;

Functionally fine!


>  def main(config):
> +
> +# Do a signed build by default
> +unsignedBuild=False

Maybe name the option signedBuild instead? :)


> +# Check if the SignTool module is present. This avoids ImportErrors 
> popping 
> +# up annoyingly _after_ the build.
> +if unsignedBuild == False:
> +   try:
> +  from signtool import SignTool
> +   except (ImportError):
> +  print "ERROR: SignTool python module not found! Can't do a signed 
> build."
> +  sys.exit(1)
> +
> +if unsignedBuild == True:
> +   print "Doing an unsigned build as requested"

I guess the pythonic way would be simply:

if signedBuild:
  try import..
else:
  print as requested..


> -sign(config, 'all')
> +
> +if unsignedBuild == False:
> +   sign(config, 'all')
> +

signedBuild would be nicer, then simply:

if signedBuild:
  sign()


//Peter

Re: [Openvpn-devel] [PATCH 1/6] Use stricter snprintf() formatting in socks_username_password_auth()

[Peter Stuge]

Gert Doering wrote:
> > -  snprintf (to_send, sizeof (to_send), "\x01%c%s%c%s", 
> > strlen(creds.username),
> > -creds.username, strlen(creds.password), creds.password);
> > +  snprintf (to_send, sizeof (to_send), "\x01%c%s%c%s", (int) 
> > strlen(creds.username) & 0xff,
> > +creds.username, (int) strlen(creds.password) & 0xff, 
> > creds.password);
> 
> I tend to NAK this.

I agree. My ack is only for the other patches. (Sorry, I should have
written that in the other mail.) A better solution for this should be
simple as Gert outlined.


//Peter


pgpav_Assz9cZ.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Removed hardcoded signtool dependency from win/build_all.py

[Peter Stuge]

Samuli Seppänen wrote:
> Peter: settings.in is stored in git.

Ok.


> I agree that there should be no way one could make an unsigned build by
> mistake. I think dazo's suggestion about having a command-line switch
> ("force unsigned build") is a good one. The SIGNTOOL variable could then
> be used to just locate signtool.exe and nothing else. This would make it
> behave the same way as most other variables in "settings.in" and allow
> making signed and unsigned builds using the same configuration file.

Acked-by: Peter Stuge <pe...@stuge.se>

Re: [Openvpn-devel] [PATCH] Removed hardcoded signtool dependency from win/build_all.py

[Peter Stuge]

Samuli Seppänen wrote:
> What if build_all.py did this:
> 
> - Check if SIGNTOOL is enabled in settings.in:
>   - Yes: fail if can't import "sign" module
>   - No: don't fail if can't import "sign" module

This also sounds good. (Who typically creates settings.in? Is one in git?)


//Peter

Re: [Openvpn-devel] [PATCH] Removed hardcoded signtool dependency from win/build_all.py

[Peter Stuge]

David Sommerseth wrote:
> >> Modified win/build_all.py so that build does not fail even if
> >> the optional signtool python class is not available.
> > 
> > What is it needed for? Is it really *always* optional?
> 
> If I've understood it correctly, this is related to signing the Windows
> TUN/TAP driver.

Right.


> So if you don't have a signing key/tool available, it is still
> possible to build the rest of OpenVPN.

Fine, but this is not really acceptable when James builds OpenVPN,
so I am requesting a solution that allows them to specify to the
build process that they want a fatal error if signing is not
possible.


//Peter

Re: [Openvpn-devel] [PATCH] Removed hardcoded signtool dependency from win/build_all.py

[Peter Stuge]

Samuli Seppänen wrote:
> From 57b983dc2a1f4a31d3b7c0e2f6de7f778d234b2e Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Samuli=20Sepp=C3=A4nen?= 
> Date: Fri, 12 Nov 2010 17:32:19 +0200
> Subject: [PATCH] Removed hardcoded signtool dependency from win/build_all.py
> 
> Modified win/build_all.py so that build does not fail even if the optional
> signtool python class is not available.

What is it needed for? Is it really *always* optional? Maybe some
setting or parameter to the script so that build can fail e.g. at
openvpn.net if building, where signing should always be done?


//Peter

Re: [Openvpn-devel] [PATCH] Added check for variable CONFIGURE_DEFINES into options.c

[Peter Stuge]

Samuli Seppänen wrote:
> >From 327d6c66fff6962a96f86110dc91b1dad07ca224 Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Samuli=20Sepp=C3=A4nen?= <sam...@openvpn.net>
> Date: Fri, 12 Nov 2010 17:30:07 +0200
> Subject: [PATCH] Added check for variable CONFIGURE_DEFINES into options.c
> 
> The file containing CONFIGURE_DEFINES variable, configure.h, is not present if
> openvpn is built using the Python + Visual C -based buildsystem. This causes 
> the
> build to fail. This patch adds a check to see if variable exists before trying
> to use it.

Acked-by: Peter Stuge <pe...@stuge.se>

Re: [Openvpn-devel] Architecture diagram & Theory of Operation documents

[Peter Stuge]

john s wolter wrote:
> When it comes to debugging problems inside OpenVPN and other FOSS
> software there is a lack of design information.

I disagree that this is relevant for debugging.


> Architecture diagram & Theory of Operation documents are a minimum
> starting point for those not directly involved in development.

Again, I disagree.

Such documentation is indeed good for some things, but as far as
interaction between PKCS#11 providers and PKCS#11 users I actually
think the need is for *less* documentation. :) There is the PKCS#11
standard, but it doesn't provide much overview, it mostly focuses on
matters relevant for implementation of PKCS#11.

There should be fairly much accessible documentation on PKCS#11
online already.

What Jan describes happening in OpenVPN doesn't make a lot of sense
and is "simply" a bug that should be investigated and fixed.


> Do you have a link to these documents?

Until you or someone else writes them, they don't exist. Indeed your
help with this would be most welcome!


//Peter

Re: [Openvpn-devel] HTTP Host header

[Peter Stuge]

Lars Hupel wrote:
> When thinking about it, the Host header doesn't make any sense to me.

It was added to support virtual hosting in a time (20? years ago)
when the domain name wasn't included anywhere in the request, but
used only for client-side IP-adress lookup. Back then it was a
reasonable assumption that IP addresses were actually useful
identifiers for network resources.


//Peter


pgpbklxqElyoC.pgp
Description: PGP signature

Re: [Openvpn-devel] HTTP Host header

[Peter Stuge]

Lars Hupel wrote:
> > Current behavior is correct. The first one is what should be sent.
> > [...]
> > Actually it is helpful. It says that *the resource being requested as
> > obtained from the original URI given by the user* is what should be
> > sent. This is the "remote" parameter and nothing else.
> 
> So what you're implying is that if a request is sent, the part after
> 'CONNECT' must be always the same as the part after 'Host'.

Yep. It's a bit redundant, but consider that CONNECT is already the
odd kid among the HTTP verbs. Most other verbs only take an absolute
path URI, without host component.


> > I'll ack your patch if it only adds a Host header where one is
> > missing, and sends the correct contents, ie. the "remote" hostname.
> 
> Hopefully the attached patch is acceptable for you.

I'm just another community member, but I think the patch is correct.
Thanks!

Acked-by: Peter Stuge <pe...@stuge.se>


pgpqMNObQRJv_.pgp
Description: PGP signature

Re: [Openvpn-devel] HTTP/1.1 Host header

[Peter Stuge]

Lars Hupel wrote:
> I would suggest to always send the Host header (even when HTTP/1.0
> is selected).

I strongly support this. Please watch out for HTTP/1.1, if a client
claims to support 1.1 then servers can respond e.g. with chunked
transfer coding, which certainly isn't supported by 1.0 clients.
On the other hand, if OpenVPN only ever CONNECTs, maybe it's not an
issue.


Lars Hupel wrote:
> As far as I can see, there are two possible hosts which can be sent:
> 
> * 'host' (denoting the 'remote' config option)
> * 'p->options.server' (denoting the 'http-proxy' config option)
> 
> I would guess that sending the latter would be correct, but the
> OpenVPN sources use the first one.

Current behavior is correct. The first one is what should be sent.


> The section about the host header isn't very helpful either:
> 
> >The Host request-header field specifies the Internet host and port
> >number of the resource being requested, as obtained from the original
> >URI given by the user or referring resource (generally an HTTP URL

Actually it is helpful. It says that *the resource being requested as
obtained from the original URI given by the user* is what should be
sent. This is the "remote" parameter and nothing else.

I'll ack your patch if it only adds a Host header where one is
missing, and sends the correct contents, ie. the "remote" hostname.


//Peter

Re: [Openvpn-devel] [PATCH] Fixed compiler warnings reported on Ubuntu 10.04

[Peter Stuge]

David Sommerseth wrote:
> The warnings reported where:
> 
> misc.c:158: warning: ignoring return value of ‘nice’, declared with attribute 
> warn_unused_result
> options.c:4033: warning: format not a string literal and no format arguments
> options.c:4043: warning: format not a string literal and no format arguments
> options.c:4053: warning: format not a string literal and no format arguments
> push.c:182: warning: format not a string literal and no format arguments
> push.c:199: warning: format not a string literal and no format arguments
> push.c:235: warning: format not a string literal and no format arguments
> status.c:171: warning: ignoring return value of ‘ftruncate’, declared with 
> attribute warn_unused_result
> 
> 
> Signed-off-by: David Sommerseth <d...@users.sourceforge.net>

Acked-by: Peter Stuge <pe...@stuge.se>

Re: [Openvpn-devel] Windows build was messed up!

[Peter Stuge]

Toby Thain wrote:
> > So we have stuff for VC6, VS2005, VS2008, DDK/WDK, MinGW and Cygwin. :)
> 
> I consider diversity of working build environments a net benefit to
> the project. It's another useful axis of portability and helps rub
> off non-portable burrs.

I see your point and I agree in part, but personally I would e.g.
prefer to ignore VC6, since it requires a fair amount source code
workarounds because it's *so* old.

Also, someone needs to maintain these build methods, and I wish
people didn't have to spend time on that part so much.


> > autotools is absolutely alien in Windows, and to be honest a bit
> > of a hassle to set up if you're already a VS wizard.
> 
> NMAKE is convenient for Windows builds as it will also happily run
> under Linux/WINE, targeting both 32 and 64 bit Windows runtimes with
> the 'freely downloadable' Express toolchain. It may even be possible
> to combine autotools with the Microsoft compiler, though I haven't
> tried it (CC="wine cl", etc).

Maybe it works for cross-compiling, but then MinGW(-w64) will be way
better tested so why bother, when for people on Windows it's no real
help, they want the GUI stuff. Also, being a library, there are some
more build options like e.g. enable debugging or not. This is
typically a separate target in VS but only a configure switch with
autotools, and while this case is easy enough, I don't think those
targets scale so well..

Personally I'm missing a way to do some build-time scripting also on
Windows. The best idea I've come up with so far is a .cmd which does
some of what autoconf does, e.g. "sed" version number from
configure.ac into .rc files and create a config.h, but that doesn't
mean it's a very good idea. :\


//Peter

Re: [Openvpn-devel] Windows build was messed up!

[Peter Stuge]

Alon Bar-Lev wrote:
> The whole build environment of openvpn is malformed, using two
> separate build systems for windows and posix,

Only two? Oh that's nothing. We're having lots of fun with build
systems in libusb-1.0, where each contributing Windows developer
actually requires a separate build system. So we have stuff for VC6,
VS2005, VS2008, DDK/WDK, MinGW and Cygwin. :) Fortunately at least
the last two use autotools, and I do consider that the canonical
build method since it covers all supported systems, and allows
automated packaging on a POSIX host, which is what still dominates in
the project. But the Windows support is important, as are the
developers, and as long as someone agrees to maintain a particular
build method then I think it's fine to offer it on some kind of
best-effort basis.

autotools is absolutely alien in Windows, and to be honest a bit of a
hassle to set up if you're already a VS wizard.


> and the mixture of different projects: openvpn, tap, windows
> installer, easy-rsa.

How can the installer situation be improved? I want to make an NSIS
installer as well, and I would quite like to keep it *within* the
project..? Is there a much better way?


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (26th Aug 2010)

[Peter Stuge]

Gert Doering wrote:
> > > Did some testing to see if creating a static binary would be
> > > trivial. As it was not,
> > 
> > Why wasn't it? Please share details from the testing.
> 
> Naively just calling "gcc -static" led to linker failures due to
> OpenSSL not being found.
> 
> This was a "can we do it that easily?" quick test, so we didn't
> investigate further why it failed.

It failed because dependencies were not met, so the test doesn't
really say much about OpenVPN..

I tried to add a comment to the ticket in Trac about USE flags in
Gentoo, but Trac rejects it as potential spam and I will not waste
my life trying to work around the filter.

I tried adding -static -lz when building openvpn-testing.git and got
the following:

$ gcc  -g -O2 -static  -o openvpn base64.o buffer.o crypto.o dhcp.o error.o 
event.o fdmisc.o forward.o fragment.o gremlin.o helper.o httpdigest.o lladdr.o 
init.o interval.o list.o lzo.o manage.o mbuf.o misc.o mroute.o mss.o mtcp.o 
mtu.o mudp.o multi.o ntlm.o occ.o pkcs11.o openvpn.o options.o otime.o 
packet_id.o perf.o pf.o ping.o plugin.o pool.o proto.o proxy.o ieproxy.o ps.o 
push.o reliable.o route.o schedule.o session_id.o shaper.o sig.o socket.o 
socks.o ssl.o status.o thread.o tun.o win32.o cryptoapi.o  -lssl -lcrypto 
-llzo2 -ldl -lz
plugin.o: In function `plugin_init_item':
/tmp/openvpn-testing/plugin.c:215: warning: Using 'dlopen' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
misc.o: In function `get_group':
/tmp/openvpn-testing/misc.c:117: warning: Using 'getgrnam' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
misc.o: In function `get_user':
/tmp/openvpn-testing/misc.c:82: warning: Using 'getpwnam' in statically linked 
applications requires at runtime the shared libraries from the glibc version 
used for linking
socket.o: In function `getaddr_multi':
/tmp/openvpn-testing/socket.c:170: warning: Using 'gethostbyname' in statically 
linked applications requires at runtime the shared libraries from the glibc 
version used for linking
/usr/lib/gcc/i686-pc-linux-gnu/4.4.4/../../../libcrypto.a(b_sock.o): In 
function `BIO_get_port':
(.text+0x48a): warning: Using 'getservbyname' in statically linked applications 
requires at runtime the shared libraries from the glibc version used for linking

I get a binary though:

$ ls -l openvpn
-rwxr-xr-x 1 stuge stuge 3781597 Aug 29 00:58 openvpn
$ strip openvpn
$ ls -l openvpn
-rwxr-xr-x 1 stuge stuge 2160084 Aug 29 00:59 openvpn


The only thing missing was -lz to satisfy OpenSSL dependencies.
If pkg-config is available in the system then pkg-config openssl --libs
is all that is needed to get the linker flags needed for OpenSSL.
Maybe use PKG_CHECK_MODULES() in configure.ac to check for OpenSSL
before trying the current seemingly manual tests?


//Peter


pgpFuTUuhcTEx.pgp
Description: PGP signature

Re: [Openvpn-devel] Summary of the IRC meeting (26th Aug 2010)

[Peter Stuge]

Alon Bar-Lev wrote:
> > > Did some testing to see if creating a static binary would be
> > > trivial. As it was not,
> >
> > Why wasn't it? Please share details from the testing.
> 
> I guess the modules, pkcs11 and other components that uses dlopen
> should be disabled in openvpn.

Well, as a first step it would be nice to build the openvpn binary
statically. Even if it still uses dlopen e.g. for p11.


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (26th Aug 2010)

[Peter Stuge]

Samuli Seppänen wrote:
> Discussed the "Some way of supporting static compilation" issue:
> 
> 
> 
> Did some testing to see if creating a static binary would be
> trivial. As it was not,

Why wasn't it? Please share details from the testing.


> decided to ask the Gentoo guys why they need static OpenVPN
> binary before going any further.

Because Gentoo users who set the static USE flag ideally want
*all* packages to be built statically.


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (19th Aug 2010)

[Peter Stuge]

Gert Doering wrote:
> The app is cross-compileable today - but to make that work requires that 
> all relevant Windows APIs are exported in mingw headers *and libraries*.

Sure. You emphasize libraries, but MinGW is not so much a library as
it is a wrapper for the standard runtime from MS.


> Windows APIs have the nasty habit of changing over time,

I know this. I also do win32 development. With MinGW.


> so if mingw stops updating their libraries etc., OpenVPN would be
> sort of stuck to the old APIs - which might turn into a problem.
> Or might not.  We don't know...

What could happen is that OpenVPN wants to use a brand new API which
was introduced in e.g. Windows 7, and if MinGW doesn't yet support
this, then it can't be used.

First of all, I'm not sure that will happen, since we want OpenVPN to
be backwards compatible for older Windows systems too.

But if it really is neccessary, then maybe that API can be added to
MinGW with very little effort, since MinGW is just a wrapper.


> > libusb-win32 can already compile their driver using MinGW.
> 
> To be able to cross-compile the driver would certainly be nice. 
> Need to check how libusb-win32 does this...

They've switched to depending on the WDK (supercedes the DDK) out of
preference and stopped supporting MinGW builds. Rev 212 from May is
the last that cross-compiles without problems.

svn co -r 212 
https://libusb-win32.svn.sourceforge.net/svnroot/libusb-win32/trunk/libusb 
libusb-win32
cd libusb-win32/
make host_prefix=i686-mingw32 libusb0.sys

(You may need another prefix.)


//Peter


pgpeXM8YX8UgX.pgp
Description: PGP signature

Re: [Openvpn-devel] Summary of the IRC meeting (19th Aug 2010)

[Peter Stuge]

Samuli Seppänen wrote:
> converting the OpenVPN build system (on Windows) to depend only on
> python, Visual Studio Professional and DDK. The original build
> system required bash and various other UNIX tools.

James' mentioned argument for prefering Visual Studio, namely that
MinGW may not have all the most recent functions in .h files, is not
so good IMO. Since James also seems to prefer working outside Windows
I would rather try to make OpenVPN for Windows crosscompileable, both
driver and app.

libusb-win32 can already compile their driver using MinGW.

But maybe that can happen later, any streamlining of builds is a good
thing.


//Peter

Re: [Openvpn-devel] features

[Peter Stuge]

Alex T wrote:
> - some kind of route / firewall manipulation for the Windows client.

Try:

push "ip-win32 dynamic"
push "redirect-gateway def1"


//Peter

Re: [Openvpn-devel] [PATCH] Choose a different field in X509 to be username

[Peter Stuge]

David Sommerseth wrote:
> > 3) "sizeof(common_name)" is useless... Line 745: char
> > common_name[TLS_USERNAME_LEN]; we can use directly TLS_USERNAME_LEN.
> 
> Thanks a lot for the patch and all rework done.  (Also: thank you to all
> reviewers!)
> 
> The third and last patch looks very good!  I've applied it to the
> feat_misc branch and merged it into allmerged.

I strongly agree with Alon about sizeof() rather than a second
reference to the same define. It is the defensive way.


//Peter

Re: [Openvpn-devel] PATCH: remove bashisms from easy-rsa

[Peter Stuge]

Matthias Andree wrote:
> I'm not sure currently if Solaris /bin/sh likes $(...) notation or
> insists on `...`

`` is the only portable way.


//Peter


pgpbnpSJnBkbG.pgp
Description: PGP signature

Re: [Openvpn-devel] bug: can't get dhcp using --up option

[Peter Stuge]

niv...@gmail.com wrote:
> I want to report this bug:

Can you help debug it? Add instrumentation to that script, or maybe
to a new script, and see what is different about the interface when
script is being run outside openvpn.


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (13th May 2010)

[Peter Stuge]

Samuli Seppänen wrote:
> Here's the summary of the previous community meeting.

Thanks for sending it out!


> user-space network stack
> packet bus

Look into vde


>  right -- if you had an openvpn core with only producer/consumer 
> modules

That would be vdeswitch


> crazy kinds of transport protocols

HTTP is the only thing that I've had a real need for besides TCP or UDP.


//Peter

Re: [Openvpn-devel] Building the TAP drivers from source and then signing them (possible?)

[Peter Stuge]

Jon Onstott wrote:
> I would like to go ahead and compile and sign the TAP drivers
> myself.  Does anyone know which certificate would be best to
> purchase?

There was some discussion about this on the libusb mailinglist just
the other day. It seems there may be a good deal to be had with
VeriSign right now, but it's not absolutely clear if it's the right
kind of certificate. (See Tim's post about WinQual vs. signing.)

http://sourceforge.net/mailarchive/forum.php?thread_name=l2wa276da401004271651uc1557d88r1106a473ade1b29%40mail.gmail.com_name=libusb-devel


//Peter

Re: [Openvpn-devel] [PULL-REQUEST v3] VLAN-Tagging

[Peter Stuge]

Fabian Knittel wrote:
> I've attached a diff containing all changes introduced by the
> current patch-set.

Thanks for doing this. It makes review so much easier.


> +++ b/configure.ac
> @@ -212,6 +212,12 @@ AC_ARG_ENABLE(selinux,
> [SELINUX="yes"]
>  )
>  
> +AC_ARG_ENABLE(vlan-tagging,
> +   [  --disable-vlan-tagging  Disable support for VLAN tagging/untagging],
> +   [VLAN_TAGGING="$enableval"],
> +   [VLAN_TAGGING="yes"]
> +)

Maybe explicitly mention that it's 802.1Q ?


> +dnl enable --vlan-tagging
> +if test "$VLAN_TAGGING" = "yes"; then
> +   AC_DEFINE(ENABLE_VLAN_TAGGING, 1, [Enable VLAN tagging/untagging])
> +fi

Maybe also here.


> +++ b/mroute.c
> @@ -164,12 +164,28 @@ mroute_extract_addr_ipv4 (struct mroute_addr *src,
>return ret;
>  }
>  
> +static void mroute_copy_ether_to_addr(struct mroute_addr *maddr,
> +   const uint8_t *eth_addr,
> +   int16_t vid)
> +{
> +  maddr->type = MR_ADDR_ETHER;
> +  maddr->netbits = 0;
> +  memcpy (maddr->addr, eth_addr, 6);
> +#ifdef ENABLE_VLAN_TAGGING
> +  maddr->len = 8;
> +  memcpy (maddr->addr + 6, , 2);

Does this need to consider htons() ?

Also, I have a weak preference for uint16_t, just to indicate that
negative values are never correct.


> @@ -303,6 +322,9 @@ mroute_addr_print_ex (const struct mroute_addr *ma,
>   {
>   case MR_ADDR_ETHER:
> buf_printf (, "%s", format_hex_ex (ma->addr, 6, 0, 1, ":", gc)); 
> +#ifdef ENABLE_VLAN_TAGGING
> +   buf_printf (, "@%d", *(int16_t*)(ma->addr + 6));
> +#endif

Also here, ntohs() ?


> +++ b/multi.c
..
> @@ -1918,6 +1924,28 @@ multi_process_post (struct multi_context *m, struct 
> multi_instance *mi, const un
>return ret;
>  }
>  
> +#ifdef ENABLE_VLAN_TAGGING
> +bool
> +buf_filter_incoming_vlan_tags (const struct buffer *buf)
> +{
> +  if (BLEN (buf) >= (int) sizeof (struct openvpn_8021qhdr))
> +{
> +  const struct openvpn_8021qhdr *vlanhdr = (const struct 
> openvpn_8021qhdr *) BPTR (buf);
> +
> +  if (ntohs (vlanhdr->tpid) == OPENVPN_ETH_P_8021Q)
> +{
> +   const int16_t vid = vlanhdr_get_vid(vlanhdr);
> +   if (vid != 0)
> + {
> +   msg (D_VLAN_DEBUG, "dropping tagged incoming frame, vid: %d", 
> vid);
> +   return true;
> + }
> + }
> +}
> +  return false;
> +}
> +#endif

What a horrible coding style this is!! But oh well, that's certainly
not your fault. :)

However, I really like code that checks for "break" conditions first,
and returns early, over code that nests multiple levels of
conditions, something like:

{
  struct openvpn_8021qhdr *vlanhdr;
  int16_t vid;
  if (BLEN (buf) < (int) sizeof (struct openvpn_8021qhdr))
return false;
  vlanhdr = (const struct openvpn_8021qhdr *) BPTR (buf);
  if (ntohs (vlanhdr->tpid) != OPENVPN_ETH_P_8021Q)
return false;
  vid = vlanhdr_get_vid(vlanhdr);
  if (0 == vid)
return false;
  msg (D_VLAN_DEBUG, "dropping .. ");
  return true;
}

But don't let this start some kind of argument about coding style. I
have no technical complaints about the hunk above.


> @@ -2033,10 +2062,27 @@ multi_process_incoming_link (struct multi_context *m, 
> struct multi_instance *ins
>   }
> else if (TUNNEL_TYPE (m->top.c1.tuntap) == DEV_TYPE_TAP)
>   {
> +#ifdef ENABLE_VLAN_TAGGING
> +   int16_t vid = 0;
> +#else
> +   const int16_t vid = 0;
> +#endif

I like how you use const here.


> @@ -2116,6 +2163,159 @@ multi_process_incoming_link (struct multi_context *m, 
> struct multi_instance *ins
>return ret;
>  }
>  
> +#ifdef ENABLE_VLAN_TAGGING
> +/*
> + * For vlan_accept == VAF_ONLY_UNTAGGED_OR_PRIORITY:
> + *   If a frame is VLAN-tagged, it is dropped.  Otherwise, the global
> + *   vlan_pvid is returned as VID.

This is just a comment in the code, but maybe mention that vid=0 in a
tag does not mean that it's VLAN-tagged. Yes, _OR_PRIORITY indicates
this, but it might be nice to be super consistent also in the text.


> +remove_vlan_tag (const struct context *c, struct buffer *buf)
> +{
..
> +  /* Tagged packet. */
> +
> +  vid = ntohs (vlanhdr_get_vid ());

Hmm - does ntohs() here mean that it shouldn't be done in mroute.c?


> +multi_prepend_vlan_tag (const struct context *c, struct buffer *buf)
> +{
..
> +  /* Frame too small? */
> +  if (BLEN (buf) < (int) sizeof (struct openvpn_ethhdr))
> +goto drop;

I like forward goto for error handling very much too!


> +  /* Frame too small for header type? */
> +  if (BLEN (buf) < (int) (sizeof (struct openvpn_8021qhdr)))
> + goto drop;

This looks like a whitespace oops?


> +  /* Not enough head room for VLAN tag? */
> +  if (buf_reverse_capacity (buf) < SIZE_ETH_TO_8021Q_HDR)
> + goto drop;

Here too? Hmm. Am I missing something?


> @@ -2158,6 +2363,16 @@ multi_process_incoming_tun (struct multi_context *m, 
> const unsigned int mpp_flag
> * the appropriate multi_instance object.

Re: [Openvpn-devel] Unpackaged Windows binaries (Was: Re: [Openvpn-users] [ANN] OS X packages - OpenVPN 2.1.1)

[Peter Stuge]

Karl O. Pinc wrote:
> IMO OpenVPN is encouraging bad practices by supplying packages for
> distros that include OpenVPN.

Ideally the package for that distro as made by OpenVPN is always
equivalent to the one made by the distributor.

What do I mean? I mean that I'm happy with .spec files and the likes,
that make up the source code for a particular kind of binary package,
being committed upstream.

And if they are, then it's of course fairly easy for upstream to use
them, and to make packages which are at the very least compatible
with the distribution.

And, I think that since it is downloaded from a site other than the
distributor's, users who actually do grab packages "manually" will be
able to tell the difference from a package delivered by the
distributor.


//Peter

Re: [Openvpn-devel] [PATCH] Serial number export, fixed

[Peter Stuge]

Davide Brini wrote:
> The users are supposed to adapt it to their needs and make it more
> robust.

Hah! Users will not.


//Peter

Re: [Openvpn-devel] Unpackaged Windows binaries (Was: Re: [Openvpn-users] [ANN] OS X packages - OpenVPN 2.1.1)

[Peter Stuge]

Karl O. Pinc wrote:
> the project is already releasing unpackaged Linux
> binaries

Really?


> and it now talking about doing the same for OS/X binaries.

Recently discussed work for contrib/ produces a .dmg, very much a
package in my view.


> There's clear utility.

I guess it depends. I'm not sure that I agree that compilation is a
much bigger bother than (cross-)compiling the NSIS installer..

But, I do agree with you that there is utility.


//Peter

Re: [Openvpn-devel] Finnish translation of OpenVPN-GUI for review

[Peter Stuge]

Samuli Seppänen wrote:
> Btw. this translation file format is by far the most difficult one
> I've ever worked with.

Probably because it isn't a translation file format at all. :)

This is the file format used by the Windows Resource Compiler to
turn "resources" such as dialogs or files into object form that can
be linked with an app and referenced easily from within the app.

The resource compiler supports #include but I don't know if any other
CPP features work. At least it would be possible to move all texts to
a separate file.


//Peter

Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames

[Peter Stuge]

Jan Just Keijser wrote:
> FYI: 802.1Q defines VLAN 1 as the 'native' LAN: all packets on VLAN 1 
> are *by definition* not encapsulated (according to my CCNA guide ;-))

802.1Q != CCNA..

Look at the spec, Table 9-2 on page 86. (100 in PDF)

VID Use
  0 "no VLAN identifier is present in the frame"
  1 "The default PVID value used for classifying frames on ingress ..
 The PVID value of a Port can be changed by management."
FFF "Reserved for implementation use."


> Perhaps we need to make sure that VID 1 means untagged ...

Any VID can be untagged. While 1 is the default it can change and
OpenVPN shouldn't really care.

One alternative approach to using tag 0 would be to introduce a
vlan-pvid (or vlan-default-tag) option to set the PVID.

But explicitly allowing tag 0 can also be useful.


//Peter

Re: [Openvpn-devel] [PATCH 3/9] vlan: Add per-client --vlan-tag option

[Peter Stuge]

Hi Fabian,

Fabian Knittel wrote:
> >> +#define OPENVPN_8021Q_MAX_VID 0xFFFE
> > 
> > The max VID in 802.1q is 4095 = 0xfff.
> 
> You are absolutely correct.  Thanks for catching that.  I intended
> it to say 0xFFE, because the standard talks about VID values being
> valid within the range 0 through 4094 [1].

Aha!


> But OPENVPN_8021Q_MASK_VID should definitely be defined as 0xFFF,
> as the bit field is 12 bits.

Right.


> VID == 0xFFF is "Reserved for implementation use." and "shall not
> be configured as PVID [...] or transmitted in a tag header." [2]
> But I'm not sure whether it's used in reality and if you'd like
> that value to be accepted, I have no objections.

I think it's good to play along with the standards, so I like MAX
0xffe too.


> Thanks again for reviewing!

You're welcome!


//Peter

Re: [Openvpn-devel] [RFC][PATCH 0/9] VLAN tagging on TAP devices in OpenVPN server mode

[Peter Stuge]

David Sommerseth wrote:
> I'll admit I don't understand too much how the VLAN's really work in
> core network code.

There's a prefix, and a wrapper to create virtual interfaces. And
there's the vconfig tool to do configuration. NIC drivers need to
support oversize frames.

vconfig add eth0 4  # VLAN 4 will be tagged on eth0

So to test this, one could do:

vconfig add tap0 4  # Creates tap9.4 which is VLAN 4 on tap9

Then run openvpn --mode server --vlan-tag 4 on that tap0, and client
traffic should then come out of tap0.4 rather than tap0.


> I'll try to find some time to read through the patches once more.
> 
> [* Other reviewers are most welcome to have a look as well! *]

I think they looked okey, apart from the things I pointed out
already.


> You scare at least me when stating that this code "was originally
> only intended as a proof of concept", which is why I'm not signing
> off these patches immediately and giving you a feature branch.
> But I'm open for full inclusion!

I'm generally in favor too. Although originally PoC I think this
looks like good quality now, and VLANs are actually really simple.

I would like to request extra special thorough review of broadcast
domain separation though, so that it's certain that nothing will jump
VLANs.


//Peter

Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames

[Peter Stuge]

Fabian Knittel wrote:
> +  if (ntohs (vlanhdr.tpid) != OPENVPN_ETH_P_8021Q)
> +{
> +  /* Drop untagged frames */
> +  goto err;
> +}

It would be nice to be able to use VID 0 to mean untagged packets.


//Peter

Re: [Openvpn-devel] [PATCH] On TARGET_LINUX define _GNU_SOURCE if not defined

[Peter Stuge]

Karl O. Pinc wrote:
> And, to reiterate, applying patches within an rpm
> spec file is normal, expected, and part of the
> rpm design so a certain level of "development" is supported.

Should also not be a problem, since those patches are likely to start
from a known release tarball, so autoconf still isn't needed.


//Peter

Re: [Openvpn-devel] [PATCH] On TARGET_LINUX define _GNU_SOURCE if not defined

[Peter Stuge]

Peter Stuge wrote:
> RHEL4.6 can easily build a release that was prepared using the most
> recent autoconf.

In fact, I would suggest that using the newest autotools when
creating the package makes it *more* likely to build effortlessly
on older systems.


//Peter

Re: [Openvpn-devel] Supporting "route-gateway dhcp" on non-Windows

[Peter Stuge]

Karl O. Pinc wrote:
> I'm not at all sure it solves the core issues, which is that
> an already running dhcp client won't have auto-detected
> the tap interface that OpenVPN creates -- iff OpenVPN is 
> started after the dhcp client.

Note that several DHCP clients only handle one interface per DHCP
client process..


//Peter

Re: [Openvpn-devel] Supporting "route-gateway dhcp" on non-Windows

[Peter Stuge]

Karl O. Pinc wrote:
> The boot order that makes sense to me is:
> 
> bring the interfaces up
> start dhcp client (if not triggered directly from the interfaces)
> start openvpn
> 
> The problem is that if the dhcp client is started before openvpn
> and openvpn is creating the tap interface then it's too late

At least some distributions work more like this:

foreach interface:
  set link up
  possibly start openvpn
  address add


//Peter

Re: [Openvpn-devel] Supporting "route-gateway dhcp" on non-Windows

[Peter Stuge]

Karl O. Pinc wrote:
> > I know of at least four DHCP clients and I avoid dhclient as much as
> > possible. It would be a tremendous mistake to tie OpenVPN to any one
> > DHCP client IMO.
> 
> Only D is tied to dhclient.  A, B, and C, work fine with any dhcp
> client daemon.  (Or A does anyway, B and C require configuration file
> support for the interface.  I assume this is a common property of
> dhcp clients.)

Actually no, neither dhcpcd nor pump nor udhcpc uses one.


> It would be nice not to force use of a particular client.

I think it's neccessary.


> Given that.  Why not just use -up and --down to do what's
> needed?  Avoid changing the code and write documentation.

Agree completely. This is how I've been using OpenVPN for years
already.


//Peter

Re: [Openvpn-devel] Supporting "route-gateway dhcp" on non-Windows

[Peter Stuge]

Karl O. Pinc wrote:
> > I'm hoping that we can make "route-gateway dhcp" work on Unix
> > platforms as well.  I'm thinking there are two possible ways we
> > could do this:

Network interface configuration is highly distribution specific.


> A) dhclient
> 
> B) dhclient.conf
> 
> C) dhclient
> 
> D) dhcptl or omapi or .. omshell
..
> This would be tied to the ISC implementation, but probably nobody
> cares.

I know of at least four DHCP clients and I avoid dhclient as much as
possible. It would be a tremendous mistake to tie OpenVPN to any one
DHCP client IMO.


What most if not all distributions have in common, or could have in
common, is that a script configures the network interface. Sometimes
it's called ifup, other times it's /etc/init.d/net.tap0, yet other
times it's something else.

I suggest instead relying on a generic configuration script to do
interface configuration when the VPN comes up.

I believe that's actually what most distributions do already.


//Peter

Re: [Openvpn-devel] Linux tun/tap performance issues

[Peter Stuge]

James Yonan wrote:
> all of the CPU-intensive cryptographic operations are being done in
> user space.

Could some kind of crypto acceleration of OpenSSL be in play?


//Peter

Re: [Openvpn-devel] [PATCH] IPv6 support for TUN/TAP driver on windows

[Peter Stuge]

Gert Doering wrote:
> -!define PRODUCT_TAP_RELDATE "06/22/2009"
> +!define PRODUCT_TAP_RELDATE "07/03/2010"

Seems month/date are swapped.


//Peter

Re: [Openvpn-devel] [PATCH] enhance tls-verify possibility

[Peter Stuge]

Karl O. Pinc wrote:
> > > +  "--tls-export-cert [directory] : Get peer cert in PEM format and
> 
> There is no man page.  It's in sample-scripts/.

It's a new option, right?


//Peter

Re: [Openvpn-devel] [PATCH v2] Do not randomize resolving of IP addresses in getaddr()

[Peter Stuge]

Karl O. Pinc wrote:
> Someone may be relying on the behavior but, at the moment
> or depending on present dns circumstances, does not have
> multiple A records returned.  In this case no warning will
> be generated.

The flip side of that coin is also valid I think.

Consider independent configuration of VPN and DNS. Early errors would
restrict VPN setup and possibly shipment until after DNS has been set
up, while lazy evaluation allows DNS changes to happen later.

May be particularly important since DNS has a builtin propagation
delay of any changes.

I'm not saying I disagree, but that it may be a tradeoff.


//Peter

Re: [Openvpn-devel] New development process ready

[Peter Stuge]

David Sommerseth wrote:
> I need a place where to put this openvpn-testing tree.

If you send me a public SSH key and prefered username I'll set up a
repo on git.stuge.se, currently hosting libusb and libssh2
development besides some of my own projects.


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (4th Feb 2010)

[Peter Stuge]

Samuli Seppänen wrote:
> http://www.secure-computing.net/wiki/index.php/OpenVPN/IRC_meetings

I really appreciated getting the major points via email from the
first meeting! The forced-attachment download from SF archive is
not so nice for me.


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (28th Jan 2010)

[Peter Stuge]

Samuli Seppänen wrote:
> > OpenID
> 
> I'm somewhat familiar with OpenID but I need to take another look
> at it. SF.net supports it, so the same OpenID could be used for the
> SF.net "openvpn" project (should we make use of it) as well as the
> community site Trac instance.

SF can be an OpenID provider when that is enabled (I belive each user
needs to do so), but I'm not sure if SF accepts external OpenID
providers. The Trac auth plugin would have no problem accepting
logins with SF OpenIDs.


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (28th Jan 2010)

[Peter Stuge]

Karl O. Pinc wrote:
> > I do not feel the forums and mailing list need to be synchronized.
> > They are two different mediums, and should be treated as such.
> 
> I disagree.

I would also like to unify forums and mailing lists, for purely
egotistical reasons - I really dislike the web.

My idea for this since many years is to have an IMAP interface to
forum databases, so that an email program can be used to access the
forum contents. It never progressed far beyond the idea stage though,
except maybe for small involvement with the BincIMAP project.
BincIMAP has backend abstraction so a SQL backend should be kindof
straightforward.

I believe this would help usability tremendously - I think a lot more
users would be able to interface with forums if they had a decent
interface without so much clutter.


//Peter

Re: [Openvpn-devel] Summary of the IRC meeting (28th Jan 2010)

[Peter Stuge]

Samuli Seppänen wrote:
> Decided to start with a single Trac-based site for the whole
> community rather than have separate user/developer sites.

I think this is a good way to get some content going, even if it
turns out that the solution is inadequate sometime in the future.


> Discussed community site spam prevention in some length:
> 
> * There is an anti-spam plugin for Trac (see below)
> * A proper user registration process needs to be chosen - not too
>   bureaucratical but not too automated, either

In this context I'd like to suggest accepting OpenID login in Trac.

When I first learned about OpenID I was very sceptical, but now that
I have implemented it for one Trac I actually really like it.

For those not yet familiar with OpenID, the idea is to let a web site
(really any web site) provide authentication service for your users.
OpenID specifies the API used between service (Trac) and
authenticator (other web site) and this way, users only need to log
in at one place.

This sounds like a gaping hole, but in combination with email address
verification before allowing write access in Trac it is pretty
efficient - and convenient.

Many different web sites are OpenID providers, and there are various
packages available for setting up your own OpenID provider on a URL
that you control.

To log in, you give the URL to that OpenID provider (which can be any
web page, OpenID provider info can be added in meta tags) and then
you log in over there, and finally Trac checks with $overthere that
you are logged in.

I think the email address verification part is important.


http://bitbucket.org/Dalius/authopenid-plugin/

(I have an ebuild in my overlay at http://stuge.se/overlay.txt)


Alon Bar-Lev wrote:
> Trac is promises to provide all but provides none, I really don't
> know which project you managed with Trac, but without ticket
> dependencies

There's a plugin for it:

http://trac-hacks.org/wiki/MasterTicketsPlugin


> and without proper CC lists

Another plugin:

http://trac-hacks.org/wiki/TracNotification


> and workflow it is difficult to manage a real project.

Hm - please expand on what you mean by workflow?


> 2. ViewVC/ViewGIT/Whatever - Browse code.

I agree that e.g. git-web is much nicer than Trac's source viewer but
again it's nice to have something integrated.


//Peter

Re: [Openvpn-devel] Summary of the "OpenVPN development model" meeting

[Peter Stuge]

David Sommerseth wrote:
> For those of us not being heavily involved in development processes
> from day-to-day, we can probably survive with whatever VCS is being
> used.

Fair enough. But I think two git features in particular matter also
in the casual patcher case. It's very nice that git keeps both author
and committer information, and it's doubleplus nice that git can
transport commits via email. These two in particular allow regular
developers to accept casual patches very easily.

Although I personally like git I'm not saying it's right for OpenVPN.
Just sharing some experience.


//Peter

Re: [Openvpn-devel] OpenVPN core development processes?

[Peter Stuge]

Gert Doering wrote:
> The nice thing about git is that people can very easily do
> "experimental branches" in their own repository, giving access to
> testers, without having to have write access to SVN.

Agree, but the hosting issue aside this is possible also from an SVN
repo thanks to the git svn support. Try:

git svn clone http://svn.openvpn.net/projects/openvpn/trunk/openvpn

(You may want to create and use a gitauthors file for best results.)


//Peter

Re: [Openvpn-devel] Crosscompile OpenVPN 2.1.1

[Peter Stuge]

Eike Lohmann wrote:
> checking for lzo1x_1_15_compress in -llzo... no
> configure: error: LZO headers were found but LZO library was not found
..
> What exactly is missing and how to build it?

Look in config.log for the exact error from the configure test.
It would help to also post it here.


//Peter

Re: [Openvpn-devel] RFE: allow 'lport 0' setup for random port binding

[Peter Stuge]

Let's try some more.

Karl O. Pinc wrote:
> > no; it is because the OpenVPN client creates the same src + dst pair
> > for every connection.
> 
> Enrico is right.  It's in the IP RFC, the 2MSL (twice the maximum
> segment lifetime) rule.  (STD 5 is the right rfc?)

I agree that the statement about TCP/UDP 4-tuple is correct for the
two participating systems. They identify the connection that way, and
it's even easy to see as a user with something basic such as netstat.
Internally they of course store lots of other information related to
the connection, associated to the 4-tuple.


> I haven't otherwise been following the discussion, but if there's
> no other way to do what he wants to do with OpenVPN then
> OpenVPN is violating the RFC.

My point is that a "stateful" firewall should keep more info about
connections than just a 4-tuple. The firewall that is causing Enrico
trouble seems to not do this, since it does not recognize a new
connection if uses the same 4-tuple. To me, that actually sounds like
the textbook definitition of a _stateless_ firewall. :)

I am convinced that it is simpler to add code to OpenVPN to work
around this problem, than to fix the firewall, but I still don't
think this is really a fault in OpenVPN. I'm just another user though.


//Peter

Re: [Openvpn-devel] Summary of the "OpenVPN development model" meeting

[Peter Stuge]

Samuli Seppänen wrote:
> Most other tasks are dependent on the setup of the community site.
> A somewhat realistic estimate for finishing them is 1-2 months.

Let me know (maybe off-list) if you want some hints on setting up
Trac with git support.

Trac is nice because it offers both tickets and a wiki. Some projects
do a really nice job using Trac for everything by changing the
styling; e.g. http://madwifi-project.org/ have it looking so nice
that no second site (MediaWiki) is neccessary.

Trac currently does not yet have stellar coverage for multiple
related projects. Components can be used, but it may not be enough.
A single 0.12 Trac can supposedly run with several repostiories, but
I don't have experience from this (yet). Some links:

http://trac.edgewall.org/ticket/130
http://trac.edgewall.org/wiki/TracMultipleProjects
http://trac.edgewall.org/wiki/TracMultipleProjects/SingleEnvironment
http://trac.edgewall.org/wiki/VcRefactoring#SupportforMultipleRepositories


//Peter