Re: [Openvpn-devel] [PATCH applied] Re: make dist: Ship ovpn_dco_freebsd.h, too

2023-01-28 Thread Matthias Andree

Am 28.01.23 um 19:55 schrieb Gert Doering:

Acked-by: Gert Doering 

To see the actual failure, one needs to build a tarball ("make dist"),
and from that tarball, compile with "configure --enable-dco", on FreeBSD
- so a pure "make distcheck" did not see it.  Apologies for that oversight.

Your patch has been applied to the master and release/2.6 branch.


Hi Gert,

Danke.

We could set AM_DISTCHECK_CONFIGURE_FLAGS=--enable-dco (and possibly
other --enable-* or -with-*) in the relevant Makefile.am though if you
want a more complete feature set in "make distcheck".

Cheers,
Matthias



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 2/4] cyryptapi.c: log the selected certificate's name

2023-01-28 Thread selva . nair
From: Selva Nair 

- With various ways of specifying the selector-string to the
  "--cryptoapicert" option, its not immediately obvious
  which certificate gets selected from the store. Log it.

  The "name" logged is a friendly name (if present), or a
  representative element of the subject (usually the common-name).

Signed-off-by: Selva Nair 
---
 src/openvpn/cryptoapi.c  | 29 +
 src/openvpn/win32-util.c | 15 +++
 src/openvpn/win32-util.h |  3 +++
 3 files changed, 47 insertions(+)

diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 39eeec1b..e3c0bc99 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -939,12 +939,31 @@ xkey_cng_sign(void *handle, unsigned char *sig, size_t 
*siglen, const unsigned c
 
 #endif /* HAVE_XKEY_PROVIDER */
 
+static char *
+get_cert_name(const CERT_CONTEXT *cc, struct gc_arena *gc)
+{
+DWORD len = CertGetNameStringW(cc, CERT_NAME_FRIENDLY_DISPLAY_TYPE, 0, 
NULL, NULL, 0);
+char *name = NULL;
+if (len)
+{
+wchar_t *wname = gc_malloc(len*sizeof(wchar_t), false, gc);
+if (!wname
+|| CertGetNameStringW(cc, CERT_NAME_FRIENDLY_DISPLAY_TYPE, 0, 
NULL, wname, len) == 0)
+{
+return NULL;
+}
+name = utf16to8(wname, gc);
+}
+return name;
+}
+
 int
 SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
 {
 HCERTSTORE cs;
 X509 *cert = NULL;
 CAPI_DATA *cd = calloc(1, sizeof(*cd));
+struct gc_arena gc = gc_new();
 
 if (cd == NULL)
 {
@@ -979,6 +998,13 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const 
char *cert_prop)
 }
 }
 
+/* try to log the "name" of the selected certificate */
+char *cert_name = get_cert_name(cd->cert_context, );
+if (cert_name)
+{
+msg(D_LOW, "cryptapicert: using certificate with name <%s>", 
cert_name);
+}
+
 /* cert_context->pbCertEncoded is the cert X509 DER encoded. */
 cert = d2i_X509(NULL, (const unsigned char **) 
>cert_context->pbCertEncoded,
 cd->cert_context->cbCertEncoded);
@@ -1022,6 +1048,7 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const 
char *cert_prop)
 EVP_PKEY *privkey = xkey_load_generic_key(tls_libctx, cd, pkey,
   xkey_cng_sign, 
(XKEY_PRIVKEY_FREE_fn *) CAPI_DATA_free);
 SSL_CTX_use_PrivateKey(ssl_ctx, privkey);
+gc_free();
 return 1; /* do not free cd -- its kept by xkey provider */
 
 #else  /* ifdef HAVE_XKEY_PROVIDER */
@@ -1047,12 +1074,14 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, 
const char *cert_prop)
 goto err;
 }
 CAPI_DATA_free(cd); /* this will do a ref_count-- */
+gc_free(gc);
 return 1;
 
 #endif /* HAVE_XKEY_PROVIDER */
 
 err:
 CAPI_DATA_free(cd);
+gc_free();
 return 0;
 }
 #endif  /* _WIN32 */
diff --git a/src/openvpn/win32-util.c b/src/openvpn/win32-util.c
index 35f2a311..32f7a00b 100644
--- a/src/openvpn/win32-util.c
+++ b/src/openvpn/win32-util.c
@@ -48,6 +48,21 @@ wide_string(const char *utf8, struct gc_arena *gc)
 return ucs16;
 }
 
+char *
+utf16to8(const wchar_t *utf16, struct gc_arena *gc)
+{
+char *utf8 = NULL;
+int n = WideCharToMultiByte(CP_UTF8, 0, utf16, -1, NULL, 0, NULL, NULL);
+if (n > 0)
+{
+utf8 = gc_malloc(n, true, gc);
+if (utf8)
+{
+WideCharToMultiByte(CP_UTF8, 0, utf16, -1, utf8, n, NULL, NULL);
+}
+}
+return utf8;
+}
 
 /*
  * Return true if filename is safe to be used on Windows,
diff --git a/src/openvpn/win32-util.h b/src/openvpn/win32-util.h
index b24242c8..ac37979f 100644
--- a/src/openvpn/win32-util.h
+++ b/src/openvpn/win32-util.h
@@ -34,6 +34,9 @@
 /* Convert a string from UTF-8 to UCS-2 */
 WCHAR *wide_string(const char *utf8, struct gc_arena *gc);
 
+/* Convert a string from UTF-16 to UTF-8 */
+char *utf16to8(const wchar_t *utf16, struct gc_arena *gc);
+
 /* return true if filename is safe to be used on Windows */
 bool win_safe_filename(const char *fn);
 
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 1/4] Option --cryptoapicert: support issuer name as a selector

2023-01-28 Thread selva . nair
From: Selva Nair 

- Certificate selection string can now specify a partial
  issuer name string as "--cryptoapicert ISSUER:" where
   is matched as a substring of the issuer (CA) name in
  the certificate.

  Partial case-insensitive matching against the "issuer name" is
  used. Here "issuer name" is a text representation of the RDN's
  separated by commas.

  E.g., "CA, Ontario, Toronto, Acme Inc., IT, Acme Root CA".

  See MSDN docs on CertFindCertificateInStore() with CERT_FIND_ISSUER_STR
  as "FindType" for more details.

  As the order of RDN's is not well-defined[*] and type names like "OU"
  or "CN" are not included, its best to match against a single attribute
  like the CN of the issuer:

  E.g., --cryptoapicert "ISSUER:Acme Root"

[*] Windows appears to order RDN's in the reverse order to which
its written in the certificate but do not rely on this.

Signed-off-by: Selva Nair 
---
 doc/man-sections/windows-options.rst | 13 +++--
 src/openvpn/cryptoapi.c  |  5 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/doc/man-sections/windows-options.rst 
b/doc/man-sections/windows-options.rst
index 368f7b19..e87291f4 100644
--- a/doc/man-sections/windows-options.rst
+++ b/doc/man-sections/windows-options.rst
@@ -41,13 +41,22 @@ Windows-Specific Options
 
  cryptoapicert "SUBJ:Peter Runestig"
 
-  To select a certificate, based on certificate's thumbprint:
+  To select a certificate, based on certificate's thumbprint (SHA1 hash):
   ::
 
  cryptoapicert "THUMB:f6 49 24 41 01 b4 ..."
 
   The thumbprint hex string can easily be copy-and-pasted from the Windows
-  Certificate Store GUI.
+  Certificate Store GUI. The embedded spaces in the hex string are optional.
+
+  To select a certificate based on a substring in certificate's
+  issuer name:
+  ::
+
+ cryptoapicert "ISSUER:Sample CA"
+
+  The first non-expired certificate found in the user's store or the
+  machine store that matches the select-string is used.
 
 --dhcp-release
   Ask Windows to release the TAP adapter lease on shutdown. This option
diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 661a9a6d..39eeec1b 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -459,6 +459,11 @@ find_certificate_in_store(const char *cert_prop, 
HCERTSTORE cert_store)
 find_param = wide_string(cert_prop + 5, );
 find_type = CERT_FIND_SUBJECT_STR_W;
 }
+else if (!strncmp(cert_prop, "ISSUER:", 7))
+{
+find_param = wide_string(cert_prop + 7, );
+find_type = CERT_FIND_ISSUER_STR_W;
+}
 else if (!strncmp(cert_prop, "THUMB:", 6))
 {
 const char *p;
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 4/4] cryptoapi.c: simplify parsing of thumbprint hex string

2023-01-28 Thread selva . nair
From: Selva Nair 

Signed-off-by: Selva Nair 
---
 src/openvpn/cryptoapi.c | 44 +++--
 1 file changed, 12 insertions(+), 32 deletions(-)

diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 6ff4fcb5..9fd5aea9 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -210,49 +210,29 @@ find_certificate_in_store(const char *cert_prop, 
HCERTSTORE cert_store)
 }
 else if (!strncmp(cert_prop, "THUMB:", 6))
 {
-const char *p;
-int i, x = 0;
 find_type = CERT_FIND_HASH;
 find_param = 
 
-/* skip the tag */
-cert_prop += 6;
-for (p = cert_prop, i = 0; *p && i < sizeof(hash); i++)
+int i = 0;
+
+for (const char *p = cert_prop + 6; *p && i < sizeof(hash); p += 2)
 {
-if (*p >= '0' && *p <= '9')
-{
-x = (*p - '0') << 4;
-}
-else if (*p >= 'A' && *p <= 'F')
+/* skip spaces */
+while (*p == ' ')
 {
-x = (*p - 'A' + 10) << 4;
+p++;
 }
-else if (*p >= 'a' && *p <= 'f')
+if (!*p) /* ending with spaces is not an error */
 {
-x = (*p - 'a' + 10) << 4;
+break;
 }
-if (!*++p)  /* unexpected end of string */
+
+if (!isxdigit(p[0]) || !isxdigit(p[1])
+|| sscanf(p, "%2hhx", [i++]) != 1)
 {
-msg(M_WARN|M_INFO, "WARNING: cryptoapicert: error parsing 
.", cert_prop);
+msg(M_WARN|M_INFO, "WARNING: cryptoapicert: error parsing 
<%s>.", cert_prop);
 goto out;
 }
-if (*p >= '0' && *p <= '9')
-{
-x += *p - '0';
-}
-else if (*p >= 'A' && *p <= 'F')
-{
-x += *p - 'A' + 10;
-}
-else if (*p >= 'a' && *p <= 'f')
-{
-x += *p - 'a' + 10;
-}
-hash[i] = x;
-/* skip any space(s) between hex numbers */
-for (p++; *p && *p == ' '; p++)
-{
-}
 }
 blob.cbData = i;
 }
-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 0/4] Improvements for cryptoapi.c

2023-01-28 Thread selva . nair
From: Selva Nair 

1. Support selecting certificate using issuer name
   (goal: "planned obsolescence" of 2.6, already :)

2. Log the selected certificate's name
 
3. Remove Pre OpenSSL-3.01 support
   (goal: leaner and meaner)

4. Simplify parsing of thumbprint hex string

 doc/man-sections/windows-options.rst |  13 +-
 src/openvpn/cryptoapi.c  | 629 +++
 src/openvpn/options.c|   2 +-
 src/openvpn/win32-util.c |  15 +
 src/openvpn/win32-util.h |   3 +
 5 files changed, 84 insertions(+), 578 deletions(-)

-- 
2.34.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 3/4] cryptoapi.c: remove pre OpenSSL-3.01 support

2023-01-28 Thread selva . nair
From: Selva Nair 

- Require xkey-provider (thus OpenSSL 3.01+) for --cryptoapicert

Note:
  Ideally we should also make ENABLE_CRYPTOAPI conditional
  on HAVE_XKEY_PROVIDER but that looks hard unless we can agree
  to move HAVE_XKEY_PROVIDER to configure/config.h.
  Or move ENABLE_CRYPTOAPI out of syshead.h ?

Signed-off-by: Selva Nair 
---
 src/openvpn/cryptoapi.c | 555 +---
 src/openvpn/options.c   |   2 +-
 2 files changed, 11 insertions(+), 546 deletions(-)

diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index e3c0bc99..6ff4fcb5 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -55,17 +55,17 @@
 #include "xkey_common.h"
 
 #ifndef HAVE_XKEY_PROVIDER
-/* index for storing external data in EC_KEY: < 0 means uninitialized */
-static int ec_data_idx = -1;
 
-/* Global EVP_PKEY_METHOD used to override the sign operation */
-static EVP_PKEY_METHOD *pmethod;
-static int (*default_pkey_sign_init) (EVP_PKEY_CTX *ctx);
-static int (*default_pkey_sign) (EVP_PKEY_CTX *ctx, unsigned char *sig,
- size_t *siglen, const unsigned char *tbs, 
size_t tbslen);
-#else  /* ifndef HAVE_XKEY_PROVIDER */
+int
+SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
+{
+msg(M_NONFATAL, "ERROR: cryptoapicert not supported in this version");
+return 0;
+}
+
+#else /* HAVE_XKEY_PROVIDER */
+
 static XKEY_EXTERNAL_SIGN_fn xkey_cng_sign;
-#endif /* HAVE_XKEY_PROVIDER */
 
 typedef struct _CAPI_DATA {
 const CERT_CONTEXT *cert_context;
@@ -146,127 +146,6 @@ CAPI_DATA_free(CAPI_DATA *cd)
 free(cd);
 }
 
-#ifndef HAVE_XKEY_PROVIDER
-
-/* Translate OpenSSL padding type to CNG padding type
- * Returns 0 for unknown/unsupported padding.
- */
-static DWORD
-cng_padding_type(int padding)
-{
-DWORD pad = 0;
-
-switch (padding)
-{
-case RSA_NO_PADDING:
-break;
-
-case RSA_PKCS1_PADDING:
-pad = BCRYPT_PAD_PKCS1;
-break;
-
-case RSA_PKCS1_PSS_PADDING:
-pad = BCRYPT_PAD_PSS;
-break;
-
-default:
-msg(M_WARN|M_INFO, "cryptoapicert: unknown OpenSSL padding type 
%d.",
-padding);
-}
-
-return pad;
-}
-
-/**
- * Sign the hash in 'from' using NCryptSignHash(). This requires an NCRYPT
- * key handle in cd->crypt_prov. On return the signature is in 'to'. Returns
- * the length of the signature or 0 on error.
- * This is used only for RSA and padding should be BCRYPT_PAD_PKCS1 or
- * BCRYPT_PAD_PSS.
- * If the hash_algo is not NULL, PKCS #1 DigestInfo header gets added
- * to |from|, else it is signed as is. Use NULL for MD5 + SHA1 hash used
- * in TLS 1.1 and earlier.
- * In case of PSS padding, |saltlen| should specify the size of salt to use.
- * If |to| is NULL returns the required buffer size.
- */
-static int
-priv_enc_CNG(const CAPI_DATA *cd, const wchar_t *hash_algo, const unsigned 
char *from,
- int flen, unsigned char *to, int tlen, DWORD padding, DWORD 
saltlen)
-{
-NCRYPT_KEY_HANDLE hkey = cd->crypt_prov;
-DWORD len = 0;
-ASSERT(cd->key_spec == CERT_NCRYPT_KEY_SPEC);
-
-DWORD status;
-
-msg(D_LOW, "Signing hash using CNG: data size = %d padding = %lu", flen, 
padding);
-
-if (padding == BCRYPT_PAD_PKCS1)
-{
-BCRYPT_PKCS1_PADDING_INFO padinfo = {hash_algo};
-status = NCryptSignHash(hkey, , (BYTE *)from, flen,
-to, tlen, , padding);
-}
-else if (padding == BCRYPT_PAD_PSS)
-{
-BCRYPT_PSS_PADDING_INFO padinfo = {hash_algo, saltlen};
-status = NCryptSignHash(hkey, , (BYTE *)from, flen,
-to, tlen, , padding);
-}
-else
-{
-msg(M_NONFATAL, "Error in cryptoapicert: Unknown padding type");
-return 0;
-}
-
-if (status != ERROR_SUCCESS)
-{
-SetLastError(status);
-msg(M_NONFATAL|M_ERRNO, "Error in cryptoapicert: NCryptSignHash 
failed");
-len = 0;
-}
-
-/* Unlike CAPI, CNG signature is in big endian order. No reversing needed. 
*/
-return len;
-}
-
-/* called at RSA_free */
-static int
-rsa_finish(RSA *rsa)
-{
-const RSA_METHOD *rsa_meth = RSA_get_method(rsa);
-CAPI_DATA *cd = (CAPI_DATA *) RSA_meth_get0_app_data(rsa_meth);
-
-if (cd == NULL)
-{
-return 0;
-}
-CAPI_DATA_free(cd);
-RSA_meth_free((RSA_METHOD *) rsa_meth);
-return 1;
-}
-
-static EC_KEY_METHOD *ec_method = NULL;
-
-/** EC_KEY_METHOD callback: called when the key is freed */
-static void
-ec_finish(EC_KEY *ec)
-{
-EC_KEY_METHOD_free(ec_method);
-ec_method = NULL;
-CAPI_DATA *cd = EC_KEY_get_ex_data(ec, ec_data_idx);
-CAPI_DATA_free(cd);
-EC_KEY_set_ex_data(ec, ec_data_idx, NULL);
-}
-
-/** EC_KEY_METHOD callback sign_setup(): we do nothing here */
-static int
-ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp)
-{

[Openvpn-devel] [PATCH 2/2] signal_reset(): combine check and reset operations

2023-01-28 Thread selva . nair
From: Selva Nair 

- "if (sig == X) signal_reset(sig)" now becomes
  "signal_reset(sig, X)" so that the check and assignment
  can be done in one place where signals are masked.
  This is required to avoid change of signal state between
  check and reset operations.

- Avoid resetting the signal except when absolutely necessary
  (resetting has the potential of losing signals)

- In 'pre_init_signal_catch()', when certain low priority signals
  are set to SIG_IGN, clear any pending signals of the same
  type. Also, reset signal at the end of the SIGUSR1 and
  SIGHUP loops where their values are checked instead of later. This
  avoids the need for 'signal_reset()' after SIGHUP or in 'init_instance()'
  which could cause a signal like SIGTERM to be lost.

Signed-off-by: Selva Nair 
---
 src/openvpn/init.c|  3 ---
 src/openvpn/multi.c   |  5 ++---
 src/openvpn/openvpn.c |  5 ++---
 src/openvpn/sig.c | 40 +---
 src/openvpn/sig.h |  7 ++-
 src/openvpn/socket.c  |  5 ++---
 src/openvpn/win32.c   |  2 +-
 7 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b500d354..76a7be7b 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -4299,9 +4299,6 @@ init_instance(struct context *c, const struct env_set 
*env, const unsigned int f
 do_inherit_env(c, env);
 }
 
-/* signals caught here will abort */
-signal_reset(c->sig);
-
 if (c->mode == CM_P2P)
 {
 init_management_callback_p2p(c);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f2559016..c52c8f14 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3868,7 +3868,7 @@ multi_push_restart_schedule_exit(struct multi_context *m, 
bool next_server)
>deferred_shutdown_signal.wakeup,

compute_wakeup_sigma(>deferred_shutdown_signal.wakeup));
 
-signal_reset(m->top.sig);
+signal_reset(m->top.sig, 0);
 }
 
 /*
@@ -3878,12 +3878,11 @@ multi_push_restart_schedule_exit(struct multi_context 
*m, bool next_server)
 bool
 multi_process_signal(struct multi_context *m)
 {
-if (m->top.sig->signal_received == SIGUSR2)
+if (signal_reset(m->top.sig, SIGUSR2) == SIGUSR2)
 {
 struct status_output *so = status_open(NULL, 0, M_INFO, NULL, 0);
 multi_print_status(m, so, m->status_file_version);
 status_close(so);
-signal_reset(m->top.sig);
 return false;
 }
 else if (proto_is_dgram(m->top.options.ce.proto)
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index cba58276..ad0aa8a2 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -194,7 +194,6 @@ openvpn_main(int argc, char *argv[])
 context_clear_all_except_first_time();
 
 /* static signal info object */
-CLEAR(siginfo_static);
 c.sig = _static;
 
 /* initialize garbage collector scoped to context object */
@@ -333,14 +332,14 @@ openvpn_main(int argc, char *argv[])
 /* pass restart status to management subsystem */
 signal_restart_status(c.sig);
 }
-while (c.sig->signal_received == SIGUSR1);
+while (signal_reset(c.sig, SIGUSR1) == SIGUSR1);
 
 env_set_destroy(c.es);
 uninit_options();
 gc_reset();
 uninit_early();
 }
-while (c.sig->signal_received == SIGHUP);
+while (signal_reset(c.sig, SIGHUP) == SIGHUP);
 }
 
 context_gc_free();
diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index 559ca35d..4eead996 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -259,15 +259,37 @@ register_signal(struct signal_info *si, int signum, const 
char *signal_text)
 }
 }
 
-void
-signal_reset(struct signal_info *si)
+/**
+ * Clear the signal if its current value equals signum. If
+ * signum is zero the signal is cleared independent of its current
+ * value. Returns the current value of the signal.
+ */
+int
+signal_reset(struct signal_info *si, int signum)
 {
+int sig_saved = 0;
 if (si)
 {
-si->signal_received = 0;
-si->signal_text = NULL;
-si->source = SIG_SOURCE_SOFT;
+if (si == _static) /* attempting to alter the global signal */
+{
+block_async_signals();
+}
+
+sig_saved = si->signal_received;
+if (!signum || sig_saved == signum)
+{
+si->signal_received = 0;
+si->signal_text = NULL;
+si->source = SIG_SOURCE_SOFT;
+msg(D_SIGNAL_DEBUG, "signal_reset: signal %s is cleared", 
signal_name(signum, true));
+}
+
+if (si == _static)
+{
+unblock_async_signals();
+}
 }
+return sig_saved;
 }
 
 void
@@ -397,6 +419,10 @@ pre_init_signal_catch(void)
 sigaction(SIGUSR2, , NULL);
 sigaction(SIGPIPE, , NULL);
 #endif /* _WIN32 */
+/* clear 

[Openvpn-devel] [PATCH 1/2] Improve signal handling using POSIX sigaction

2023-01-28 Thread selva . nair
From: Selva Nair 

Currently we use the old signal API which follows system-V or
BSD semantics depending on the platform and/or feature-set macros.
Further, signal has many weaknesses which makes proper masking
(deferring) of signals during update not possible.

Improve this:

- Use sigaction to properly mask signals when modifying.

Notes:

Updating signal_reset() is handled in a follow up patch

SIG_SOURCE_CONNECTION_FAILED is retained in a hackish way. This value
has the same meaning as SIG_SOURCE_SOFT everywhere except where the
signal is printed. Looks cosmetic --- could be eliminated?

In pre_init_signal_catch() we ignore some unix signals, but the same signals
from management are not ignored though both are treated as "HARD" signals.
For example, during auth-user-pass query, "kill -SIGUSR1 " will be ignored,
but "signal SIGUSR1" from management interface will cause M_FATAL and exit.
This is the current behaviour, but could be improved?

This patch was originally submitted as 5/5 of the signals series. Now this is
1/2 of a new series with signal_reset changes moved to 2/2

Signed-off-by: Selva Nair 
---
 src/openvpn/errlevel.h |   1 +
 src/openvpn/sig.c  | 264 +++--
 src/openvpn/socket.c   |   1 -
 3 files changed, 202 insertions(+), 64 deletions(-)

diff --git a/src/openvpn/errlevel.h b/src/openvpn/errlevel.h
index c69ea91d..dedc0790 100644
--- a/src/openvpn/errlevel.h
+++ b/src/openvpn/errlevel.h
@@ -115,6 +115,7 @@
 #define D_CLIENT_NAT LOGLEV(6, 69, M_DEBUG)  /* show client NAT debug 
info */
 #define D_XKEY   LOGLEV(6, 69, M_DEBUG)  /* show xkey-provider 
debug info */
 #define D_DCO_DEBUG  LOGLEV(6, 69, M_DEBUG)  /* show DCO related 
lowlevel debug messages */
+#define D_SIGNAL_DEBUG   LOGLEV(6, 69, M_DEBUG)  /* show signal related 
debug messages */
 
 #define D_SHOW_KEYS  LOGLEV(7, 70, M_DEBUG)  /* show data channel 
encryption keys */
 #define D_SHOW_KEY_SOURCELOGLEV(7, 70, M_DEBUG)  /* show data channel key 
source entropy */
diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index 0d534601..559ca35d 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -6,6 +6,7 @@
  * packet compression.
  *
  *  Copyright (C) 2002-2023 OpenVPN Inc 
+ *  Copyright (C) 2016-2023 Selva Nair 
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2
@@ -60,6 +61,9 @@ static const struct signame signames[] = {
 { SIGUSR2, 1, "SIGUSR2", "sigusr2" }
 };
 
+/* mask for hard signals from management or windows */
+static unsigned long long ignored_hard_signals_mask;
+
 int
 parse_signal(const char *signame)
 {
@@ -114,24 +118,144 @@ signal_description(const int signum, const char *sigtext)
 }
 }
 
+/**
+ * Block (i.e., defer) all unix signals.
+ * Used while directly modifying the volatile elements of
+ * siginfo_static.
+ */
+static inline void
+block_async_signals(void)
+{
+#ifndef _WIN32
+sigset_t all;
+sigfillset(); /* all signals */
+sigprocmask(SIG_BLOCK, , NULL);
+#endif
+}
+
+/**
+ * Unblock all unix signals.
+ */
+static inline void
+unblock_async_signals(void)
+{
+#ifndef _WIN32
+sigset_t none;
+sigemptyset();
+sigprocmask(SIG_SETMASK, , NULL);
+#endif
+}
+
+/**
+ * Private function for registering a signal in the specified
+ * signal_info struct. This could be the global siginfo_static
+ * or a context specific signinfo struct.
+ *
+ * A signal is allowed to override an already registered
+ * one only if it has a higher priority.
+ * Returns true if the signal is set, false otherwise.
+ *
+ * Do not call any "AS-unsafe" functions such as printf from here
+ * as this may be called from signal_handler().
+ */
+static bool
+try_throw_signal(struct signal_info *si, int signum, int source)
+{
+bool ret = false;
+if (signal_priority(signum) >= signal_priority(si->signal_received))
+{
+si->signal_received = signum;
+si->source = source;
+ret = true;
+}
+return ret;
+}
+
+/**
+ * Throw a hard signal. Called from management and when windows
+ * signals are received through ctrl-c, exit event etc.
+ */
 void
 throw_signal(const int signum)
 {
-if (signal_priority(signum) >= 
signal_priority(siginfo_static.signal_received))
+if (ignored_hard_signals_mask & (1LL << signum))
+{
+msg(D_SIGNAL_DEBUG, "Signal %s is currently ignored", 
signal_name(signum, true));
+return;
+}
+block_async_signals();
+
+if (!try_throw_signal(_static, signum, SIG_SOURCE_HARD))
 {
-siginfo_static.signal_received = signum;
-siginfo_static.source = SIG_SOURCE_HARD;
+msg(D_SIGNAL_DEBUG, "Ignoring %s when %s has been received", 
signal_name(signum, true),
+signal_name(siginfo_static.signal_received, true));
 }
+else
+{
+msg(D_SIGNAL_DEBUG, "Throw signal (hard): %s ", 

[Openvpn-devel] [PATCH applied] Re: make dist: Ship ovpn_dco_freebsd.h, too

2023-01-28 Thread Gert Doering
Acked-by: Gert Doering 

To see the actual failure, one needs to build a tarball ("make dist"),
and from that tarball, compile with "configure --enable-dco", on FreeBSD
- so a pure "make distcheck" did not see it.  Apologies for that oversight.

Your patch has been applied to the master and release/2.6 branch.

commit ffcf20ca7070027ccb16c3697b2a0e263cbc78a4 (master)
commit 680ba43355f6d9e4dcdf6c6eb9ace09946dba8f3 (HEAD -> release/2.6)
Author: Matthias Andree
Date:   Fri Jan 27 21:32:08 2023 +0100

 make dist: Ship ovpn_dco_freebsd.h, too

 Acked-by: Gert Doering 
 Message-Id: <20230127203208.305638-1-matthias.and...@gmx.de>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26085.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel