[Openvpn-devel] [PATCH] options: allow setting default --ca argument value

2017-01-14 Thread Pavel Raiskup
Adding a new --with-ca-bundle configure option.  It's argument is
used as default CA file when no --ca option is specified at
runtime.

This option is primarily designed for systems where users are
allowed to manage trusted authorities for whole system (in one
consolidated file; usually implemented in 'ca-certificates'
package).

Signed-off-by: Pavel Raiskup 
---
 configure.ac  | 5 +
 src/openvpn/options.c | 9 +
 2 files changed, 14 insertions(+)

diff --git a/configure.ac b/configure.ac
index 43487b0..f5e1e63 100644
--- a/configure.ac
+++ b/configure.ac
@@ -308,6 +308,11 @@ AC_ARG_WITH(
[with_plugindir="\$(libdir)/openvpn/plugins"]
 )
 
+AC_ARG_WITH(
+   [ca-bundle],
+   [AS_HELP_STRING([--with-ca-bundle], [use consolidated CA bundle])],
+   [AC_DEFINE_UNQUOTED([DEFAULT_CA_FILE], ["$withval"], [Default --ca 
argument])]
+)
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our 
host])
 case "$host" in
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d9c384e..92d81ae 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3000,6 +3000,15 @@ options_postprocess_mutate(struct options *o)
 }
 #endif
 
+#ifdef DEFAULT_CA_FILE
+if (!o->ca_file && !platform_access(DEFAULT_CA_FILE, R_OK))
+{
+msg(M_WARN, "option '--ca' unspecified; using system bundle '%s'",
+DEFAULT_CA_FILE);
+o->ca_file = DEFAULT_CA_FILE;
+}
+#endif
+
 #if ENABLE_MANAGEMENT
 if (o->http_proxy_override)
 {
-- 
2.9.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2] Fix user's group membership check in interactive service to work with domains

2017-01-14 Thread selva . nair
From: Selva Nair 

Currently the username unqualified by the domain is used to validate
a user which fails for domain users. Instead authorize the user

(i) if the built-in admin group or ovpn_admin group is in the process token
(ii) else if the user's SID is in the built-in admin or ovpn_admin groups

The second check is needed to recognize dynamic updates to group membership
on the local machine that will not be reflected in the token.

These checks do not require connection to a domain controller and will
work even when user is logged in with cached credentials.

Resolves Trac: #810

v2: include the token check as described above

Signed-off-by: Selva Nair 
---
 src/openvpnserv/interactive.c |   2 +-
 src/openvpnserv/validate.c| 171 +++---
 src/openvpnserv/validate.h|   2 +-
 3 files changed, 130 insertions(+), 45 deletions(-)

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index dbe2b9b..9436fba 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1475,7 +1475,7 @@ RunOpenvpn(LPVOID p)
 }
 
 /* Check user is authorized or options are white-listed */
-if (!IsAuthorizedUser(ovpn_user->User.Sid, )
+if (!IsAuthorizedUser(ovpn_user->User.Sid, imp_token, 
settings.ovpn_admin_group)
 && !ValidateOptions(pipe, sud.directory, sud.options))
 {
 goto out;
diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index c9c3855..894c51b 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -49,6 +49,9 @@ static const WCHAR *white_list[] =
 NULL/* last value */
 };
 
+static BOOL IsUserInGroup(PSID sid, const PTOKEN_GROUPS groups, const WCHAR 
*group_name);
+static PTOKEN_GROUPS GetTokenGroups(const HANDLE token);
+
 /*
  * Check workdir\fname is inside config_dir
  * The logic here is simple: we may reject some valid paths if ..\ is in any 
of the strings
@@ -147,21 +150,16 @@ GetBuiltinAdminGroupName(WCHAR *name, DWORD nlen)
 
 /*
  * Check whether user is a member of Administrators group or
- * the group specified in s->ovpn_admin_group
+ * the group specified in ovpn_admin_group
  */
 BOOL
-IsAuthorizedUser(SID *sid, settings_t *s)
+IsAuthorizedUser(PSID sid, const HANDLE token, const WCHAR *ovpn_admin_group)
 {
-LOCALGROUP_USERS_INFO_0 *groups = NULL;
-DWORD nread;
-DWORD nmax;
-WCHAR *tmp = NULL;
 const WCHAR *admin_group[2];
 WCHAR username[MAX_NAME];
 WCHAR domain[MAX_NAME];
 WCHAR sysadmin_group[MAX_NAME];
-DWORD err, len = MAX_NAME;
-int i;
+DWORD len = MAX_NAME;
 BOOL ret = FALSE;
 SID_NAME_USE sid_type;
 
@@ -169,17 +167,9 @@ IsAuthorizedUser(SID *sid, settings_t *s)
 if (!LookupAccountSidW(NULL, sid, username, , domain, , _type))
 {
 MsgToEventLog(M_SYSERR, TEXT("LookupAccountSid"));
-goto out;
-}
-
-/* Get an array of groups the user is member of */
-err = NetUserGetLocalGroups(NULL, username, 0, LG_INCLUDE_INDIRECT, 
(LPBYTE *) ,
-MAX_PREFERRED_LENGTH, , );
-if (err && err != ERROR_MORE_DATA)
-{
-SetLastError(err);
-MsgToEventLog(M_SYSERR, TEXT("NetUserGetLocalGroups"));
-goto out;
+/* not fatal as this is now used only for logging */
+username[0] = '\0';
+domain[0] = '\0';
 }
 
 if (GetBuiltinAdminGroupName(sysadmin_group, _countof(sysadmin_group)))
@@ -192,41 +182,136 @@ IsAuthorizedUser(SID *sid, settings_t *s)
 /* use the default value */
 admin_group[0] = SYSTEM_ADMIN_GROUP;
 }
+admin_group[1] = ovpn_admin_group;
 
-#ifdef UNICODE
-admin_group[1] = s->ovpn_admin_group;
-#else
-tmp = NULL;
-len = MultiByteToWideChar(CP_UTF8, 0, s->ovpn_admin_group, -1, NULL, 0);
-if (len == 0 || (tmp = malloc(len*sizeof(WCHAR))) == NULL)
+PTOKEN_GROUPS token_groups = GetTokenGroups(token);
+for (int i = 0; i < 2; ++i)
 {
-MsgToEventLog(M_SYSERR, TEXT("Failed to convert admin group name to 
WideChar"));
-goto out;
+ret = IsUserInGroup(sid, token_groups, admin_group[i]);
+if (ret)
+{
+MsgToEventLog(M_INFO, TEXT("Authorizing user '%s@%s' by virtue of 
membership in group '%s'"),
+  username, domain, admin_group[i]);
+goto out;
+}
 }
-MultiByteToWideChar(CP_UTF8, 0, s->ovpn_admin_group, -1, tmp, len);
-admin_group[1] = tmp;
-#endif
 
-/* Check if user's groups include any of the admin groups */
-for (i = 0; i < nread; i++)
+out:
+free(token_groups);
+return ret;
+}
+
+/**
+ * Get a list of groups in token.
+ * Returns a pointer to TOKEN_GROUPS struct or NULL on error.
+ * The caller should free the returned pointer.
+ */
+static PTOKEN_GROUPS
+GetTokenGroups(const HANDLE token)
+{
+PTOKEN_GROUPS groups = NULL;
+   

[Openvpn-devel] [PATCH v3] convert *_inline attributes to bool

2017-01-14 Thread Antonio Quartulli
Carrying around the INLINE_TAG is not really efficient,
because it requires a strcmp() to be performed every
time we want to understand if the data is stored inline
or not.

Convert all the *_inline attributes to bool to make the
logic easier and checks more efficient.

Signed-off-by: Antonio Quartulli 
---

Based on master + [PATCH (master)] reformatting: fix style in crypto*.{c, h}

Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to
  be inline. Options not having this permission will fail to be
  parsed if is_inline is true.


Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a
  boolean variable is passed around.
- add print_if_inline() helper function (to misc.c/h) to make sure we
  never print the inline data, but only the INLINE tag. Such function
  checks also for NULL pointers;
- make sure print_if_inline() is always used when printing possibly
  inline data;
- remove the INLINE_TAG from the options parsing logic at all. Now a
  boolean variable is passed around.
- fix alignment error in comment;
- remove CHKACC_INLINE from check_file_access() logic: this function
  is now not invoked at all in case of inline data.


 src/openvpn/crypto.c  |  25 +++--
 src/openvpn/crypto.h  |   5 +-
 src/openvpn/misc.c|  17 ++-
 src/openvpn/misc.h|  15 ++-
 src/openvpn/options.c | 281 ++
 src/openvpn/options.h |  21 ++--
 src/openvpn/plugin.c  |   6 +-
 src/openvpn/plugin.h  |   3 +-
 src/openvpn/push.c|   2 +-
 src/openvpn/push.h|   3 +-
 src/openvpn/ssl.c |   6 +-
 src/openvpn/ssl_backend.h |  64 ++-
 src/openvpn/ssl_common.h  |   2 +-
 src/openvpn/ssl_mbedtls.c |  63 +--
 src/openvpn/ssl_openssl.c |  91 ---
 src/openvpn/tls_crypt.c   |   2 +-
 src/openvpn/tls_crypt.h   |   9 +-
 17 files changed, 343 insertions(+), 272 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 944f4c5..9a7745f 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -36,6 +36,7 @@
 #include "crypto.h"
 #include "error.h"
 #include "integer.h"
+#include "misc.h"
 #include "platform.h"
 
 #include "memdbg.h"
@@ -1165,27 +1166,26 @@ test_crypto(struct crypto_options *co, struct frame 
*frame)
 
 void
 crypto_read_openvpn_key(const struct key_type *key_type, struct key_ctx_bi 
*ctx,
-const char *key_file, const char *key_inline,
+const char *key_file, bool key_inline,
 const int key_direction, const char *key_name,
 const char *opt_name)
 {
 struct key2 key2;
 struct key_direction_state kds;
 char log_prefix[128] = { 0 };
+unsigned int flags = RKF_MUST_SUCCEED;
 
 if (key_inline)
 {
-read_key_file(, key_inline, RKF_MUST_SUCCEED | RKF_INLINE);
-}
-else
-{
-read_key_file(, key_file, RKF_MUST_SUCCEED);
+flags |= RKF_INLINE;
 }
+read_key_file(, key_file, flags);
 
 if (key2.n != 2)
 {
 msg(M_ERR, "File '%s' does not have OpenVPN Static Key format.  Using "
-"free-form passphrase file is not supported anymore.", key_file);
+"free-form passphrase file is not supported anymore.",
+print_if_inline(key_file, key_inline));
 }
 
 /* check for and fix highly unlikely key problems */
@@ -1225,7 +1225,6 @@ read_key_file(struct key2 *key2, const char *file, const 
unsigned int flags)
 struct buffer in;
 int fd, size;
 uint8_t hex_byte[3] = {0, 0, 0};
-const char *error_filename = file;
 
 /* parse info */
 const unsigned char *cp;
@@ -1264,7 +1263,6 @@ read_key_file(struct key2 *key2, const char *file, const 
unsigned int flags)
 /* 'file' is a string containing ascii representation of key */
 size = strlen(file) + 1;
 buf_set_read(, (const uint8_t *)file, size);
-error_filename = INLINE_FILE_TAG;
 }
 else
 {
@@ -1372,7 +1370,8 @@ read_key_file(struct key2 *key2, const char *file, const 
unsigned int flags)
 {
 msg(M_FATAL,
 isprint(c) ? printable_char_fmt : unprintable_char_fmt,
-c, line_num, error_filename, count, onekeylen, keylen);
+c, line_num, print_if_inline(file, flags & RKF_INLINE),
+count, onekeylen, keylen);
 }
 }
 ++line_index;
@@ -1394,14 +1393,16 @@ read_key_file(struct key2 *key2, const char *file, 
const unsigned int flags)
 {
 msg(M_FATAL,
 "Insufficient key material or header text not found in file 
'%s' 

[Openvpn-devel] [PATCH v3] More broadly enforce Allman style and braces-around-conditionals

2017-01-14 Thread Steffan Karger
We want { and } aligned, which means also adding a newline between each
for() and {, while() and {, etc.

Also, we agreed to always use braces with conditionals.  The previous
uncrustify config added these for if()s, now also add these for while()
and for().

Signed-off-by: Steffan Karger 
---
This patch replaces the "Add nl_for_brace=add to uncrustify.conf" patch.

v2: also add nl_do_brace=add and mod_full_brace_do=add
v3: rebase onto current master

 dev-tools/uncrustify.conf  |  8 +
 src/compat/compat-daemon.c |  3 +-
 src/compat/compat-dirname.c|  7 +++-
 src/compat/compat-inet_ntop.c  |  3 +-
 src/compat/compat-inet_pton.c  |  3 +-
 src/compat/compat-versionhelpers.h | 36 ++---
 src/openvpn/argv.c | 10 ++
 src/openvpn/base64.c   | 11 +--
 src/openvpn/buffer.c   |  9 +-
 src/openvpn/buffer.h   |  2 ++
 src/openvpn/comp-lz4.c |  3 +-
 src/openvpn/compstub.c |  3 +-
 src/openvpn/console.c  |  6 ++--
 src/openvpn/crypto.c   | 18 +++
 src/openvpn/crypto.h   |  3 +-
 src/openvpn/crypto_mbedtls.h   |  3 +-
 src/openvpn/crypto_openssl.c   |  9 --
 src/openvpn/cryptoapi.c|  9 --
 src/openvpn/dhcp.c | 11 +--
 src/openvpn/error.c|  3 +-
 src/openvpn/error.h|  3 +-
 src/openvpn/event.c|  4 +++
 src/openvpn/fragment.c |  9 +-
 src/openvpn/gremlin.c  | 15 ++---
 src/openvpn/httpdigest.c   |  3 +-
 src/openvpn/init.c | 11 ++-
 src/openvpn/interval.h |  3 +-
 src/openvpn/list.c |  6 ++--
 src/openvpn/lzo.c  |  3 +-
 src/openvpn/manage.c   | 15 +++--
 src/openvpn/mbuf.c |  3 +-
 src/openvpn/misc.c |  9 +-
 src/openvpn/mroute.c   |  3 +-
 src/openvpn/mss.c  |  3 +-
 src/openvpn/mtcp.c |  3 +-
 src/openvpn/multi.c| 14 ++--
 src/openvpn/ntlm.c | 11 +--
 src/openvpn/occ.c  |  3 +-
 src/openvpn/openvpn.c  |  6 ++--
 src/openvpn/options.c  | 52 +++---
 src/openvpn/otime.h|  3 +-
 src/openvpn/packet_id.c|  3 +-
 src/openvpn/perf.c |  5 ++-
 src/openvpn/perf.h |  9 --
 src/openvpn/pkcs11.c   | 66 +-
 src/openvpn/plugin.c   | 11 ++-
 src/openvpn/pool.c |  2 ++
 src/openvpn/proxy.c| 10 --
 src/openvpn/reliable.c |  7 +++-
 src/openvpn/route.c| 13 +++-
 src/openvpn/route.h|  3 +-
 src/openvpn/schedule.c |  4 +++
 src/openvpn/session_id.c   |  3 +-
 src/openvpn/shaper.c   |  3 +-
 src/openvpn/socket.c   | 12 +--
 src/openvpn/socket.h   | 24 +-
 src/openvpn/ssl.c  | 26 +++
 src/openvpn/ssl_mbedtls.c  |  8 +++--
 src/openvpn/ssl_openssl.c  |  3 +-
 src/openvpn/ssl_verify.c   |  6 
 src/openvpn/ssl_verify_mbedtls.c   |  2 ++
 src/openvpn/ssl_verify_openssl.c   |  3 +-
 src/openvpn/tls_crypt.c|  6 ++--
 src/openvpn/tun.c  | 26 +++
 src/openvpn/win32.c|  8 +++--
 src/openvpnserv/automatic.c|  3 +-
 src/openvpnserv/interactive.c  |  5 ++-
 src/plugins/auth-pam/utils.c   |  5 ++-
 src/plugins/down-root/down-root.c  |  2 ++
 69 files changed, 477 insertions(+), 143 deletions(-)

diff --git a/dev-tools/uncrustify.conf b/dev-tools/uncrustify.conf
index 95e0b2a..d8ea870 100644
--- a/dev-tools/uncrustify.conf
+++ b/dev-tools/uncrustify.conf
@@ -9,6 +9,11 @@ nl_brace_else=add
 nl_elseif_brace=add
 nl_else_brace=add
 nl_else_if=remove
+nl_for_brace=add
+nl_while_brace=add
+nl_switch_brace=add
+nl_fdef_brace=add
+nl_do_brace=add
 sp_func_proto_paren=Remove
 sp_func_def_paren=Remove
 sp_func_call_paren=Remove
@@ -44,6 +49,9 @@ nl_after_func_proto=2
 # Always use scoping braces for conditionals
 mod_full_brace_if=add
 mod_full_brace_if_chain=false
+mod_full_brace_while=add
+mod_full_brace_for=add
+mod_full_brace_do=add
 
 # Annotate #else and #endif statements
 mod_add_long_ifdef_endif_comment=20
diff --git a/src/compat/compat-daemon.c b/src/compat/compat-daemon.c
index 5093942..b54e813 100644
--- a/src/compat/compat-daemon.c
+++ b/src/compat/compat-daemon.c
@@ -58,7 +58,8 @@ int
 daemon(int nochdir, int noclose)
 {
 #if defined(HAVE_FORK) && defined(HAVE_SETSID)
-switch (fork()) {
+switch (fork())
+{
 case -1:
 return (-1);
 
diff --git a/src/compat/compat-dirname.c b/src/compat/compat-dirname.c
index 7687108..7747dbd