[Openvpn-devel] [PATCH 1/2] Fix interactive service ignoring stop command if openvpn is running
Make the exit event not auto-reset so that the signal propagates to
all worker threads and finally to the main thread.
Fixes Trac #666
Signed-off-by: Selva Nair
---
src/openvpnserv/interactive.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 22239b2..39397d1 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1289,7 +1289,7 @@ ServiceStartInteractive (DWORD dwArgc, LPTSTR *lpszArgv)
goto out;
io_event = InitOverlapped (&overlapped);
- exit_event = CreateEvent (NULL, FALSE, FALSE, NULL);
+ exit_event = CreateEvent (NULL, TRUE, FALSE, NULL);
if (!exit_event || !io_event)
{
error = MsgToEventLog (M_SYSERR, TEXT("Could not create event"));
@@ -1356,6 +1356,7 @@ ServiceStartInteractive (DWORD dwArgc, LPTSTR *lpszArgv)
{
/* exit event signaled */
CloseHandleEx (&pipe);
+ ResetEvent (exit_event);
error = NO_ERROR;
break;
}
--
1.7.10.4
[Openvpn-devel] [PATCH 2/2] Use appropriate buffer size for WideCharToMultiByte output in interactive.c
A widechar can potentially take more than 2 bytes in UTF-8.
Signed-off-by: Selva Nair
---
src/openvpnserv/interactive.c |7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 39397d1..6a7227b 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1063,17 +1063,16 @@ RunOpenvpn (LPVOID p)
CloseHandleEx (&stdin_read);
CloseHandleEx (&svc_pipe);
- DWORD input_size = wcslen (sud.std_input) * 2;
- if (input_size)
+ DWORD input_size = WideCharToMultiByte (CP_UTF8, 0, sud.std_input, -1, NULL,
0, NULL, NULL);
+ LPSTR input = NULL;
+ if (input_size && (input = malloc (input_size)))
{
DWORD written;
- LPSTR input = malloc (input_size);
WideCharToMultiByte (CP_UTF8, 0, sud.std_input, -1, input, input_size,
NULL, NULL);
WriteFile (stdin_write, input, strlen (input), &written, NULL);
free (input);
}
-
while (TRUE)
{
DWORD bytes = PeekNamedPipeAsync (ovpn_pipe, 1, &exit_event);
--
1.7.10.4
[Openvpn-devel] [PATCH applied] Re: Handle localized Administrators group name in windows
ACK, thanks (based on "stare at code and MSDN docs" and Leonardo's testing). Your patch has been applied to the master branch. commit 6370f703573c6284e0b3c5935ab204285cdda8e6 Author: Selva Nair List-Post: openvpn-devel@lists.sourceforge.net Date: Sat Mar 5 14:39:56 2016 -0500 Handle localized Administrators group name in windows Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <1457206796-11863-1-git-send-email-selva.n...@gmail.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/11316 Signed-off-by: Gert Doering -- kind regards, Gert Doering
[Openvpn-devel] [PATCH applied] Re: Fix interactive service ignoring stop command if openvpn is running
ACK. https://msdn.microsoft.com/en-us/library/windows/desktop/ms682396(v=vs.85).aspx explains the behaviour described in trac#666 (only a single thread is stopping, all the rest does not receive the event due to auto-reset behaviour). I'm not sure if the ResetEvent() call is actually needed but it's certainly good style to acknowledge in the master thread after all clients are done. Your patch has been applied to the master branch. commit 239d09938b300f8eafa12bfb8c43373f0215f7bd Author: Selva Nair List-Post: openvpn-devel@lists.sourceforge.net Date: Sun Mar 6 00:19:19 2016 -0500 Fix interactive service ignoring stop command if openvpn is running Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <1457241559-23374-1-git-send-email-selva.n...@gmail.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/11317 Signed-off-by: Gert Doering -- kind regards, Gert Doering
[Openvpn-devel] [PATCH applied] Re: Use appropriate buffer size for WideCharToMultiByte output in interactive.c
ACK. https://msdn.microsoft.com/en-us/library/windows/desktop/dd374130(v=vs.85).aspx explains about WideCharToMultiByte() usage with cbMultiByte==0. I'm not sure if I understand in which scenarios data is fed to the nascent openvpn.exe on stdin - buf if done at all, we should better do it right :-) - I do wonder, though, if WriteFile() could block here, leading to a dead worker thread... all other pipe operations seem to go through AsyncPipeOp() - but that is orthogonal to this fix) Your patch has been applied to the master branch. commit 3654d953eb0bf40fb8c9e1fbaa3de1dd898dcbab Author: Selva Nair List-Post: openvpn-devel@lists.sourceforge.net Date: Sun Mar 6 00:22:02 2016 -0500 Use appropriate buffer size for WideCharToMultiByte output in interactive.c Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <1457241722-23433-1-git-send-email-selva.n...@gmail.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/11318 Signed-off-by: Gert Doering -- kind regards, Gert Doering
[Openvpn-devel] [PATCH] Make AEAD modes work with OpenSSL 1.0.1-1.0.1c
The 'nobody uses OpenSSL 1.0.1-1.0.1c'-gamble in commit 66407e11 (add AEAD
support) did not turn out well; apparently Ubuntu 12.04 LTS ships with a
broken OpenSSL 1.0.1. Since this is still a popular platform, re-add the
fixup code, now with a clear version check so it's easy to remove once we
drop support for OpenSSL 1.0.1.
Signed-off-by: Steffan Karger
---
src/openvpn/crypto.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index bd86679..269ec4b 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -450,6 +450,13 @@ openvpn_decrypt_aead (struct buffer *buf, struct buffer
work,
tag_ptr = BPTR(buf);
ASSERT (buf_advance (buf, tag_size));
dmsg (D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex (tag_ptr, tag_size, 0,
&gc));
+#if defined(ENABLE_CRYPTO_OPENSSL) && OPENSSL_VERSION_NUMBER < 0x10001040L
+ /* OpenSSL <= 1.0.1c bug requires set tag before processing ciphertext */
+ if (!EVP_CIPHER_CTX_ctrl (ctx->cipher, EVP_CTRL_GCM_SET_TAG, tag_size,
tag_ptr))
+{
+ CRYPT_ERROR ("setting tag failed");
+}
+#endif
if (buf->len < 1)
{
--
2.5.0
[Openvpn-devel] [PATCH applied] Re: Make AEAD modes work with OpenSSL 1.0.1-1.0.1c
ACK. Tested on Ubuntu 12.04, makes "make check" succeed for AES*GCM modes (and has no effect on systems with older/newer OpenSSL versions). Your patch has been applied to the master branch. commit 13de0103ea361e2be24ab8b16f5be269c6ab7496 Author: Steffan Karger List-Post: openvpn-devel@lists.sourceforge.net Date: Sun Mar 6 10:31:55 2016 +0100 Make AEAD modes work with OpenSSL 1.0.1-1.0.1c Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1457256715-4467-1-git-send-email-stef...@karger.me> URL: http://article.gmane.org/gmane.network.openvpn.devel/11322 Signed-off-by: Gert Doering -- kind regards, Gert Doering
Re: [Openvpn-devel] [PATCH] Implement inlining of crl files
Hi,
On Sat, Mar 5, 2016 at 3:34 PM, Arne Schwabe wrote:
> While crl files can change regulary and it is usually not a good idea to
> statically include them into config files, handling multiple files and
> updating files on mobile files is tiresome/problematic. Inlining a static
> version of the crl file is better in these use cases than to use no crl at
> all.
>
> OpenVPN 3 already supports inlining crl-verify, so is already
> used in config files.
Feature-ACK, but some remarks:
> @@ -6498,7 +6498,7 @@ X509_1_C=KG
> .\"*
> .SH INLINE FILE SUPPORT
> OpenVPN allows including files in the main configuration for the
> -.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12,
> \-\-secret
> +.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12,
> \-\-secret, \-\-crl-verify
> and
> .B \-\-tls\-auth
> options.
Real nitpicking, but now that I'm sending mail anyway: this exceeds
the 80-char 'limit', while the surrounding text seems to obey. Would
you mind putting .B \-\-crl-verify on a new line instead?
> @@ -2729,10 +2729,11 @@ options_postprocess_filechecks (struct options
> *options)
> "--pkcs12");
>
>if (options->ssl_flags & SSLF_CRL_VERIFY_DIR)
> -errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE,
> options->crl_file, R_OK|X_OK,
> +errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE,
> +options->crl_file, R_OK|X_OK,
> "--crl-verify directory");
This looks like an accidental formatting change.
>else
> -errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE,
> options->crl_file, R_OK,
> +errs |= check_file_access_chroot (options->chroot_dir,
> CHKACC_FILE|CHKACC_INLINE, options->crl_file, R_OK,
> "--crl-verify");
This line too becomes quite long (it at least exceeds my default
editor window width ;) ).
> --- a/src/openvpn/ssl_verify_backend.h
> +++ b/src/openvpn/ssl_verify_backend.h
> @@ -254,7 +254,7 @@ result_t x509_write_pem(FILE *peercert_file,
> openvpn_x509_cert_t *peercert);
> * certificate or does not contain an entry for it.
> * \c FAILURE otherwise.
> */
> -result_t x509_verify_crl(const char *crl_file, openvpn_x509_cert_t *cert,
> -const char *subject);
> +result_t x509_verify_crl(const char *crl_file, const char *crl_inline,
> + openvpn_x509_cert_t *cert, const char *subject);
Could you add the new parameter to the doxygen too?
> --- a/src/openvpn/ssl_verify_openssl.c
> +++ b/src/openvpn/ssl_verify_openssl.c
> @@ -578,7 +578,7 @@ x509_write_pem(FILE *peercert_file, X509 *peercert)
> * check peer cert against CRL
> */
> result_t
> -x509_verify_crl(const char *crl_file, X509 *peer_cert, const char *subject)
> +x509_verify_crl(const char *crl_file, const char* crl_inline, X509
> *peer_cert, const char *subject)
This becomes a quite long line.
> --- a/src/openvpn/ssl_verify_polarssl.c
> +++ b/src/openvpn/ssl_verify_polarssl.c
> @@ -359,18 +359,29 @@ x509_write_pem(FILE *peercert_file, x509_crt *peercert)
> * check peer cert against CRL
> */
> result_t
> -x509_verify_crl(const char *crl_file, x509_crt *cert, const char *subject)
> +x509_verify_crl(const char *crl_file, const char* crl_inline, x509_crt
> *cert, const char *subject)
This becomes a quite long line.
> {
>result_t retval = FAILURE;
>x509_crl crl = {0};
>struct gc_arena gc = gc_new();
>char *serial;
>
> - if (!polar_ok(x509_crl_parse_file(&crl, crl_file)))
> + if (!strcmp (cert_file, INLINE_FILE_TAG) && crl_inline)
This doesn't compile. I think you meant to write crl_file there, not cert_file.
> {
> - msg (M_WARN, "CRL: cannot read CRL from file %s", crl_file);
> - goto end;
> + if (!polar_ok(x509_crl_parse_(&crl, crl_inline, strlen(crl_inline
This doesn't compile. The final _ of x509_crl_parse_() should be removed.
> +{
> + msg (M_WARN, "CRL: cannot read CRL from file %s", crl_file);
> + goto end;
> +}
This warning message does not make sense for an inline crl. Change to
"cannot parse inline CRL"?
After the above changes, I tested the code and it works as expected.
-Steffan
[Openvpn-devel] [PATCH] Only include aead encrypt/decrypt functions if AEAD modes are supported
This fixes the build for OpenSSL < 1.0.1 (broken by commit 3654d953),
which has no AEAD support.
Signed-off-by: Steffan Karger
---
src/openvpn/crypto.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 269ec4b..f15ac35 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -86,6 +86,7 @@ memcmp_constant_time (const void *a, const void *b, size_t
size) {
static void
openvpn_encrypt_aead (struct buffer *buf, struct buffer work,
struct crypto_options *opt) {
+#ifdef HAVE_AEAD_CIPHER_MODES
struct gc_arena gc;
int outlen = 0;
const struct key_ctx *ctx = &opt->key_ctx_bi.encrypt;
@@ -173,6 +174,9 @@ err:
crypto_clear_error();
buf->len = 0;
goto cleanup;
+#else /* HAVE_AEAD_CIPHER_MODES */
+ ASSERT (0);
+#endif
}
static void
@@ -385,6 +389,7 @@ openvpn_decrypt_aead (struct buffer *buf, struct buffer
work,
struct crypto_options *opt, const struct frame* frame,
const uint8_t *ad_start)
{
+#ifdef HAVE_AEAD_CIPHER_MODES
static const char error_prefix[] = "AEAD Decrypt error";
struct packet_id_net pin = { 0 };
const struct key_ctx *ctx = &opt->key_ctx_bi.decrypt;
@@ -511,6 +516,10 @@ openvpn_decrypt_aead (struct buffer *buf, struct buffer
work,
buf->len = 0;
gc_free (&gc);
return false;
+#else /* HAVE_AEAD_CIPHER_MODES */
+ ASSERT (0);
+ return false;
+#endif
}
/*
--
2.5.0
[Openvpn-devel] [PATCH applied] Re: hardening: add safe FD_SET() wrapper openvpn_fd_set()
ACK. The FD_SET() calls in event.c should all be safe as there are checks already, but many of the others are not checked - "usually" we're well in the range of FD_SETSIZE with our file descriptors, and "usually" platforms are using poll() anyway, but in the exceptional case, having a clear ASSERT() is much better than silent malfunction or a crash. As explanation for the archives why this is #ifndef WIN32 - the windows file handles do not follow the same "start with 0, count up" approach as unix file descriptors, so the windows FD_SET() API is actually more similar to unix poll() - the file descriptor is added to a list, instead of just setting the bit in an array (of FD_SETSIZE bits) that corresponds to the numeric value. Your patch has been applied to the master and release/2.3 branch. commit e0b3fd49e2b5bba8cb57419a13cb75b56ac91b94 (master) commit 1746908f66f5517a525ee2c114a0f7104c29dfad (release/2.3) Author: Steffan Karger List-Post: openvpn-devel@lists.sourceforge.net Date: Thu Mar 3 10:22:48 2016 +0100 hardening: add safe FD_SET() wrapper openvpn_fd_set() Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1456996968-29472-1-git-send-email-steffan.kar...@fox-it.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/11285 Signed-off-by: Gert Doering -- kind regards, Gert Doering
[Openvpn-devel] [PATCH applied] Re: Only include aead encrypt/decrypt functions if AEAD modes are supported
ACK, thanks for the quick followup. As discussed on IRC. Your patch has been applied to the master branch. commit 71d89065ad56dda19996deeeffeddcea632b8349 Author: Steffan Karger List-Post: openvpn-devel@lists.sourceforge.net Date: Sun Mar 6 13:09:50 2016 +0100 Only include aead encrypt/decrypt functions if AEAD modes are supported Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1457266190-27228-1-git-send-email-stef...@karger.me> URL: http://article.gmane.org/gmane.network.openvpn.devel/11325 Signed-off-by: Gert Doering -- kind regards, Gert Doering
Re: [Openvpn-devel] [PATCH applied] Re: Use appropriate buffer size for WideCharToMultiByte output in interactive.c
Hi, On Sun, Mar 6, 2016 at 4:31 AM, Gert Doering wrote: > > I'm not sure if I understand in which scenarios data is fed to the > nascent openvpn.exe on stdin - buf if done at all, we should better do > it right :-) - I do wonder, though, if WriteFile() could block here, > leading to a dead worker thread... all other pipe operations seem to > go through AsyncPipeOp() - but that is orthogonal to this fix) This is used for writing the management password to stdin. The GUI sends an "ascii" password, but a client could potentially use some char set that needs 3 or 4 bytes per char. And you are right, the write should not block. We don't see any deadlock even when openvpn errros out early, because of buffering, I suppose.. But bettter to make this non-blocking. Selva
Re: [Openvpn-devel] [PATCH v2 1/2] Refactor and move the block-outside-dns code to a new file (block_dns.[ch])
Hi, On Thu, Feb 25, 2016 at 10:24:50PM -0500, Selva Nair wrote: > - Move the core of win_wfp_block_dns() to a new function > - Remove globals and make it independent of the rest of the code Trying to compile this on mingw 32bit, block_dns.c blows up for me unless I add two more header files to get definitions for ADDRESS_FAMILY and SOCKADDR_INET (which are referenced by iphlpapi.c) --- a/src/openvpn/block_dns.c +++ b/src/openvpn/block_dns.c @@ -29,6 +29,8 @@ #include #include #include +#include +#include #include #include "block_dns.h" ... with these changes, both openvpn.exe and openvpnserv.exe compile and link nicely. MS documentation says one should include , but that is not sufficient for me. So, question 1: is this a mingw issue, or just a 32bit windows requirement and you only tested on 64bit? question 2: shall I just add these two header files and take note in the ACK-and-merge e-mail, or do you want to re-send a v3? 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
Re: [Openvpn-devel] [PATCH applied] Re: Use appropriate buffer size for WideCharToMultiByte output in interactive.c
Hi, On Sun, Mar 06, 2016 at 08:34:38AM -0500, Selva Nair wrote: > On Sun, Mar 6, 2016 at 4:31 AM, Gert Doering wrote: > > > I'm not sure if I understand in which scenarios data is fed to the > > nascent openvpn.exe on stdin - buf if done at all, we should better do > > it right :-) - I do wonder, though, if WriteFile() could block here, > > leading to a dead worker thread... all other pipe operations seem to > > go through AsyncPipeOp() - but that is orthogonal to this fix) > > > This is used for writing the management password to stdin. The GUI sends an > "ascii" password, but a client could potentially use some char set that > needs 3 or 4 bytes per char. Ah! Tanks for the explanation, this is showing my lack of familiarity with using the management interface in a programmatic fashion... (I use it on the server side and manually telnet to it, having the password in a file) > And you are right, the write should not block. We don't see any deadlock > even when openvpn errros out early, because of buffering, I suppose.. But > bettter to make this non-blocking. For "short" writes, it shouldn't block, no. On a unix pipe, you'd get blocking if the buffers fill up and the reader is just sitting there and not fetching the data - if the reader *dies*, the server would get an error back (since nobody could ever read it). Probably not very important, just wondering. 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
Re: [Openvpn-devel] [PATCH v2 1/2] Refactor and move the block-outside-dns code to a new file (block_dns.[ch])
On Sun, Mar 6, 2016 at 8:44 AM, Gert Doering wrote: > Trying to compile this on mingw 32bit, block_dns.c blows up for me > unless I add two more header files to get definitions for ADDRESS_FAMILY > and SOCKADDR_INET (which are referenced by iphlpapi.c) > > --- a/src/openvpn/block_dns.c > +++ b/src/openvpn/block_dns.c > @@ -29,6 +29,8 @@ > #include > #include > #include > +#include > +#include > #include > #include "block_dns.h" > > ... with these changes, both openvpn.exe and openvpnserv.exe compile > and link nicely. > It must have been my sloppiness. I copied the relevant snippets from win32.c and only the headers needed to compile cleanly. Did not check the docs thoroughly.. > > MS documentation says one should include , but that is not > sufficient for me. > > So, question 1: is this a mingw issue, or just a 32bit windows requirement > and you only tested on 64bit? > Strange thing is that it compiles and links without error (even with -Wall) using mingw I have here (uses gcc 4.6.3). > > question 2: shall I just add these two header files and take note in > the ACK-and-merge e-mail, or do you want to re-send a v3? > Please add the headers and the note. Thanks. Selva
Re: [Openvpn-devel] [PATCH v2 1/2] Refactor and move the block-outside-dns code to a new file (block_dns.[ch])
On Sun, Mar 6, 2016 at 9:20 AM, Selva Nair wrote: > >> So, question 1: is this a mingw issue, or just a 32bit windows requirement >> and you only tested on 64bit? >> > > Strange thing is that it compiles and links without error (even with > -Wall) using mingw I have here (uses gcc 4.6.3). > For got to add, I meant the above for both 32 bit and 64 bit.
[Openvpn-devel] [PATCH applied] Re: Refactor and move the block-outside-dns code to a new file (block_dns.[ch])
ACK. This is a bit bigger than "just move to new file" because it gets rid of global variables at the same time (h_EngineHandle etc.) and simplifies the error handling by introducing a CHECK_ERROR() macro - but as far as I could figure out, it's still doing the same things. I have not actually tested it, just "stare at code" - Samuli's buildbot will make us a new installer based on this, and people using block_dns functionality should thoroughly test it, with and without the iservice running (iservice patch next). As discussed on the list, I've added and to block_dns.c Your patch has been applied to the master branch. commit 6a33a34dee8f3b574275d8df1635fb550ec054f3 Author: Selva Nair List-Post: openvpn-devel@lists.sourceforge.net Date: Thu Feb 25 22:24:50 2016 -0500 Refactor and move the block-outside-dns code to a new file (block_dns.[ch]) Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <1456457091-3872-1-git-send-email-selva.n...@gmail.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/11264 Signed-off-by: Gert Doering -- kind regards, Gert Doering
[Openvpn-devel] [PATCH applied] Re: Add support for block-outside-dns through the interactive service
ACK. The OpenVPN changes are fairly straightforward and fully in-line with the other service-using modules. Same for the iservice changes - look reasonable and are fully in-line with the the other function calls. Again, I did not test, just stared at the code and did a test compile (mingw 32bit on ubuntu 14.04) which worked. Your patch has been applied to the master branch. commit 2282b1be7968ef44accde705ccc64addab6d77ba Author: Selva Nair List-Post: openvpn-devel@lists.sourceforge.net Date: Thu Feb 25 22:24:51 2016 -0500 Add support for block-outside-dns through the interactive service Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <1456457091-3872-2-git-send-email-selva.n...@gmail.com> URL: http://article.gmane.org/gmane.network.openvpn.devel/11265 Signed-off-by: Gert Doering -- kind regards, Gert Doering
[Openvpn-devel] [PATCH v2] Implement inlining of crl files
While crl files can change regulary and it is usually not a good idea to
statically include them into config files, handling multiple files and updating
files on mobile files is tiresome/problematic. Inlining a static version of the
crl file is better in these use cases than to use no crl at all.
OpenVPN 3 already supports inlining crl-verify, so is already used
in config files.
V2: Fixed PolarSSL and made formatting respect the 80 column limit
---
doc/openvpn.8 | 3 ++-
src/openvpn/init.c| 1 +
src/openvpn/options.c | 7 ++-
src/openvpn/options.h | 1 +
src/openvpn/ssl_common.h | 1 +
src/openvpn/ssl_verify.c | 2 +-
src/openvpn/ssl_verify_backend.h | 5 +++--
src/openvpn/ssl_verify_openssl.c | 8 ++--
src/openvpn/ssl_verify_polarssl.c | 20
9 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 628d877..decffc7 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -6490,7 +6490,8 @@ X509_1_C=KG
.\"*
.SH INLINE FILE SUPPORT
OpenVPN allows including files in the main configuration for the
-.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12, \-\-secret
+.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12, \-\-secret,
+.B \-\-crl-verify
and
.B \-\-tls\-auth
options.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index cb73a3d..33a1420 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2323,6 +2323,7 @@ do_init_crypto_tls (struct context *c, const unsigned int
flags)
to.verify_x509_type = (options->verify_x509_type & 0xff);
to.verify_x509_name = options->verify_x509_name;
to.crl_file = options->crl_file;
+ to.crl_file_inline = options->crl_file_inline;
to.ssl_flags = options->ssl_flags;
to.ns_cert_type = options->ns_cert_type;
memmove (to.remote_cert_ku, options->remote_cert_ku, sizeof
(to.remote_cert_ku));
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 02def3a..e7fd358 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6783,12 +6783,17 @@ add_option (struct options *options,
VERIFY_PERMISSION (OPT_P_GENERAL);
options->cipher_list = p[1];
}
- else if (streq (p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2], "dir"))
|| !p[2]) && !p[3])
+ else if (streq (p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2], "dir"))
+ || (p[2] && streq (p[1], INLINE_FILE_TAG) ) || !p[2]) &&
!p[3])
{
VERIFY_PERMISSION (OPT_P_GENERAL);
if (p[2] && streq(p[2], "dir"))
options->ssl_flags |= SSLF_CRL_VERIFY_DIR;
options->crl_file = p[1];
+ if (streq (p[1], INLINE_FILE_TAG) && p[2])
+ {
+ options->crl_file_inline = p[2];
+ }
}
else if (streq (p[0], "tls-verify") && p[1])
{
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 23d3992..8a26e14 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -511,6 +511,7 @@ struct options
const char *ca_file_inline;
const char *cert_file_inline;
const char *extra_certs_file_inline;
+ const char *crl_file_inline;
char *priv_key_file_inline;
const char *dh_file_inline;
const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12
file */
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index eaf4a91..334ccb0 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -247,6 +247,7 @@ struct tls_options
int verify_x509_type;
const char *verify_x509_name;
const char *crl_file;
+ const char *crl_file_inline;
int ns_cert_type;
unsigned remote_cert_ku[MAX_PARMS];
const char *remote_cert_eku;
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index ccfa9d2..ea381f8 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -690,7 +690,7 @@ verify_cert(struct tls_session *session,
openvpn_x509_cert_t *cert, int cert_dep
}
else
{
- if (SUCCESS != x509_verify_crl(opt->crl_file, cert, subject))
+ if (SUCCESS != x509_verify_crl(opt->crl_file, opt->crl_file_inline,
cert, subject))
goto cleanup;
}
}
diff --git a/src/openvpn/ssl_verify_backend.h b/src/openvpn/ssl_verify_backend.h
index 4e9ad60..5001764 100644
--- a/src/openvpn/ssl_verify_backend.h
+++ b/src/openvpn/ssl_verify_backend.h
@@ -248,13 +248,14 @@ result_t x509_write_pem(FILE *peercert_file,
openvpn_x509_cert_t *peercert);
*
* @param crl_file File name of the CRL file
* @param cert Certificate to verify
+ * @param crl_inline Contents of the crl file if it is inlined
* @param subject Subject of the given certificate
*
* @return \c SUCCESS if the CRL was not signed by the issuer of
the
* certificate or does not contain an entry for it.
* \c FAILURE ot
Re: [Openvpn-devel] [PATCH 04/10] Added flags parameter to format_hex_ex.
On Thu, Mar 3, 2016 at 9:19 AM, James Yonan wrote:
> We add the flags parameter without changing the signature of
> the function by repurposing the space_break parameter into
> space_break_flags where the lower 8 bits are used for the
> previous space_break parameter and the higher bits are used
> for flag values.
>
> Added new flag FHE_CAPS that formats the generated hex string
> in upper case.
>
> Signed-off-by: James Yonan
> ---
> src/openvpn/buffer.c | 11 +++
> src/openvpn/buffer.h | 4 +++-
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
> index bc67d65..52c6ab9 100644
> --- a/src/openvpn/buffer.c
> +++ b/src/openvpn/buffer.c
> @@ -435,18 +435,21 @@ gc_transfer (struct gc_arena *dest, struct gc_arena
> *src)
>
> char *
> format_hex_ex (const uint8_t *data, int size, int maxoutput,
> - int space_break, const char* separator,
> + unsigned int space_break_flags, const char* separator,
>struct gc_arena *gc)
> {
>struct buffer out = alloc_buf_gc (maxoutput ? maxoutput :
> - ((size * 2) + (size / space_break) *
> (int) strlen (separator) + 2),
> + ((size * 2) + (size / (space_break_flags
> & FHE_SPACE_BREAK_MASK)) * (int) strlen (separator) + 2),
> gc);
>int i;
>for (i = 0; i < size; ++i)
> {
> - if (separator && i && !(i % space_break))
> + if (separator && i && !(i % (space_break_flags &
> FHE_SPACE_BREAK_MASK)))
> buf_printf (&out, "%s", separator);
> - buf_printf (&out, "%02x", data[i]);
> + if (space_break_flags & FHE_CAPS)
> + buf_printf (&out, "%02X", data[i]);
> + else
> + buf_printf (&out, "%02x", data[i]);
> }
>buf_catrunc (&out, "[more...]");
>return (char *)out.data;
> diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
> index 24f52aa..8070439 100644
> --- a/src/openvpn/buffer.h
> +++ b/src/openvpn/buffer.h
> @@ -403,9 +403,11 @@ bool buf_parse (struct buffer *buf, const int delim,
> char *line, const int size)
> /*
> * Hex dump -- Output a binary buffer to a hex string and return it.
> */
> +#define FHE_SPACE_BREAK_MASK 0xFF /* space_break parameter in lower 8 bits */
> +#define FHE_CAPS 0x100/* output hex in caps */
> char *
> format_hex_ex (const uint8_t *data, int size, int maxoutput,
> - int space_break, const char* separator,
> + unsigned int space_break_flags, const char* separator,
>struct gc_arena *gc);
>
> static inline char *
ACK
-Steffan
[Openvpn-devel] [PATCH v3] Implement inlining of crl files
While crl files can change regulary and it is usually not a good idea to
statically include them into config files, handling multiple files and updating
files on mobile files is tiresome/problematic. Inlining a static version of the
crl file is better in these use cases than to use no crl at all.
OpenVPN 3 already supports inlining crl-verify, so is already used
in config files.
V2: Fixed PolarSSL and made formatting respect the 80 column limit
V3: Accidentally reverted one change too much in V2
---
doc/openvpn.8 | 3 ++-
src/openvpn/init.c| 1 +
src/openvpn/options.c | 11 ---
src/openvpn/options.h | 1 +
src/openvpn/ssl_common.h | 1 +
src/openvpn/ssl_verify.c | 2 +-
src/openvpn/ssl_verify_backend.h | 5 +++--
src/openvpn/ssl_verify_openssl.c | 8 ++--
src/openvpn/ssl_verify_polarssl.c | 20
9 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 628d877..decffc7 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -6490,7 +6490,8 @@ X509_1_C=KG
.\"*
.SH INLINE FILE SUPPORT
OpenVPN allows including files in the main configuration for the
-.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12, \-\-secret
+.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12, \-\-secret,
+.B \-\-crl-verify
and
.B \-\-tls\-auth
options.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index cb73a3d..33a1420 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2323,6 +2323,7 @@ do_init_crypto_tls (struct context *c, const unsigned int
flags)
to.verify_x509_type = (options->verify_x509_type & 0xff);
to.verify_x509_name = options->verify_x509_name;
to.crl_file = options->crl_file;
+ to.crl_file_inline = options->crl_file_inline;
to.ssl_flags = options->ssl_flags;
to.ns_cert_type = options->ns_cert_type;
memmove (to.remote_cert_ku, options->remote_cert_ku, sizeof
(to.remote_cert_ku));
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 02def3a..57f3dc5 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2747,8 +2747,8 @@ options_postprocess_filechecks (struct options *options)
errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE,
options->crl_file, R_OK|X_OK,
"--crl-verify directory");
else
-errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE,
options->crl_file, R_OK,
- "--crl-verify");
+errs |= check_file_access_chroot (options->chroot_dir,
CHKACC_FILE|CHKACC_INLINE,
+ options->crl_file, R_OK, "--crl-verify");
errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE,
options->tls_auth_file, R_OK,
"--tls-auth");
@@ -6783,12 +6783,17 @@ add_option (struct options *options,
VERIFY_PERMISSION (OPT_P_GENERAL);
options->cipher_list = p[1];
}
- else if (streq (p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2], "dir"))
|| !p[2]) && !p[3])
+ else if (streq (p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2], "dir"))
+ || (p[2] && streq (p[1], INLINE_FILE_TAG) ) || !p[2]) &&
!p[3])
{
VERIFY_PERMISSION (OPT_P_GENERAL);
if (p[2] && streq(p[2], "dir"))
options->ssl_flags |= SSLF_CRL_VERIFY_DIR;
options->crl_file = p[1];
+ if (streq (p[1], INLINE_FILE_TAG) && p[2])
+ {
+ options->crl_file_inline = p[2];
+ }
}
else if (streq (p[0], "tls-verify") && p[1])
{
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 23d3992..8a26e14 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -511,6 +511,7 @@ struct options
const char *ca_file_inline;
const char *cert_file_inline;
const char *extra_certs_file_inline;
+ const char *crl_file_inline;
char *priv_key_file_inline;
const char *dh_file_inline;
const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12
file */
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index eaf4a91..334ccb0 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -247,6 +247,7 @@ struct tls_options
int verify_x509_type;
const char *verify_x509_name;
const char *crl_file;
+ const char *crl_file_inline;
int ns_cert_type;
unsigned remote_cert_ku[MAX_PARMS];
const char *remote_cert_eku;
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index ccfa9d2..ea381f8 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -690,7 +690,7 @@ verify_cert(struct tls_session *session,
openvpn_x509_cert_t *cert, int cert_dep
}
else
{
- if (SUCCESS != x509_verify_crl(opt->crl_file, cert, subject))
+ if (SUCCESS != x509_verify_crl(opt->crl_file, opt->crl_file_inline,
cert,
Re: [Openvpn-devel] [PATCH 05/10] Extended x509-track for OpenSSL to report SHA1 fingerprint.
Hi, On Thu, Mar 3, 2016 at 9:19 AM, James Yonan wrote: > + char *sha1_fingerprint = format_hex_ex(x509->sha1_hash, > SHA_DIGEST_LENGTH, 0, 1 | FHE_CAPS, ":", &gc); This line could use some wrapping. Perhaps Gert can fix this when applying? Otherwise, ACK. -Steffan
Re: [Openvpn-devel] [PATCH v3] Implement inlining of crl files
On Sun, Mar 6, 2016 at 8:39 PM, Arne Schwabe wrote:
> While crl files can change regulary and it is usually not a good idea to
> statically include them into config files, handling multiple files and
> updating files on mobile files is tiresome/problematic. Inlining a static
> version of the crl file is better in these use cases than to use no crl at
> all.
>
> OpenVPN 3 already supports inlining crl-verify, so is already
> used in config files.
>
> V2: Fixed PolarSSL and made formatting respect the 80 column limit
> V3: Accidentally reverted one change too much in V2
> ---
> doc/openvpn.8 | 3 ++-
> src/openvpn/init.c| 1 +
> src/openvpn/options.c | 11 ---
> src/openvpn/options.h | 1 +
> src/openvpn/ssl_common.h | 1 +
> src/openvpn/ssl_verify.c | 2 +-
> src/openvpn/ssl_verify_backend.h | 5 +++--
> src/openvpn/ssl_verify_openssl.c | 8 ++--
> src/openvpn/ssl_verify_polarssl.c | 20
> 9 files changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/doc/openvpn.8 b/doc/openvpn.8
> index 628d877..decffc7 100644
> --- a/doc/openvpn.8
> +++ b/doc/openvpn.8
> @@ -6490,7 +6490,8 @@ X509_1_C=KG
> .\"*
> .SH INLINE FILE SUPPORT
> OpenVPN allows including files in the main configuration for the
> -.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12,
> \-\-secret
> +.B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12,
> \-\-secret,
> +.B \-\-crl-verify
> and
> .B \-\-tls\-auth
> options.
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index cb73a3d..33a1420 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -2323,6 +2323,7 @@ do_init_crypto_tls (struct context *c, const unsigned
> int flags)
>to.verify_x509_type = (options->verify_x509_type & 0xff);
>to.verify_x509_name = options->verify_x509_name;
>to.crl_file = options->crl_file;
> + to.crl_file_inline = options->crl_file_inline;
>to.ssl_flags = options->ssl_flags;
>to.ns_cert_type = options->ns_cert_type;
>memmove (to.remote_cert_ku, options->remote_cert_ku, sizeof
> (to.remote_cert_ku));
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 02def3a..57f3dc5 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -2747,8 +2747,8 @@ options_postprocess_filechecks (struct options *options)
> errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE,
> options->crl_file, R_OK|X_OK,
> "--crl-verify directory");
>else
> -errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE,
> options->crl_file, R_OK,
> - "--crl-verify");
> +errs |= check_file_access_chroot (options->chroot_dir,
> CHKACC_FILE|CHKACC_INLINE,
> + options->crl_file, R_OK,
> "--crl-verify");
>
>errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE,
> options->tls_auth_file, R_OK,
> "--tls-auth");
> @@ -6783,12 +6783,17 @@ add_option (struct options *options,
>VERIFY_PERMISSION (OPT_P_GENERAL);
>options->cipher_list = p[1];
> }
> - else if (streq (p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2],
> "dir")) || !p[2]) && !p[3])
> + else if (streq (p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2],
> "dir"))
> + || (p[2] && streq (p[1], INLINE_FILE_TAG) ) || !p[2]) &&
> !p[3])
> {
>VERIFY_PERMISSION (OPT_P_GENERAL);
>if (p[2] && streq(p[2], "dir"))
> options->ssl_flags |= SSLF_CRL_VERIFY_DIR;
>options->crl_file = p[1];
> + if (streq (p[1], INLINE_FILE_TAG) && p[2])
> + {
> + options->crl_file_inline = p[2];
> + }
> }
>else if (streq (p[0], "tls-verify") && p[1])
> {
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index 23d3992..8a26e14 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -511,6 +511,7 @@ struct options
>const char *ca_file_inline;
>const char *cert_file_inline;
>const char *extra_certs_file_inline;
> + const char *crl_file_inline;
>char *priv_key_file_inline;
>const char *dh_file_inline;
>const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12
> file */
> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
> index eaf4a91..334ccb0 100644
> --- a/src/openvpn/ssl_common.h
> +++ b/src/openvpn/ssl_common.h
> @@ -247,6 +247,7 @@ struct tls_options
>int verify_x509_type;
>const char *verify_x509_name;
>const char *crl_file;
> + const char *crl_file_inline;
>int ns_cert_type;
>unsigned remote_cert_ku[MAX_PARMS];
>const char *remote_cert_eku;
> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index ccfa9d2..ea381f8 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/open
[Openvpn-devel] operations that still require elevation on windows
Hi, There at least two more things on windows that the interactive service has to support: (i) register-dns (ii) ipv6 address and route setup Is there anything else? For (ii) can't it be done using the IP Helper API on vista+ -- is the use of net command required? If not, it should be easy to delegate it to the service. Thanks, Selva
Re: [Openvpn-devel] operations that still require elevation on windows
Hi,
On Sun, Mar 06, 2016 at 03:17:40PM -0500, Selva Nair wrote:
> There at least two more things on windows that the interactive service has
> to support:
>
> (i) register-dns
Indeed, this seems to be missing today.
> (ii) ipv6 address and route setup
>
> Is there anything else? For (ii) can't it be done using the IP Helper API
> on vista+ -- is the use of net command required? If not, it should be easy
> to delegate it to the service.
... but this should all be nicely done...?
tun.c, around line 1390:
if (tt->options.msg_channel)
{
do_address_service (true, AF_INET6, tt);
}
else
{
/* example: netsh interface ipv6 set address interface=42 2001:608:8
003::d store=active */
and route.c, add_route_ipv6()
...
#elif defined (WIN32)
if (tt->options.msg_channel)
status = add_route_ipv6_service (r6, tt);
else
...
Looking at interactive.c, "HandleAddressMessage()" uses
CreateUnicastIpAddressEntry(), which according to my reading of MSDN
supports IPv4 and IPv6. It's filled using "sockaddr_inet()", which
actually does IPv4 and IPv6 even if the name doesn't suggest so:
static SOCKADDR_INET
sockaddr_inet (short family, inet_address_t *addr)
{
SOCKADDR_INET sa_inet;
ZeroMemory (&sa_inet, sizeof (sa_inet));
sa_inet.si_family = family;
if (family == AF_INET)
sa_inet.Ipv4.sin_addr = addr->ipv4;
else if (family == AF_INET6)
sa_inet.Ipv6.sin6_addr = addr->ipv6;
return sa_inet;
}
So, we should be fine - and I'm fairly sure it actually works as one of
my test cases was "unprivileged user, iservice, VPN tunnel has IPv6".
Now, I have no clue whether this might be a deprecated API, and something
in the IP Helper API suite is "better" - just pointing out that we seem
to be actually fine regarding IPv6 address and route.
(HandleRouteMessage() uses CreateIpForwardEntry2(), so that's also
IPv4+IPv6)
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
Re: [Openvpn-devel] operations that still require elevation on windows
On Sun, Mar 6, 2016 at 4:48 PM, Gert Doering wrote: > On Sun, Mar 06, 2016 at 03:17:40PM -0500, Selva Nair wrote: > > There at least two more things on windows that the interactive service > has > > to support: > > > > (i) register-dns > > Indeed, this seems to be missing today. > > > (ii) ipv6 address and route setup > > > > Is there anything else? For (ii) can't it be done using the IP Helper > API > > on vista+ -- is the use of net command required? If not, it should be > easy > > to delegate it to the service. > > ... but this should all be nicely done...? Oh, I missed that. Based on the Trac ticket (#71) discussion on fatal error after sleep-resume, which was apparently caused by netsh failures treated as FATAL, and seeing some comments that ipv6 setup needs to use netsh command, I just assumed this is not done. Sorry for jumping into that conclusion. Then its only register-dns that remains... Selva
