Re: [Openvpn-devel] OpenVPN Interactive Service Branding

2017-11-01 Thread Simon Rozman 
Hi,

> 
> Hi Simon,
> 
> Speaking of MSIs... we are planning on moving from NSIS to MSI due to
> security issues like the one we fixed in previous release. At the moment four
> (other) people who have expressed interest in taking part in the "create MSI
> installers for OpenVPN" project. One of these people is me.
> 
> Would you be interested in joining forces in this task?

I'd love to. Count me in!

I have been in the MSI packaging for about 10 years now - automated builds from 
command line, developed DLL custom actions in C/C++, localization, GUI, WiX etc.

Funny, I was thinking of repacking OpenVPN as an MSI package to ease the 
in-organization deployment and updating for some time now.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 1/1] openssl: add engine method for loading the key

2017-11-01 Thread Steffan Karger 
Hi,

On 29-10-17 16:57, James Bottomley wrote:
> On Sun, 2017-10-29 at 23:15 +0800, Antonio Quartulli wrote:
>> James,
>>
>> could you please resend a full patch, so to have a better overview of
>> the whole change?
> 
> Sure thing.  It's below.

Feature makes sense, so feature-ACK.  An early question though:

> --- a/src/openvpn/crypto_openssl.c
> +++ b/src/openvpn/crypto_openssl.c
> @@ -969,4 +969,59 @@ hmac_ctx_final(HMAC_CTX *ctx, uint8_t *dst)
>  HMAC_Final(ctx, dst, _hmac_len);
>  }
>  
> +#ifdef HAVE_OPENSSL_ENGINE
> +static int
> +ui_read(UI *ui, UI_STRING *uis)
> +{
> +SSL_CTX *ctx = UI_get0_user_data(ui);
> +
> +if (UI_get_string_type(uis) == UIT_PROMPT) {
> +pem_password_cb *cb = SSL_CTX_get_default_passwd_cb(ctx);
> +void *d = SSL_CTX_get_default_passwd_cb_userdata(ctx);
> +char password[64];
> +
> +cb(password, sizeof(password), 0, d);
> +UI_set_result(ui, uis, password);
> +
> +return 1;
> +}
> +return 0;
> +}
> +#endif

This looks like it should use our user query wrappers from (e.g.)
console.h.  David, you're the expert here, what should James use to
query for passwords?

-Steffan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] autoconf: Fix engine checks for openssl 1.1

2017-11-01 Thread Steffan Karger 
Hi,

On 29-10-17 16:34, James Bottomley wrote:
> In openssl 1.1, ENGINE_cleanup became a #define instead of a function
> (because it's no longer needed as engines are self cleaning).  Update
> the autoconf.ac script to check for ENGINE_cleanup as a declaration to
> avoid falsely underfinig HAVE_OPENSSL_ENGINE in openssl 1.1+

Typo in "underfinig", but that should be fixable when applying the patch.

> Signed-off-by: James Bottomley 
> ---
>  configure.ac | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 6f59baef..45aa5017 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -899,6 +899,13 @@ if test "${enable_crypto}" = "yes" -a 
> "${with_crypto_library}" = "openssl"; then
>   ,
>   [have_openssl_engine="no"; break]
>   )
> + if test "${have_openssl_engine}" = "no"; then
> + AC_CHECK_DECL( [ENGINE_cleanup], [have_openssl_engine="yes"],,
> + [[
> + #include 
> + ]]
> + )
> + fi
>   if test "${have_openssl_engine}" = "yes"; then
>   AC_DEFINE([HAVE_OPENSSL_ENGINE], [1], [OpenSSL engine support 
> available])
>   fi
> 

Change looks good, and does what it says on the tin.  ACK.

-Steffan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Don't throw fatal errors from create_temp_file()

2017-11-01 Thread Steffan Karger 
Hi,

The following four patches are a rebased and reordered set of patched that
replace the original 2 patches I send in September.  These should include
fixes for all Antonio's and David's comments (where needed).

-Steffan


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 4/4 v3] create_temp_file/gen_path: prevent memory leak if gc == NULL

2017-11-01 Thread Steffan Karger 
If gc == NULL, the data allocated in the alloc_gc_buf() call in
create_temp_file or the string_mod_const call in gen_path would never
be free'd.

These functions are currently never called that way, but let's prevent
future problems.

While touching create_temp_file, also remove the counter variable, which is
never read, simplify the do-while to a while loop, and truncate the prefix
(if needed) to preserve the random and extension of the created filename.

Signed-off-by: Steffan Karger 
---
v2:
 - change create_temp_file to avoid using a struct buffer (simpler)
 - add gc != NULL check for gen_path (avoid similar memleak pitfall)
v3:
 - Check the return value of openvpn_snprintf()

 src/openvpn/misc.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 25f38003..67011169 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -723,21 +723,26 @@ test_file(const char *filename)
 const char *
 create_temp_file(const char *directory, const char *prefix, struct gc_arena 
*gc)
 {
-static unsigned int counter;
-struct buffer fname = alloc_buf_gc(256, gc);
 int fd;
 const char *retfname = NULL;
 unsigned int attempts = 0;
+char fname[256] = { 0 };
+const char *fname_fmt = PACKAGE "_%.*s_%08lx%08lx.tmp";
+const int max_prefix_len = sizeof(fname) - (sizeof(PACKAGE) + 7 + (2 * 8));
 
-do
+while (attempts < 6)
 {
 ++attempts;
-++counter;
 
-buf_printf(, PACKAGE "_%s_%08lx%08lx.tmp", prefix,
-   (unsigned long) get_random(), (unsigned long) get_random());
+if (!openvpn_snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len,
+  prefix, (unsigned long) get_random(),
+  (unsigned long) get_random()))
+{
+msg(M_WARN, "ERROR: temporary filename too long");
+return NULL;
+}
 
-retfname = gen_path(directory, BSTR(), gc);
+retfname = gen_path(directory, fname, gc);
 if (!retfname)
 {
 msg(M_WARN, "Failed to create temporary filename and path");
@@ -760,7 +765,6 @@ create_temp_file(const char *directory, const char *prefix, 
struct gc_arena *gc)
 return NULL;
 }
 }
-while (attempts < 6);
 
 msg(M_WARN, "Failed to create temporary file after %i attempts", attempts);
 return NULL;
@@ -812,6 +816,12 @@ gen_path(const char *directory, const char *filename, 
struct gc_arena *gc)
 #else
 const int CC_PATH_RESERVED = CC_SLASH;
 #endif
+
+if (!gc)
+{
+return NULL; /* Would leak memory otherwise */
+}
+
 const char *safe_filename = string_mod_const(filename, CC_PRINT, 
CC_PATH_RESERVED, '_', gc);
 
 if (safe_filename
-- 
2.14.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/4 v3] pf: clean up temporary files if plugin init fails

2017-11-01 Thread Steffan Karger 
From: Steffan Karger 

close_instance() tries to remove the file in c2.pf.filename, but that only
works if we actually set that if we fail.  So, set that filename as soon
as we know we've created the file.

Signed-off-by: Steffan Karger 
---
v2: As suggested by Antionio, get rid of local 'gc' and 'file' vars.
v3: make c->c2.pf.filename const (fixes compile warning)

 src/openvpn/pf.c | 10 --
 src/openvpn/pf.h |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/openvpn/pf.c b/src/openvpn/pf.c
index 5cb002bf..e1b5b0e7 100644
--- a/src/openvpn/pf.c
+++ b/src/openvpn/pf.c
@@ -618,19 +618,18 @@ pf_load_from_buffer_list(struct context *c, const struct 
buffer_list *config)
 void
 pf_init_context(struct context *c)
 {
-struct gc_arena gc = gc_new();
 #ifdef PLUGIN_PF
 if (plugin_defined(c->plugins, OPENVPN_PLUGIN_ENABLE_PF))
 {
-const char *pf_file = create_temp_file(c->options.tmp_dir, "pf", );
-if (pf_file)
+c->c2.pf.filename = create_temp_file(c->options.tmp_dir, "pf",
+ >c2.gc);
+if (c->c2.pf.filename)
 {
-setenv_str(c->c2.es, "pf_file", pf_file);
+setenv_str(c->c2.es, "pf_file", c->c2.pf.filename);
 
 if (plugin_call(c->plugins, OPENVPN_PLUGIN_ENABLE_PF, NULL, NULL, 
c->c2.es) == OPENVPN_PLUGIN_FUNC_SUCCESS)
 {
 event_timeout_init(>c2.pf.reload, 1, now);
-c->c2.pf.filename = string_alloc(pf_file, >c2.gc);
 c->c2.pf.enabled = true;
 #ifdef ENABLE_DEBUG
 if (check_debug_level(D_PF_DEBUG))
@@ -658,7 +657,6 @@ pf_init_context(struct context *c)
 #endif
 }
 #endif
-gc_free();
 }
 
 void
diff --git a/src/openvpn/pf.h b/src/openvpn/pf.h
index 414c85b8..b839fd2e 100644
--- a/src/openvpn/pf.h
+++ b/src/openvpn/pf.h
@@ -75,7 +75,7 @@ struct pf_context {
 bool enabled;
 struct pf_set *pfs;
 #ifdef PLUGIN_PF
-char *filename;
+const char *filename;
 time_t file_last_mod;
 unsigned int n_check_reload;
 struct event_timeout reload;
-- 
2.14.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/4 v2] Don't throw fatal errors from create_temp_file()

2017-11-01 Thread Steffan Karger 
From: Steffan Karger 

This function is called in response to connecting clients, and can fail
when I/O fails for some (possibly temporary) reason.  In such cases we
should not exit the process, but just reject the connecting client.

This commit changes the function to actually return NULL on errors, and
(where needed) changes the callers to check for and handle errors.

Since the tls-crypt-v2 metadata code also calls create_temp_file() when
clients connect, I consider this a prerequisite for tls-crypt-v2.

Signed-off-by: Steffan Karger 
---
v2: put || at the beginning of a line (not the end)

 src/openvpn/misc.c   |  6 +++---
 src/openvpn/ssl_verify.c | 32 +---
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 8c7f6116..25f38003 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -740,7 +740,7 @@ create_temp_file(const char *directory, const char *prefix, 
struct gc_arena *gc)
 retfname = gen_path(directory, BSTR(), gc);
 if (!retfname)
 {
-msg(M_FATAL, "Failed to create temporary filename and path");
+msg(M_WARN, "Failed to create temporary filename and path");
 return NULL;
 }
 
@@ -755,14 +755,14 @@ create_temp_file(const char *directory, const char 
*prefix, struct gc_arena *gc)
 else if (fd == -1 && errno != EEXIST)
 {
 /* Something else went wrong, no need to retry.  */
-msg(M_FATAL | M_ERRNO, "Could not create temporary file '%s'",
+msg(M_WARN | M_ERRNO, "Could not create temporary file '%s'",
 retfname);
 return NULL;
 }
 }
 while (attempts < 6);
 
-msg(M_FATAL, "Failed to create temporary file after %i attempts", 
attempts);
+msg(M_WARN, "Failed to create temporary file after %i attempts", attempts);
 return NULL;
 }
 
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 9cd36d7a..de54fb74 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -547,14 +547,14 @@ verify_cert_export_cert(openvpn_x509_cert_t *peercert, 
const char *tmp_dir, stru
 FILE *peercert_file;
 const char *peercert_filename = "";
 
-if (!tmp_dir)
+/* create tmp file to store peer cert */
+if (!tmp_dir
+|| !(peercert_filename = create_temp_file(tmp_dir, "pcf", gc)))
 {
+msg (M_WARN, "Failed to create peer cert file");
 return NULL;
 }
 
-/* create tmp file to store peer cert */
-peercert_filename = create_temp_file(tmp_dir, "pcf", gc);
-
 /* write peer-cert in tmp-file */
 peercert_file = fopen(peercert_filename, "w+");
 if (!peercert_file)
@@ -589,10 +589,13 @@ verify_cert_call_command(const char *verify_command, 
struct env_set *es,
 
 if (verify_export_cert)
 {
-if ((tmp_file = verify_cert_export_cert(cert, verify_export_cert, 
)))
+tmp_file = verify_cert_export_cert(cert, verify_export_cert, );
+if (!tmp_file)
 {
-setenv_str(es, "peer_cert", tmp_file);
+ret = false;
+goto cleanup;
 }
+setenv_str(es, "peer_cert", tmp_file);
 }
 
 argv_parse_cmd(, verify_command);
@@ -609,6 +612,7 @@ verify_cert_call_command(const char *verify_command, struct 
env_set *es,
 }
 }
 
+cleanup:
 gc_free();
 argv_reset();
 
@@ -879,21 +883,21 @@ key_state_rm_auth_control_file(struct key_state *ks)
 }
 }
 
-static void
+static bool
 key_state_gen_auth_control_file(struct key_state *ks, const struct tls_options 
*opt)
 {
 struct gc_arena gc = gc_new();
-const char *acf;
 
 key_state_rm_auth_control_file(ks);
-acf = create_temp_file(opt->tmp_dir, "acf", );
+const char *acf = create_temp_file(opt->tmp_dir, "acf", );
 if (acf)
 {
 ks->auth_control_file = string_alloc(acf, NULL);
 setenv_str(opt->es, "auth_control_file", ks->auth_control_file);
-} /* FIXME: Should have better error handling? */
+}
 
 gc_free();
+return acf;
 }
 
 static unsigned int
@@ -1184,7 +1188,12 @@ verify_user_pass_plugin(struct tls_session *session, 
const struct user_pass *up,
 
 #ifdef PLUGIN_DEF_AUTH
 /* generate filename for deferred auth control file */
-key_state_gen_auth_control_file(ks, session->opt);
+if (!key_state_gen_auth_control_file(ks, session->opt))
+{
+msg (D_TLS_ERRORS, "TLS Auth Error (%s): "
+ "could not create deferred auth control file", __func__);
+goto cleanup;
+}
 #endif
 
 /* call command */
@@ -1209,6 +1218,7 @@ verify_user_pass_plugin(struct tls_session *session, 
const struct user_pass *up,
 msg(D_TLS_ERRORS, "TLS Auth Error (verify_user_pass_plugin): peer 
provided a blank username");
 }
 
+cleanup:
 return retval;
 }
 
-- 
2.14.1

Re: [Openvpn-devel] [PATCH 0/2] Reject client if PF plugin is configured, but init fails

2017-11-01 Thread Steffan Karger 
Hi,

On 30-09-17 03:00, Antonio Quartulli wrote:
> On 30/09/17 00:24, Steffan Karger wrote:
>> This changes the behavior for pf plugins: instead of just not initializing
>> the firewall rules and happily continuing, this now rejects the client in
>> the case of an (unlikely) failure to initialize the pf.
>>
>> Signed-off-by: Steffan Karger 
>> ---
>>  src/openvpn/pf.c | 9 +
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/openvpn/pf.c b/src/openvpn/pf.c
>> index 5cb002bf..29231b67 100644
>> --- a/src/openvpn/pf.c
>> +++ b/src/openvpn/pf.c
>> @@ -639,10 +639,11 @@ pf_init_context(struct context *c)
>>  }
>>  #endif
>>  }
>> -else
>> -{
>> -msg(M_WARN, "WARNING: OPENVPN_PLUGIN_ENABLE_PF disabled");
>> -}
>> +}
>> +if (!c->c2.pf.enabled)
>> +{
>> +msg(M_WARN, "WARNING: failed to init PF plugin, rejecting 
>> client.");
>> +register_signal(c, SIGUSR1, "plugin-pf-init-failed");
>>  }
> 
> after this part there is some code dealing with the management
> interface: wouldn't it be better to have a return after registering the
> signal so that we don't attempt any interaction with the management
> interface? If the plugin initialization failed, we should exit right
> away no?
> 
> (although this PF thing with the plugin being optional makes everything
> more complicated every time :/)

Good point, I'll fix that and send a whole new patch set with all your
comments processed.  With the expansion from 2 to 4 patches, the thread
has become quite a mess.

> Another thought: if we abort every time we can't initialize the pf
> context, do you think we need the c->c2.pf.enabled member at all?
> 
> It looks to me that after this patch if PF is enabled, either a client
> is connected and has a PF context, or its connection was rejected
> entirely. Thus a overall state should be enough rather than a per-client
> one. (Can probably be adjusted with another patch if you don't feel like
> doing all this in here?)

The pf.enabled member is used in a number of quick inline checks to
determine whether to go into the whole pf code path or not.  I think
it's best to keep it.

-Steffan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

2017-11-01 Thread Simon Rozman 
Hi,

> Named instances sounds like is a good idea. As you pointed out, Microsoft
> itself uses command line parameters on service (like -i NAME for SQL server)
> so that looks kosher.
>
> There is a problem though: multiple instances also need multiple service
> names but the service name is hardcoded in the source. In our case even
> the dispatcher needs to know the service name(s) as we use
> SERVICE_WIN32_SHARE_PROCESS and share the process between
> the legacy service[*] and the interactive service.
>
> Looks like the service name also will have to be dynamically-generated
> from the instance name.

There is a simpler solution for that:

1. Change the order of services listed in `dispatchTable` so the interactive 
service is listed first.

2. All non-default instances could install as `SERVICE_WIN32_OWN_PROCESS`, 
thus SCM will launch them as a separate process picking only the first entry 
in the `dispatchTable` ignoring the service name and ignoring all non-first 
entries.

> If you want to support multiple installations of openvpn.exe/service,
> building with a different PACKAGE_NAME can't be avoided, can it?.
> Instead, if using the "official" executables is ok, why not just use
> oepnvpnserviceinteractive as is. Does eduVPN really need its own instance
> of the service?

No, actually eduVPN client is quite happy with the official interactive 
service as it is. Until we hit deployment issues with the 
openvpn-install-2.4.4-I601.exe installer described below.

- When OpenVPN is not already installed, we would like to install it using 
only the minimum feature selection. You see, OpenVPN GUI desktop shortcut and 
auto-start in system tray is not desirable, as it will confuse the end users 
how to start using eduVPN.
This can be accomplished using the command line parameters. So far so good.

- When an older version of OpenVPN is already installed, we would like to 
update it. Unfortunately, the installer does not detect currently installed 
features. It just copies the default feature set overwriting previous 
installation. Again, users get the OpenVPN GUI desktop shortcut and system 
tray icon. Even if they were not installed before.
We could use the command line parameters again to suggest a feature selection, 
but if the OpenVPN GUI was installed before, the update would not update it, 
leaving behind some sort of Frankenstein OpenVPN installation.
The installer is missing a command line parameter to tell it "if feature is 
present then update, else leave it absent".

- eduVPN will require latest up-to-date OpenVPN. Should any user require a 
specific older version of OpenVPN to connect to other service providers than 
eduVPN, this would be a problem.

Because of those issues, we would like to install local OpenVPN - and keep it 
up-to-date - using an MSI package. While openvpn.exe and its dependency DLLs 
can easily be managed as a local copy, the interactive service cannot - 
without interfering with original OpenVPN installation.

> That said, I like the idea of being able to run multiple (named) instances
> of the service each with its own service pipe.

I shall prepare, test and git-send-email a patch for it shortly. Thank you for 
your valued opinion.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

2017-11-01 Thread Илья Шипицин 
2017-11-01 23:46 GMT+05:00 Simon Rozman :

> Hi,
>
> >
> > Hi Simon,
> >
> > Speaking of MSIs... we are planning on moving from NSIS to MSI due to
> > security issues like the one we fixed in previous release. At the moment
> four
> > (other) people who have expressed interest in taking part in the "create
> MSI
> > installers for OpenVPN" project. One of these people is me.
> >
> > Would you be interested in joining forces in this task?
>
> I'd love to. Count me in!
>

I seen your commits regarding win32 refactoring in
https://github.com/amebis/openvpn
they are good

can we count on you in https://github.com/OpenVPN/openvpn-gui/issues/77 ?



>
> I have been in the MSI packaging for about 10 years now - automated builds
> from command line, developed DLL custom actions in C/C++, localization,
> GUI, WiX etc.
>
> Funny, I was thinking of repacking OpenVPN as an MSI package to ease the
> in-organization deployment and updating for some time now.
>
> Best regards,
> Simon
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/2] doxygen: use relative paths from the project root

2017-11-01 Thread Steffan Karger 
Instead of genering docs with full path names (e.g.
/home/steffan/dev/openvpn/src/openvpn/crypto.h), use a relative path wrt
the project root (e.g. src/openvpn/crypto.h).  This makes the generated
doxygen easier to read.

Signed-off-by: Steffan Karger 
---
 doc/doxygen/openvpn.doxyfile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/doxygen/openvpn.doxyfile b/doc/doxygen/openvpn.doxyfile
index a7d9728..80cda2b 100644
--- a/doc/doxygen/openvpn.doxyfile
+++ b/doc/doxygen/openvpn.doxyfile
@@ -25,7 +25,7 @@ ABBREVIATE_BRIEF   = "The $name class" \
 ALWAYS_DETAILED_SEC= NO
 INLINE_INHERITED_MEMB  = NO
 FULL_PATH_NAMES= YES
-STRIP_FROM_PATH= ""
+STRIP_FROM_PATH= "."
 STRIP_FROM_INC_PATH=
 SHORT_NAMES= NO
 JAVADOC_AUTOBRIEF  = YES # NO
-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

2017-11-01 Thread Samuli Seppänen 
On 01/11/2017 10:25, Simon Rozman wrote:
> Because of those issues, we would like to install local OpenVPN - and keep it 
> up-to-date - using an MSI package. While openvpn.exe and its dependency DLLs 
> can easily be managed as a local copy, the interactive service cannot - 
> without interfering with original OpenVPN installation.
> 

Hi Simon,

Speaking of MSIs... we are planning on moving from NSIS to MSI due to
security issues like the one we fixed in previous release. At the moment
four (other) people who have expressed interest in taking part in the
"create MSI installers for OpenVPN" project. One of these people is me.

Would you be interested in joining forces in this task?

-- 
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] Summary of the community meeting (Wed, 1st Nov 2017)

2017-11-01 Thread Samuli Seppänen 


Hi,

Here's the summary of the IRC meeting.

---

COMMUNITY MEETING

Place: #openvpn-meeting on irc.freenode.net
Date: Wednesday 1st Nov 2017
Time: 11:30 CET (10:30 UTC)

Planned meeting topics for this meeting were here:



The next meeting has not been scheduled yet.

Your local meeting time is easy to check from services such as



SUMMARY

cron2, dazo, mattock, ordex, plaisthos and syzzer participated in this
meeting.

--

Discussed how to make testing of unmerged patches easier. While pushing
the patches through buildbot would be optimal, buildslaves run with root
privileges. Hence this approach would be limited to trusted developers.

It was thus decided to tackle this problem with Vagrant, which has base
boxes for all/most operating systems we have in buildbot:



The benefit with Vagrant is that anyone, including untrusted developers,
will be able to use it for testing their patches on other, possibly
esoteric, platforms.

We will start by providing a proof-of-concept Vagrantfile that sets up
OpenVPN build environments on a couple of operating systems, then build
on top of that work. Mattock will try to have the PoC ready by the
hackathon (i.e. late next week).

At this point it also makes sense to take a second look at the existing
Vagrant pull request in here:



It got stuck primarily due to a few bashisms.

--

Discussed our "oldstable" support policy. It was agreed that we do _not_
want to support "oldstable" and "stable" until "stable+1" is released.
Or, in other words, support two stable releases at all times.

We will work on defining an EOL policy for future releases on this page:



It was agreed that 2.3.19 will be the last 2.3 release for which we will
build Windows installers and Debian packages. Subsequent releases of 2.3
will be source-only, and at some point support for 2.3 will be dropped
altogether. Source-only release means that we will provide signed
tarballs but no installers/packages.

That said, if particularly nasty security issues arise we will consider
providing updated installers and packages even for source-only releases.

-- 
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock


(12:28:07) mattock: meeting in 2 minutes
(12:28:17) dazo: good morning :)
(12:28:27) mattock: afternoon :)
(12:28:33) ordex: :D
(12:28:33) mattock: literally
(12:28:39) cron2: mattock: which time zone are you in, right now?
(12:28:43) cron2: UTC+2?
(12:30:30) syzzer: morning :)
(12:30:56) cron2: train connections from munich to karlsruhe suck
(12:31:21) mattock: let's begin
(12:31:36) ordex: sure
(12:31:45) cron2: go!
(12:32:13) dazo ha scelto come argomento: Meeting 2017-11-01 11:30 CET: Agenda 
at https://community.openvpn.net/openvpn/wiki/Topics-2017-11-01
(12:32:26) mattock: ah, good
(12:32:33) mattock: topic #1
(12:32:58) mattock: "Test branches on github, targeting buildslaves"
(12:33:09) cron2: here's a recap of the problem statement: "someone who is not 
me is working on something that might break strange platforms, how to test for 
it?"
(12:33:34) syzzer: yes plz!
(12:33:35) cron2: (I can just log into my buildslaves and test :-) - but maybe 
we can use buildbot to do it)
(12:33:35) mattock: buildbot is definitely the best candidate
(12:33:35) dazo: just thinking aloud ... is it possible to have a host a 
tarball can be pushed to, which gets configured and built by the buildslaves?
(12:34:00) mattock: dazo: possibly
(12:34:13) mattock: another option would an "experimental" branch where we 
would just rewrite history as we please
(12:34:19) cron2: another idea that came to me just now is "I could open up the 
buildslaves to trusted users" (because in the end it does not matter if I give 
you build access, or buildbot can run things as root that you put into a 
tarball)
(12:34:20) dazo: that would probably be easier to manage than "everyone" can 
push to this git repo
(12:34:34) ordex: pushing a tarball or a branch sounds similar. the question 
is: how often can we do this? and can everybody do that unconditionally?
(12:34:57) ordex: or we want to limit one person to be able to push the tarball 
?
(12:34:58) cron2: ordex: most definitely not "everybody"
(12:35:03) mattock: ordex: only those we trust, because otherwise people could 
make buildbot do pretty much anything
(12:35:16) dazo: I was thinking along the lines of ssh/sftp ... where only 
trusted and vetted users gets access
(12:35:25) dazo: (typically us core developers)
(12:35:34) ordex: yep that is reasonable. that can be done on git hub, no? 
maybe with a new repo where only few people have access
(12:35:37) cron2: since the t_client tests run the freshly built openvpn 
executable with sudo, this is full system compromise :-)
(12:35:45) ordex: eheh

[Openvpn-devel] [PATCH 2/2] doxygen: add make target

2017-11-01 Thread Steffan Karger 
Add a make target, such that 'make doxygen' works (both for in-tree and
out-of-tree builds).

Signed-off-by: Steffan Karger 
---
 Makefile.am |   5 +-
 configure.ac|   1 +
 doc/Makefile.am |   2 +
 doc/doxygen/openvpn.doxyfile| 279 
 doc/doxygen/openvpn.doxyfile.in | 279 
 5 files changed, 286 insertions(+), 280 deletions(-)
 delete mode 100644 doc/doxygen/openvpn.doxyfile
 create mode 100644 doc/doxygen/openvpn.doxyfile.in

diff --git a/Makefile.am b/Makefile.am
index 87af724..34632c4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,7 +46,7 @@ EXTRA_DIST = \
contrib \
debug
 
-.PHONY: config-version.h
+.PHONY: config-version.h doxygen
 
 if GIT_CHECKOUT
 BUILT_SOURCES = \
@@ -96,3 +96,6 @@ config-version.h:
else \
rm -f config-version.h.tmp; \
fi
+
+doxygen: doc/doxygen/openvpn.doxyfile
+   doxygen doc/doxygen/openvpn.doxyfile
diff --git a/configure.ac b/configure.ac
index 6f59bae..6570610 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1413,6 +1413,7 @@ AC_CONFIG_FILES([
distro/rpm/Makefile
distro/rpm/openvpn.spec
distro/systemd/Makefile
+   doc/doxygen/openvpn.doxyfile
include/Makefile
src/Makefile
src/compat/Makefile
diff --git a/doc/Makefile.am b/doc/Makefile.am
index dedd1fa..f9e7f4a 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -14,6 +14,8 @@ MAINTAINERCLEANFILES = \
 
 CLEANFILES = openvpn.8.html
 
+SUBDIRS = doxygen
+
 dist_doc_DATA = \
management-notes.txt
 
diff --git a/doc/doxygen/openvpn.doxyfile b/doc/doxygen/openvpn.doxyfile
deleted file mode 100644
index 80cda2b..000
--- a/doc/doxygen/openvpn.doxyfile
+++ /dev/null
@@ -1,279 +0,0 @@
-# Doxyfile 1.5.5
-
-#---
-# Project related configuration options
-#---
-DOXYFILE_ENCODING  = UTF-8
-PROJECT_NAME   = "OpenVPN"
-PROJECT_NUMBER =
-OUTPUT_DIRECTORY   = doxygen
-CREATE_SUBDIRS = NO
-OUTPUT_LANGUAGE= English
-BRIEF_MEMBER_DESC  = YES
-REPEAT_BRIEF   = YES
-ABBREVIATE_BRIEF   = "The $name class" \
- "The $name widget" \
- "The $name file" \
- is \
- provides \
- specifies \
- contains \
- represents \
- a \
- an \
- the
-ALWAYS_DETAILED_SEC= NO
-INLINE_INHERITED_MEMB  = NO
-FULL_PATH_NAMES= YES
-STRIP_FROM_PATH= "."
-STRIP_FROM_INC_PATH=
-SHORT_NAMES= NO
-JAVADOC_AUTOBRIEF  = YES # NO
-QT_AUTOBRIEF   = NO
-MULTILINE_CPP_IS_BRIEF = NO
-DETAILS_AT_TOP = NO
-INHERIT_DOCS   = YES
-SEPARATE_MEMBER_PAGES  = NO
-TAB_SIZE   = 8
-ALIASES=
-OPTIMIZE_OUTPUT_FOR_C  = YES
-OPTIMIZE_OUTPUT_JAVA   = NO
-OPTIMIZE_FOR_FORTRAN   = NO
-OPTIMIZE_OUTPUT_VHDL   = NO
-BUILTIN_STL_SUPPORT= NO
-CPP_CLI_SUPPORT= NO
-SIP_SUPPORT= NO
-DISTRIBUTE_GROUP_DOC   = NO
-SUBGROUPING= YES
-TYPEDEF_HIDES_STRUCT   = NO
-#---
-# Build related configuration options
-#---
-EXTRACT_ALL= YES
-EXTRACT_PRIVATE= YES
-EXTRACT_STATIC = YES
-EXTRACT_LOCAL_CLASSES  = YES
-EXTRACT_LOCAL_METHODS  = YES
-EXTRACT_ANON_NSPACES   = YES
-HIDE_UNDOC_MEMBERS = NO
-HIDE_UNDOC_CLASSES = NO
-HIDE_FRIEND_COMPOUNDS  = NO
-HIDE_IN_BODY_DOCS  = NO
-INTERNAL_DOCS  = NO
-CASE_SENSE_NAMES   = NO
-HIDE_SCOPE_NAMES   = NO
-SHOW_INCLUDE_FILES = YES
-INLINE_INFO= YES
-SORT_MEMBER_DOCS   = YES
-SORT_BRIEF_DOCS= NO
-SORT_GROUP_NAMES   = NO
-SORT_BY_SCOPE_NAME = NO
-GENERATE_TODOLIST  = YES
-GENERATE_TESTLIST  = YES
-GENERATE_BUGLIST   = YES
-GENERATE_DEPRECATEDLIST= YES
-ENABLED_SECTIONS   =
-MAX_INITIALIZER_LINES  = 30
-SHOW_USED_FILES= YES
-SHOW_DIRECTORIES   = NO
-FILE_VERSION_FILTER=
-#---
-# configuration options related to warning and progress messages
-#---
-QUIET  = NO
-WARNINGS   = YES
-WARN_IF_UNDOCUMENTED   = YES
-WARN_IF_DOC_ERROR  = YES
-WARN_NO_PARAMDOC   = NO
-WARN_FORMAT= "$file:$line: $text"
-WARN_LOGFILE   =
-#---
-#

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

2017-11-01 Thread Selva 
Hi

On Wed, Nov 1, 2017 at 4:25 AM, Simon Rozman  wrote:

> Hi,
>
> > Named instances sounds like is a good idea. As you pointed out, Microsoft
> > itself uses command line parameters on service (like -i NAME for SQL
> server)
> > so that looks kosher.
> >
> > There is a problem though: multiple instances also need multiple service
> > names but the service name is hardcoded in the source. In our case even
> > the dispatcher needs to know the service name(s) as we use
> > SERVICE_WIN32_SHARE_PROCESS and share the process between
> > the legacy service[*] and the interactive service.
> >
> > Looks like the service name also will have to be dynamically-generated
> > from the instance name.
>
> There is a simpler solution for that:
>
> 1. Change the order of services listed in `dispatchTable` so the
> interactive
> service is listed first.
>
> 2. All non-default instances could install as `SERVICE_WIN32_OWN_PROCESS`,
> thus SCM will launch them as a separate process picking only the first
> entry
> in the `dispatchTable` ignoring the service name and ignoring all non-first
> entries.
>

Its not just the dispatch table, RegisterServiceCtrlHandlerEx() has to be
called
with the right name of the service, isn't it? Anyway, once you have a tested
patch all this will fall into place..


> > If you want to support multiple installations of openvpn.exe/service,
> > building with a different PACKAGE_NAME can't be avoided, can it?.
> > Instead, if using the "official" executables is ok, why not just use
> > oepnvpnserviceinteractive as is. Does eduVPN really need its own instance
> > of the service?
>
> No, actually eduVPN client is quite happy with the official interactive
> service as it is. Until we hit deployment issues with the
> openvpn-install-2.4.4-I601.exe installer described below.
>
> - When OpenVPN is not already installed, we would like to install it using
> only the minimum feature selection. You see, OpenVPN GUI desktop shortcut
> and
> auto-start in system tray is not desirable, as it will confuse the end
> users
> how to start using eduVPN.
> This can be accomplished using the command line parameters. So far so good.
>
> - When an older version of OpenVPN is already installed, we would like to
> update it. Unfortunately, the installer does not detect currently installed
> features. It just copies the default feature set overwriting previous
> installation. Again, users get the OpenVPN GUI desktop shortcut and system
> tray icon. Even if they were not installed before.
> We could use the command line parameters again to suggest a feature
> selection,
> but if the OpenVPN GUI was installed before, the update would not update
> it,
> leaving behind some sort of Frankenstein OpenVPN installation.
> The installer is missing a command line parameter to tell it "if feature is
> present then update, else leave it absent".
>
> - eduVPN will require latest up-to-date OpenVPN. Should any user require a
> specific older version of OpenVPN to connect to other service providers
> than
> eduVPN, this would be a problem.
>

This becomes an issue only if/when a future version of openvpn becomes
incompatible with the service in 2.4. (Prior to 2.4 there is no interactive
service).
At that point even the official releases will benefit from support for
multiple
instances (same of different versions) of the service.


>
> Because of those issues, we would like to install local OpenVPN - and keep
> it
> up-to-date - using an MSI package. While openvpn.exe and its dependency
> DLLs
> can easily be managed as a local copy, the interactive service cannot -
> without interfering with original OpenVPN installation.
>

Looks like most of these can be solved by packaging improvements. As samuli
wrote, we want to move to MSI, so contributing to that effort would greatly
help.

Thanks,

Selva
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Further enhance async-push feature description

2017-11-01 Thread Gert Doering 
Hi,

finding lost gems in my mailbox...

On Wed, Dec 14, 2016 at 01:45:09PM +0100, Steffan Karger wrote:
> >  AC_ARG_ENABLE(
> > [async-push],
> > -   [AS_HELP_STRING([--enable-async-push], [enable async-push support 
> > @<:@default=no@:>@])],
> > +   [AS_HELP_STRING([--enable-async-push], [enable async-push support for 
> > plugins providing deferred authentication @<:@default=no@:>@])],
> > ,
> > [enable_async_push="no"]
> 
> ACK.
> 
> Related question:  looks both useful and harmless.  Should we enable
> this by default in 2.5?

Skimmed through the code, and generally speaking, this would make sense.

OTOH the code is currently linux only (inotify is not available on FreeBSD,
at least), so this would just change the default but won't get rid of 
any #ifdef, unfortunately... :-(

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


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 0/1] add engine keys keys

2017-11-01 Thread Selva 
Hi,

On Wed, Nov 1, 2017 at 2:18 PM, Steffan Karger  wrote:

> Hi,
>
> On 29-10-17 22:03, Selva wrote:
> > I would like to see new features transparently supported on Windows
> > as well without the need for too much extra code and associated
> > maintenance burden. Our 'cryptoapicert' implementation is already in
> > need of a major re-write to support TLS 1.2 and newer.
>
> Fully agree.  Since cryptoapicert is windows-specific, I actually think
> it would be better to add a 'CNG'[0] implementation to the windows
> wrapper, and make that use management-external-key.  That would probably
> improve UX a lot too, showing users a drop-down to select a key, etc.
> We can then remove the whole deprecated cryptoapi implementation from
> the openvpn core.
>
> > From that point of view, instead of file-based wrapped keys, if a pkcs11
> > compatible API can be used to access TPM (that's possible isn't it?) TPM
> > could be more widely usable without the need of any additional support
> > in openssl or openvpn.
>
> Since this one is transparent, and works as long as the user loads the
> right engine, I don't see any limitations to include this patch.
>

Agreed this is a simple patch that could be used right away. Let's address
'Cryptography Next Gen'  (CNG) and general use of management-external-key
as a separate topic.

I too feel the management interface client and/or plugins is a better place
to add support for platform specific cert/key storage etc., instead of
polluting
openvpn core.

Simon was planning to add support for TLS 1.2+ in eduVPN for keys
stored in Windows certstore -- I suppose that would involve CNG.

Selva
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 0/1] add engine keys keys

2017-11-01 Thread Steffan Karger 
Hi,

On 29-10-17 22:03, Selva wrote:
> I would like to see new features transparently supported on Windows
> as well without the need for too much extra code and associated
> maintenance burden. Our 'cryptoapicert' implementation is already in
> need of a major re-write to support TLS 1.2 and newer.

Fully agree.  Since cryptoapicert is windows-specific, I actually think
it would be better to add a 'CNG'[0] implementation to the windows
wrapper, and make that use management-external-key.  That would probably
improve UX a lot too, showing users a drop-down to select a key, etc.
We can then remove the whole deprecated cryptoapi implementation from
the openvpn core.

> From that point of view, instead of file-based wrapped keys, if a pkcs11
> compatible API can be used to access TPM (that's possible isn't it?) TPM
> could be more widely usable without the need of any additional support
> in openssl or openvpn.

Since this one is transparent, and works as long as the user loads the
right engine, I don't see any limitations to include this patch.

-Steffan

[0]
https://msdn.microsoft.com/en-us/library/windows/desktop/aa376210(v=vs.85).aspx

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel