[Openvpn-devel] async plugin-auth-pam
Hi,
I ran into a problem at a customer installation recently, where
plugin-auth-pam was blocking for some extended time (~30 seconds?)
due to pam_radius not receiving answers due to problems in the backend.
Now, maybe I should use radiusplugin in the first place, but since
the pam_radius setup on this machine is shared between sshd and OpenVPN,
I actually *like* using plugin-auth-pam -> pam_radius ("test one service,
know that radius very likely works for both").
That said, I'm considering modifying the plugin-auth-pam plugin to
add async authentication - which is supposedly not so hard
("sample-plugins/defer/simple.c").
Has one of you already done this, and just forgot to send in patches? :-)
Any particular caveats?
thanks,
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Simplify pool size handling, fix possible array overrun on pool reading.
Patch has been applied to the master branch. I have amended most whitespace as requested, and removed the "pool-> size = 0" initialization (not needed, indeed). I have left the brackets on the pool size calculation as they were - uncrustify says "that is also ok", and I find visibly grouping the different parts of the formula helps understanding what it does. commit e7c0cd996f35965172c5def9531b6ab9ca10c389 Author: Gert Doering Date: Tue Jun 9 10:02:29 2020 +0200 Simplify pool size handling, fix possible array overrun on pool reading. Signed-off-by: Gert Doering Acked-by: Antonio Quartulli Message-Id: <20200609080229.2564-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20006.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
[Openvpn-devel] [PATCH applied] Re: ipv6-pool: get rid of size constraint
Acked-by: Gert Doering Stared-at-code and tested on the t_server machine. And crashed. But that was not actually *this* patch's fault, but 6a8cd033b18's (fixed). This code does what it says on the lid - loosen up the "/112 is the minimum size" restriction on IPv6 pools, up to a /124 (which we can *now* handle correctly, as we do actual size calculations and checks). At the same time it introduces a minimum size sanity check - a pool that has less than 2 free addresses left (::/124) will error out. (Which is not working right now, because the IPv6 pool size calculation is not correct yet for small pools - but that will be fixed with the next commit) Your patch has been applied to the master branch. commit 1379e5271d0057fcaed82d6985e614ca2ed8c265 Author: Antonio Quartulli Date: Mon Jun 8 22:16:13 2020 +0200 ipv6-pool: get rid of size constraint Signed-off-by: Antonio Quartulli Acked-by: Gert Doering Message-Id: <20200608201613.23750-...@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20005.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
Re: [Openvpn-devel] [PATCH] Simplify pool size handling, fix possible array overrun on pool reading.
Hi,
On 09/06/2020 10:02, Gert Doering wrote:
> Remove separate ipv4.size and ipv6.size in the pool structure, return
> to a single pool_size, which is also the allocated array size.
>
> All calls to ifconfig_pool_size() change to "pool->size" now.
>
> pool->size is set to the size of the active pool, or if both IPv4 and IPv6
> are in use, to the smaller size (same underlying logic as in 452113155e7,
> but really put it into the size field).
>
> This fixes a SIGSEGV crash if an ifconfig-pool-persist file is loaded
> that has IPv6 and no IPv4 (= ipv6 handle is used) and that has more
> entries than the IPv4 pool size (comparison was done with ipv6.size,
> not with actual pool size), introduced by commit 6a8cd033b18.
>
> While at it, fix pool size calculation for IPv6 pools >= /112
> (too many -1), introduced by commit 452113155e7.
Thanks for taking care of these issues :-)
>
> Signed-off-by: Gert Doering
> ---
> src/openvpn/pool.c | 96 --
> src/openvpn/pool.h | 3 +-
> 2 files changed, 42 insertions(+), 57 deletions(-)
>
> diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
> index 29667623..f60cf291 100644
> --- a/src/openvpn/pool.c
> +++ b/src/openvpn/pool.c
> @@ -59,29 +59,6 @@ ifconfig_pool_entry_free(struct ifconfig_pool_entry *ipe,
> bool hard)
> }
> }
>
> -static const int
> -ifconfig_pool_size(const struct ifconfig_pool *pool)
> -{
> -int min = INT_MAX;
> -
> -if (!pool || (!pool->ipv4.enabled && !pool->ipv6.enabled))
> -{
> -return 0;
> -}
> -
> -if (pool->ipv4.enabled)
> -{
> -min = pool->ipv4.size;
> -}
> -
> -if (pool->ipv6.enabled && pool->ipv6.size < min)
> -{
> -min = pool->ipv6.size;
> -}
> -
> -return min;
> -}
> -
> static int
> ifconfig_pool_find(struct ifconfig_pool *pool, const char *common_name)
> {
> @@ -89,9 +66,8 @@ ifconfig_pool_find(struct ifconfig_pool *pool, const char
> *common_name)
> time_t earliest_release = 0;
> int previous_usage = -1;
> int new_usage = -1;
> -int pool_size = ifconfig_pool_size(pool);
>
> -for (i = 0; i < pool_size; ++i)
> +for (i = 0; i < pool->size; ++i)
> {
> struct ifconfig_pool_entry *ipe = >list[i];
> if (!ipe->in_use)
> @@ -179,12 +155,13 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
> type, in_addr_t start,
> {
> struct gc_arena gc = gc_new();
> struct ifconfig_pool *pool = NULL;
> -int pool_size = -1;
> +int pool_ipv4_size = -1, pool_ipv6_size = -1;
>
> ASSERT(start <= end && end - start < IFCONFIG_POOL_MAX);
> ALLOC_OBJ_CLEAR(pool, struct ifconfig_pool);
>
> pool->duplicate_cn = duplicate_cn;
> +pool->size = 0;
This is not needed. The object is already cleared out after allocation
by the ALLOC_OBJ_CLEAR() macro.
>
> pool->ipv4.enabled = ipv4_pool;
>
> @@ -195,26 +172,28 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
> type, in_addr_t start,
> {
> case IFCONFIG_POOL_30NET:
> pool->ipv4.base = start & ~3;
> -pool->ipv4.size = (((end | 3) + 1) - pool->ipv4.base) >> 2;
> +pool_ipv4_size = (((end | 3) + 1) - pool->ipv4.base) >> 2;
> break;
>
> case IFCONFIG_POOL_INDIV:
> pool->ipv4.base = start;
> -pool->ipv4.size = end - start + 1;
> +pool_ipv4_size = end - start + 1;
> break;
>
> default:
> ASSERT(0);
> }
>
> -if (pool->ipv4.size < 2)
> +if (pool_ipv4_size < 2)
> {
> msg(M_FATAL, "IPv4 pool size is too small (%d), must be at least
> 2",
> -pool->ipv4.size);
> +pool_ipv4_size);
> }
>
> msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv4: base=%s size=%d",
> -print_in_addr_t(pool->ipv4.base, 0, ), pool->ipv4.size);
> +print_in_addr_t(pool->ipv4.base, 0, ), pool_ipv4_size);
> +
> +pool->size = pool_ipv4_size;
> }
>
> /* IPv6 pools are always "INDIV" type */
> @@ -239,49 +218,57 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
> type, in_addr_t start,
> * the address.
> *
> * Example: if we have netbits=31, the first bit has to be
> zero'd,
> - * the following operation first computes off=0x3f and then
> uses
> - * mask to extract the wanted bits from base
> + * the following operation first computes mask=0x3f and then
> + * uses mask to extract the wanted bits from base
> */
> -uint32_t mask = (1 << (128 - ipv6_netbits - 1)) - 1;
> +uint32_t mask = (1 << (128 - ipv6_netbits) ) - 1;
please remove the space between the 2 parenthesis.
The computation now looks correct :-)
> base &= mask;
>
[Openvpn-devel] [PATCH] Simplify pool size handling, fix possible array overrun on pool reading.
Remove separate ipv4.size and ipv6.size in the pool structure, return
to a single pool_size, which is also the allocated array size.
All calls to ifconfig_pool_size() change to "pool->size" now.
pool->size is set to the size of the active pool, or if both IPv4 and IPv6
are in use, to the smaller size (same underlying logic as in 452113155e7,
but really put it into the size field).
This fixes a SIGSEGV crash if an ifconfig-pool-persist file is loaded
that has IPv6 and no IPv4 (= ipv6 handle is used) and that has more
entries than the IPv4 pool size (comparison was done with ipv6.size,
not with actual pool size), introduced by commit 6a8cd033b18.
While at it, fix pool size calculation for IPv6 pools >= /112
(too many -1), introduced by commit 452113155e7.
Signed-off-by: Gert Doering
---
src/openvpn/pool.c | 96 --
src/openvpn/pool.h | 3 +-
2 files changed, 42 insertions(+), 57 deletions(-)
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 29667623..f60cf291 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -59,29 +59,6 @@ ifconfig_pool_entry_free(struct ifconfig_pool_entry *ipe,
bool hard)
}
}
-static const int
-ifconfig_pool_size(const struct ifconfig_pool *pool)
-{
-int min = INT_MAX;
-
-if (!pool || (!pool->ipv4.enabled && !pool->ipv6.enabled))
-{
-return 0;
-}
-
-if (pool->ipv4.enabled)
-{
-min = pool->ipv4.size;
-}
-
-if (pool->ipv6.enabled && pool->ipv6.size < min)
-{
-min = pool->ipv6.size;
-}
-
-return min;
-}
-
static int
ifconfig_pool_find(struct ifconfig_pool *pool, const char *common_name)
{
@@ -89,9 +66,8 @@ ifconfig_pool_find(struct ifconfig_pool *pool, const char
*common_name)
time_t earliest_release = 0;
int previous_usage = -1;
int new_usage = -1;
-int pool_size = ifconfig_pool_size(pool);
-for (i = 0; i < pool_size; ++i)
+for (i = 0; i < pool->size; ++i)
{
struct ifconfig_pool_entry *ipe = >list[i];
if (!ipe->in_use)
@@ -179,12 +155,13 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
type, in_addr_t start,
{
struct gc_arena gc = gc_new();
struct ifconfig_pool *pool = NULL;
-int pool_size = -1;
+int pool_ipv4_size = -1, pool_ipv6_size = -1;
ASSERT(start <= end && end - start < IFCONFIG_POOL_MAX);
ALLOC_OBJ_CLEAR(pool, struct ifconfig_pool);
pool->duplicate_cn = duplicate_cn;
+pool->size = 0;
pool->ipv4.enabled = ipv4_pool;
@@ -195,26 +172,28 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
type, in_addr_t start,
{
case IFCONFIG_POOL_30NET:
pool->ipv4.base = start & ~3;
-pool->ipv4.size = (((end | 3) + 1) - pool->ipv4.base) >> 2;
+pool_ipv4_size = (((end | 3) + 1) - pool->ipv4.base) >> 2;
break;
case IFCONFIG_POOL_INDIV:
pool->ipv4.base = start;
-pool->ipv4.size = end - start + 1;
+pool_ipv4_size = end - start + 1;
break;
default:
ASSERT(0);
}
-if (pool->ipv4.size < 2)
+if (pool_ipv4_size < 2)
{
msg(M_FATAL, "IPv4 pool size is too small (%d), must be at least
2",
-pool->ipv4.size);
+pool_ipv4_size);
}
msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv4: base=%s size=%d",
-print_in_addr_t(pool->ipv4.base, 0, ), pool->ipv4.size);
+print_in_addr_t(pool->ipv4.base, 0, ), pool_ipv4_size);
+
+pool->size = pool_ipv4_size;
}
/* IPv6 pools are always "INDIV" type */
@@ -239,49 +218,57 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
type, in_addr_t start,
* the address.
*
* Example: if we have netbits=31, the first bit has to be zero'd,
- * the following operation first computes off=0x3f and then
uses
- * mask to extract the wanted bits from base
+ * the following operation first computes mask=0x3f and then
+ * uses mask to extract the wanted bits from base
*/
-uint32_t mask = (1 << (128 - ipv6_netbits - 1)) - 1;
+uint32_t mask = (1 << (128 - ipv6_netbits) ) - 1;
base &= mask;
}
pool->ipv6.base = ipv6_base;
-pool->ipv6.size = ipv6_netbits > 112
+pool_ipv6_size = ipv6_netbits >= 112
? (1 << (128 - ipv6_netbits)) - base
: IFCONFIG_POOL_MAX;
-if (pool->ipv6.size < 2)
+if (pool_ipv6_size < 2)
{
msg(M_FATAL, "IPv6 pool size is too small (%d), must be at least
2",
-pool->ipv6.size);
+pool_ipv6_size);
}
msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: base=%s size=%d
