Patch looks reasonable when looking at it with -w.
But I have some suggestions to further simplify the function. See below.
> Arne Schwabe <a...@rfc2549.org> hat am 18.05.2022 11:32 geschrieben:
> This simplifies the buffer handling in the method and adds a quick
> return instead of wrapping the whole method in a if (pull) block
> ---
> src/openvpn/push.c | 96 ++++++++++++++++++++++++----------------------
> 1 file changed, 51 insertions(+), 45 deletions(-)
>
> diff --git a/src/openvpn/push.c b/src/openvpn/push.c
> index 51b8bd521..be118831d 100644
> --- a/src/openvpn/push.c
> +++ b/src/openvpn/push.c
[...]
> #ifdef ENABLE_MANAGEMENT
> - if (management)
> + if (management)
> + {
> + const char *reason = NULL;
> + if (authfail_extended && BLEN(&buf))
> {
> - const char *reason = NULL;
> - struct buffer buf = *buffer;
> - if (buf_string_compare_advance(&buf, "AUTH_FAILED,") &&
> BLEN(&buf))
> - {
> - reason = BSTR(&buf);
> - }
> - management_auth_failure(management, UP_TYPE_AUTH, reason);
> + reason = BSTR(&buf);
> }
> + management_auth_failure(management, UP_TYPE_AUTH, reason);
> + }
> #endif
> - /*
> - * Save the dynamic-challenge text even when management is defined
> - */
> - {
> + /*
> + * Save the dynamic-challenge text even when management is defined
> + */
> + {
This additional block has no meaning now that you don't define a variable
anymore.
> #ifdef ENABLE_MANAGEMENT
This ifdef can be merged with the previous ifdef to further simplify the
function.
> - struct buffer buf = *buffer;
> - if (buf_string_match_head_str(&buf, "AUTH_FAILED,CRV1:") &&
> BLEN(&buf))
> - {
> - buf_advance(&buf, 12); /* Length of "AUTH_FAILED," substring
> */
> - ssl_put_auth_challenge(BSTR(&buf));
> - }
> -#endif
> + if (authfail_extended
> + && buf_string_match_head_str(&buf, "CRV1:") && BLEN(&buf))
> + {
> + ssl_put_auth_challenge(BSTR(&buf));
> }
> +#endif
> }
> }
So something like this on top of this patch:
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index be118831..6b8bc0e1 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -104,19 +104,15 @@ receive_auth_failed(struct context *c, const struct
buffer *buffer)
}
management_auth_failure(management, UP_TYPE_AUTH, reason);
}
-#endif
/*
* Save the dynamic-challenge text even when management is defined
*/
+ if (authfail_extended
+ && buf_string_match_head_str(&buf, "CRV1:") && BLEN(&buf))
{
-#ifdef ENABLE_MANAGEMENT
- if (authfail_extended
- && buf_string_match_head_str(&buf, "CRV1:") && BLEN(&buf))
- {
- ssl_put_auth_challenge(BSTR(&buf));
- }
-#endif
+ ssl_put_auth_challenge(BSTR(&buf));
}
+#endif
}
/*
flichtenheld@MININT-961QE0A:~/openvpn/community/openvpn (master *)$ git
--no-pager diff
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index be118831..6b8bc0e1 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -104,19 +104,15 @@ receive_auth_failed(struct context *c, const struct
buffer *buffer)
}
management_auth_failure(management, UP_TYPE_AUTH, reason);
}
-#endif
/*
* Save the dynamic-challenge text even when management is defined
*/
+ if (authfail_extended
+ && buf_string_match_head_str(&buf, "CRV1:") && BLEN(&buf))
{
-#ifdef ENABLE_MANAGEMENT
- if (authfail_extended
- && buf_string_match_head_str(&buf, "CRV1:") && BLEN(&buf))
- {
- ssl_put_auth_challenge(BSTR(&buf));
- }
-#endif
+ ssl_put_auth_challenge(BSTR(&buf));
}
+#endif
}
/*
Regards,
--
Frank Lichtenheld
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
