Re: [Openvpn-devel] [PATCH] Change CTR DRBG update function call to new mbedtls 2.16.0 API
> Am 02.04.21 um 15:26 schrieb Max Fillinger:
> > From: Uipko Berghuis
> >
> > In mbedtls 2.16.0 mbedtls_ctr_drbg_update() changed to
> > mbedtls_ctr_drbg_update_ret(). Change the function name and handle the
> > new return value error code.
> > ---
> > src/openvpn/ssl_mbedtls.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> > index 5d7af351..56e9f045 100644
> > --- a/src/openvpn/ssl_mbedtls.c
> > +++ b/src/openvpn/ssl_mbedtls.c
> > @@ -950,7 +950,10 @@ tls_ctx_personalise_random(struct tls_root_ctx
> > *ctx)
> >
> > if (0 != memcmp(old_sha256_hash, sha256_hash,
> sizeof(sha256_hash)))
> > {
> > -mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32);
> > +if (!mbed_ok(mbedtls_ctr_drbg_update_ret(cd_ctx,
> sha256_hash, 32)))
> > +{
> > +msg(M_WARN, "WARNING: failed to personalise random,
> could not update CTR_DRBG");
> > +}
> > memcpy(old_sha256_hash, sha256_hash,
> sizeof(old_sha256_hash));
> > }
> > }
> >
>
> This change will break compilation with anything that is < 2.16.0.
This function is deprecated in 2.16. I don't mind keeping this change to
OpenVPN-NL for now, but for future reference, what's the best solution
when a new version of mbedtls removes the function?
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/1] Let mbedtls_ssl_configs find reloaded CRLs
> > }
> >
> > void
> > +make_empty_crl(struct tls_root_ctx *ctx)
> > +{
> > +if (ctx->crl == NULL)
> > +{
> > +ALLOC_OBJ_CLEAR(ctx->crl, mbedtls_x509_crl);
> > +}
> > +else
> > +{
> > +mbedtls_x509_crl_free(ctx->crl);
> > +}
> > +}
> > +
>
> This function is confsung me. This needs at least more documentation
> what it is doing as docstring etc. I would also expect to have
> mbedtls_x509_crl_init to init the struct and not just a malloc that set
> the whole structure to zero. And mbedtls_x509_crl_free does from its
> description doesn't guarantee that the object is left in a proper state.
All mbedtls_x509_crl_init() does is set the struct to 0, but I agree it's
better from a readability standpoint to call it anyway. Another function in
the file also uses ALLOC_OBJ_CLEAR instead of the init function; I'll fix
that too.
When OpenVPN fails to parse a CRL file, it also calls mbedtls_x509_crl_free()
and leaves it at that (see backend_tls_ctx_reload_crl()). This leaves it in
a state where all connection attempts are rejected. The same happens when you
initialize a CRL struct and then don't do anything further with it. So that's
what I did in this function. You're right, this needs more comments. If we want
to do it that way at all.
As far as I'm aware, none of that is documented. Actually, if I understand
correctly,
you're not even supposed to touch the mbedtls_ssl_config struct (which contains
a pointer to the CRL) after calling mbedtls_ssl_setup(). So far, reloading the
CRL
has worked anyway. It looks like the code in ssl_mbedtls.c needs some rewriting
if
we want to do it properly. I'll need to think about that some more.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/1] Rework mbedtls CRL handling
Hi Antonio, Thanks for taking a look! > Mh... I have tried to reproduce this issue, but I was not able to. > Would you be able to provide me with the exact steps to hit this > situation? This bug happens only if the CRL file can't be opened in init_ssl(). (Un-openable CRL files and un-parseable CRL files cause different behaviors.) Realistically, this only happens when using chroot: 1) Have a valid CRL in /path/to/chroot/crl.pem 2) Run with --chroot /path/to/chroot and --crl-verify /crl.pem. (Note the absolute path for crl-verify!) 3) OpenVPN will ignore the CRL. (In principle, this could happen without chroot if the CRL file gets deleted right after OpenVPN detected that it exists in options.c.) I've written it up in more detail in an earlier e-mail: https://sourceforge.net/p/openvpn/mailman/message/37254045/ > To fix this issue, wouldn't be enough to reload the CRL *before* calling > mbedtls_ssl_setup()? > This way we know that a new ssl_ctx is always initialized with the > latest CRL, rather than having the old one still around? > > I have attached a proposal for your review. Please let me know what you > think about it. I tried your diff with the chroot setup above. The first connection attempt causes a restart (whether the client cert was revoked or not). 2021-04-08 09:54:05 VERIFY ERROR: CRL not loaded 2021-04-08 09:54:05 VERIFY ERROR: CRL not loaded 2021-04-08 09:54:05 TLS_ERROR: read tls_read_plaintext error: X509 - Certificate verification failed, e.g. CRL, CA or signature check failed 2021-04-08 09:54:05 TLS Error: TLS object -> incoming plaintext read error 2021-04-08 09:54:05 TLS Error: TLS handshake failed 2021-04-08 09:54:05 Closing TUN/TAP interface 2021-04-08 09:54:05 SIGUSR1[soft,tls-error] received, process restarting After the restart, it works correctly: Revoked certificates are rejected, and non-revoked ones are accepted. Also, the reason why I want to keep old CRLs around is not directly related to this bug. It's because every key_state_ssl struct contains a mbedtls_ssl_context and a corresponding mbedtls_ssl_config. That config is not supposed to be changed, according to the mbedtls documentation. However, every config struct in OpenVPN contains a pointer to tls_root_ctx->crl. (Well, at least as long as you're not running in a chroot.) This crl is modified in-place whenever we run tls_ctx_reload_crl(). It does not seem like a big deal in practice. Because OpenVPN is single-threaded we're not at risk of overwriting the CRL while it is used, but I thought while I'm already working on the code, I could try to make it conform to the documentation better... > As a side note: how did you notice this issue? Compumatica has found this bug in OpenVPN-NL and they reported it to us at Fox-IT. I've reproduced it with stock OpenVPN using mbedtls. It does not happen with OpenSSL. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Replace deprecated mbedtls DRBG update function
> From: Arne Schwabe [mailto:a...@rfc2549.org] > Sent: dinsdag 10 augustus 2021 12:12 > To: Maximilian Fillinger ; openvpn- > de...@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH] Replace deprecated mbedtls DRBG > update function > > Am 10.08.21 um 08:16 schrieb Max Fillinger: > > +#if MBEDTLS_VERSION_NUMBER < 0x0210 > > Is that really 2.16? Looking at the API doc > (https://tls.mbed.org/api/version_8h.html#adb4f54ebb33fd1a25e2c4d4480cf4 > 936) > it sounds like there should be a 16 in that number. It's a hexadecimal 16. Version 2.26.0 for example does #define MBEDTLS_VERSION_NUMBER 0x021A ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Replace deprecated mbedtls DRBG update function
> Normally we have patch v2 here and also a patch v2 in the subject (use > -v 2 when doing git format-patch) but for this small patch it is not a > problem. I'll keep it in mind for next time! > Apart from the fact that we might want to abort (M_FATAL) if this fails > instead basically ignoring the error and just log it, the change is > fine. Considering the return status was ignored before, this patch is > otherwise good. But failing also does not have any really bad impact... I was thinking about making it fatal, but there's another place where personalizing random can fail that also only gives a warning. So that's what I went with. (I wouldn't be opposed to making them both fatal, though.) Anyway, thanks for the ACK! ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Don't "undo" ifconfig when given --ifconfig-noexec
> ... so why is "did_ifconfig_setup" true, if ifconfig wasn't done? > > Or, phrased differently, what is did_ifconfig_setup used for, across the > code, and can we just "not set it to true" if ifconfig-noexec is in > effect? Or does it have nasty side effects? tt->did_ifconfig_setup is set in init_tun, which sets up the tuntap struct but doesn't do ifconfig yet. This is executed independently of --ifconfig-noexec. There are probably some things to refactor there. Maybe we could remove did_ifconfig_setup and instead use did_ifconfig on all platforms. But to be honest, I'm scared of touching it. Also, could we backport it to 2.5? If so, isn't it better to keep the fix small? ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: Update openssl_compat.h for newer LibreSSL
> but they think the revamped OpenSSL 3.0 way of calculating the TLS1 PRF > might actually not be in 2.5 yet, so they do not need a patch for that. In 2.5, openssl_compat.h also doesn't try to define X509_OBJECT_free(), so there's nothing to backport there. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 1/2] Update openssl_compat.h for newer LibreSSL
> -Original Message- > From: Arne Schwabe [mailto:a...@rfc2549.org] > Sent: donderdag 18 augustus 2022 22:16 > To: Maximilian Fillinger ; openvpn- > de...@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH v2 1/2] Update openssl_compat.h for > newer LibreSSL > > Am 11.08.22 um 19:11 schrieb Max Fillinger: > > LibreSSL has added some of the functions that are defined here. > However, > > we still need RSA_F_RSA_OSSL_PRIVATE_ENCRYPT. > > > > v2: Change ifdef condition for RSA_F_RSA_OSSL_PRIVATE_ENCRYPT. > > > > Signed-off-by: Max Fillinger > > --- > > src/openvpn/openssl_compat.h | 8 +--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/src/openvpn/openssl_compat.h > b/src/openvpn/openssl_compat.h > > index b3ee94f1..c78d2229 100644 > > --- a/src/openvpn/openssl_compat.h > > +++ b/src/openvpn/openssl_compat.h > > @@ -51,8 +51,8 @@ > > #define SSL_CTX_set1_groups SSL_CTX_set1_curves > > #endif > > > > -/* Functionality missing in LibreSSL and OpenSSL 1.0.2 */ > > -#if (OPENSSL_VERSION_NUMBER < 0x1010L || > defined(LIBRESSL_VERSION_NUMBER)) && !defined(ENABLE_CRYPTO_WOLFSSL) > > +/* Functionality missing in LibreSSL before 3.5 and OpenSSL 1.0.2 */ > > +#if (OPENSSL_VERSION_NUMBER < 0x1010L || > (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < > 0x305fL)) && !defined(ENABLE_CRYPTO_WOLFSSL) > > /** > >* Destroy a X509 object > >* > > @@ -68,11 +68,13 @@ X509_OBJECT_free(X509_OBJECT *obj) > > } > > } > > > > -#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT > RSA_F_RSA_EAY_PRIVATE_ENCRYPT > > #define EVP_CTRL_AEAD_SET_TAGEVP_CTRL_GCM_SET_TAG > > #define EVP_CTRL_AEAD_GET_TAGEVP_CTRL_GCM_GET_TAG > > #endif > > > > +#if OPENSSL_VERSION_NUMBER < 0x1010L || > defined(LIBRESSL_VERSION_NUMBER) > > +#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT > RSA_F_RSA_EAY_PRIVATE_ENCRYPT > > +#endif > > > > The patch basically removes the !defined(ENABLE_CRYPTO_WOLFSSL) from > this part of the ifdef and that breaks wolfSSL. While I don't think we > should much effort into wolfSSL, we should also not break it on purpose. > > Arne You told me to ignore it in IRC. But sure, I can make a v3. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: Update openssl_compat.h for newer LibreSSL
> I'm a bit unsure if we need this for 2.5 - it's "long term compat" > and not very intrusive, but on the other hand, not too many people > seem to care about LibreSSL. OpenBSD has packaged 2.5.7 for snapshots, so they must already have a workaround. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 2/2] Handle EVP_MD_CTX as an opaque struct
> -Original Message- > From: Arne Schwabe [mailto:a...@rfc2549.org] > Sent: donderdag 11 augustus 2022 14:21 > To: Maximilian Fillinger ; openvpn- > de...@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH 2/2] Handle EVP_MD_CTX as an opaque > struct > > Am 11.08.22 um 14:07 schrieb Max Fillinger: > > Building OpenVPN on the latest OpenBSD snapshot failed because > EVP_MD_CTX > > is an opaque struct in LibreSSL now. Therefore, call md_ctx_new() > instead > > of declaring them on the stack. When they're not on the stack anymore, > we > > don't have to call EVP_MD_CTX_init() anymore, but we need to call > > EVP_MD_CTX_free() instead of cleanup. > > Urgh. The whole reason I left this code with the EVP_MD_CTX is that it > is OpenSSL 1.0.2 only and I expected to be able to remove it sooner or > later. So LibreSSL doeds not support the alternative API for that? > > EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_TLS1_PRF, NULL); > > is what we use for OpenSSL 1.1.0+ > > I am not happy to soon have LibreSSL specific code in our code but it > seems like if want to continue that library, we have to. > > The change looks good itself. > > Acked-By: Arne Schwabe LibreSSL now has EVP_PKEY_CTX_new_id(), but it does not define EVP_PKEY_TLS1_PRF. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 2/2] Fix message for too long tls-crypt-v2 metadata
Hi! > -Original Message- > From: Gert Doering [mailto:g...@greenie.muc.de] > Sent: maandag 12 december 2022 13:03 > To: Maximilian Fillinger > Cc: openvpn-devel@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH 2/2] Fix message for too long tls- > crypt-v2 metadata > > Hi, > > On Sat, Nov 26, 2022 at 05:26:48PM +0100, Max Fillinger wrote: > > The current code only checks if the base64-encoded metadata is at most > > 980 characters. However, that can encode up to 735 bytes of data, > while > > only up to 733 bytes are allowed. When passing 734 or 735 bytes, > openvpn > > prints a misleading error message saying that the base64 cannot be > > decoded. > > > > This patch checks the decoded length to show an accurate error > message. > > This patch looks overly complex to me for "change 735 to 733" - could > you (or Arne) explain the change a bit better? What's going on is that the tls-crypt-v2 metadata can be at most 733 bytes. But the metadata is supplied in base64. If you want to encode 733 bytes in base64, you need 980 characters. Right now, openvpn just checks that we have at most 980 base64 characters and then tries to decode them into a 733 byte buffer. But 980 characters of base64 can encode up to 735 bytes. In that case, openvpn gives a fatal error about being unable to decode the base64 which I find misleading. My patch always allocates a large enough buffer to decode the base64 and checks that the decoded length is <= 733. An alternative would be to check the base64 length, decode to a 735 bytes buffer, then check the decoded length. I thought it's cleaner to have one length check, but I don't have a strong opinion about this. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 2/2] Fix message for too long tls-crypt-v2 metadata
> So if you have a limit like 733, you need to actually decode the base64 > to check if it is short enough. The alternative would be to only allow > 732 bytes, so we could check the base64 length again or use 735 bytes > and use a maximum tls-crypt wrapped key size of 1026 bytes (which sounds > a bit weird) Allowing 732 bytes/979 characters actually seems like the cleanest solution to me. It somehow just didn't occur to me. Well, now that my solution is acked, we can just go with it. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 2/2] Implement using --peer-fingerprint without CA certificates
The grammar in the 3rd sentence in the comment below is messed up. (I think I
understand it, but I'm not sure.)
> +if (session->opt->verify_hash_no_ca)
> +{
> +/*
> + * If we decide to verify the peer certificate based on the
> fingerprint
> + * we ignore wrong dates and the certificate not being trusted.
> + * Any other problem with the certificate (wrong key, bad cert,...)
> + * will still trigger an error.
> + * Clearing these flags relies on verify_cert will later rejecting a
> + * certificate that has no matching fingerprint.
> + */
> +uint32_t flags_ignore = MBEDTLS_X509_BADCERT_NOT_TRUSTED
> +| MBEDTLS_X509_BADCERT_EXPIRED
> +| MBEDTLS_X509_BADCERT_FUTURE;
> +*flags = *flags & ~flags_ignore;
> +}
> +
Also, this comment is copied verbatim from Jason's commit 423ced962d which has
been reverted. I'm not a lawyer, but since comments are relatively easy to
rephrase, I think it's better to do that. My suggestion:
/*
* If we verify the peer certificate based only on the fingerprint,
* we ignore flags regarding the certificate's validity period and
* the certificate being untrusted (because we don't have a CA to
* check against).
* Any other flags will still trigger an error.
*
* If the certificate's fingerprint doesn't match, it will be rejected
* by verify_cert later.
*/
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] reliable: retransmit if 3 follow-up ACKs are received
Hi!
I'm currently preparing the OpenVPN-NL 2.5 release at Fox-IT. (We're a
bit behind the times...) I thought that one of our patches, by Steffan
Karger, could be useful in regular OpenVPN. He said that he hadn't
submitted it yet, and thought it would be a good idea to ask.
The patch increases the performance of the control channel when there is
a small amount of packet loss by resending packets more aggressively. In
reliable_send, packets are resent before the timeout expires if at least
3 later packets have been ACK'd.
The reasoning is that if we receive ACKs for later packets, the
connection seems to be functional and we should be able to try again.
This policy is similar to many TCP implementations.
Please let me know if you think this is useful, and if you would like me
to make any changes to the patch.
Signed-off-by: Steffan Karger
---
src/openvpn/reliable.c | 14 +++---
src/openvpn/reliable.h | 1 +
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index 6c1f2da1..04d053bd 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -41,6 +41,8 @@
#include "memdbg.h"
+#define N_ACK_RETRANSMIT 3
+
/*
* verify that test - base < extent while allowing for base or test wraparound
*/
@@ -382,7 +384,10 @@ reliable_send_purge(struct reliable *rel, const struct
reliable_ack *ack)
}
#endif
e->active = false;
-break;
+}
+else if (e->active && e->packet_id < pid)
+{
+e->n_acks++;
}
}
}
@@ -555,7 +560,7 @@ reliable_can_send(const struct reliable *rel)
if (e->active)
{
++n_active;
-if (now >= e->next_try)
+if (now >= e->next_try || e->n_acks >= N_ACK_RETRANSMIT)
{
++n_current;
}
@@ -581,7 +586,8 @@ reliable_send(struct reliable *rel, int *opcode)
for (i = 0; i < rel->size; ++i)
{
struct reliable_entry *e = >array[i];
-if (e->active && local_now >= e->next_try)
+if (e->active
+&& (e->n_acks >= N_ACK_RETRANSMIT || local_now >= e->next_try))
{
if (!best || reliable_pid_min(e->packet_id, best->packet_id))
{
@@ -599,6 +605,7 @@ reliable_send(struct reliable *rel, int *opcode)
/* constant timeout, no backoff */
best->next_try = local_now + best->timeout;
#endif
+best->n_acks = 0;
*opcode = best->opcode;
dmsg(D_REL_DEBUG, "ACK reliable_send ID " packet_id_format " (size=%d
to=%d)",
(packet_id_print_type)best->packet_id, best->buf.len,
@@ -686,6 +693,7 @@ reliable_mark_active_incoming(struct reliable *rel, struct
buffer *buf,
e->opcode = opcode;
e->next_try = 0;
e->timeout = 0;
+e->n_acks = 0;
dmsg(D_REL_DEBUG, "ACK mark active incoming ID " packet_id_format,
(packet_id_print_type)e->packet_id);
return;
}
diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h
index a84d4290..bf0b561b 100644
--- a/src/openvpn/reliable.h
+++ b/src/openvpn/reliable.h
@@ -72,6 +72,7 @@ struct reliable_entry
interval_t timeout;
time_t next_try;
packet_id_type packet_id;
+size_t n_acks;
int opcode;
struct buffer buf;
};
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [Patch] Wipe Socks5 credentials after use
Sorry about that! I'll send it again from my personal account later. -Original Message- From: Gert Doering [mailto:g...@greenie.muc.de] Sent: vrijdag 19 maart 2021 18:30 To: Maximilian Fillinger Cc: openvpn-devel@lists.sourceforge.net Subject: Re: [Openvpn-devel] [Patch] Wipe Socks5 credentials after use Hi, On Fri, Mar 19, 2021 at 04:45:18PM +, Maximilian Fillinger via Openvpn-devel wrote: [..] The patch itself is OK (I think), but actually applying it will mess up the Author: information in git, because you are sending from a domain that has DMARC p=reject. So mailman is massacring your From: line into this From: Maximilian Fillinger via Openvpn-devel ... and this is what will be the Author of the commit. Can you send this from an account that is not set up to intentionally harm mailing lists? 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 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [Patch] Wipe Socks5 credentials after use
Socks5 plaintext authentication is not exactly high security, but we
might as well memzero the credentials before leaving the function.
---
src/openvpn/socks.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c
index 36df7470..add7a6d4 100644
--- a/src/openvpn/socks.c
+++ b/src/openvpn/socks.c
@@ -104,12 +104,13 @@ socks_username_password_auth(struct socks_proxy_info *p,
const int timeout_sec = 5;
struct user_pass creds;
ssize_t size;
+bool ret = false;
creds.defined = 0;
if (!get_user_pass(, p->authfile, UP_TYPE_SOCKS,
GET_USER_PASS_MANAGEMENT))
{
msg(M_NONFATAL, "SOCKS failed to get username/password.");
-return false;
+goto cleanup;
}
if ( (strlen(creds.username) > 255) || (strlen(creds.password) > 255) )
@@ -117,7 +118,7 @@ socks_username_password_auth(struct socks_proxy_info *p,
msg(M_NONFATAL,
"SOCKS username and/or password exceeds 255 characters. "
"Authentication not possible.");
-return false;
+goto cleanup;
}
openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", (int)
strlen(creds.username),
creds.username, (int) strlen(creds.password),
creds.password);
@@ -126,7 +127,7 @@ socks_username_password_auth(struct socks_proxy_info *p,
if (size != strlen(to_send))
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP port
write failed on send()");
-return false;
+goto cleanup;
}
while (len < 2)
@@ -147,21 +148,21 @@ socks_username_password_auth(struct socks_proxy_info *p,
get_signal(signal_received);
if (*signal_received)
{
-return false;
+goto cleanup;
}
/* timeout? */
if (status == 0)
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP
port read timeout expired");
-return false;
+goto cleanup;
}
/* error */
if (status < 0)
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP
port read failed on select()");
-return false;
+goto cleanup;
}
/* read single char */
@@ -171,7 +172,7 @@ socks_username_password_auth(struct socks_proxy_info *p,
if (size != 1)
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP
port read failed on recv()");
-return false;
+goto cleanup;
}
/* store char in buffer */
@@ -182,10 +183,14 @@ socks_username_password_auth(struct socks_proxy_info *p,
if (buf[0] != 5 && buf[1] != 0)
{
msg(D_LINK_ERRORS, "socks_username_password_auth: server refused the
authentication");
-return false;
+goto cleanup;
}
-return true;
+ret = true;
+cleanup:
+secure_memzero(, sizeof(creds));
+secure_memzero(to_send, sizeof(to_send));
+return ret;
}
static bool
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
