| Message ID | CAAYFXLmcybkvkw0RJ5gPVhkxrYKRr4XkdPUJwB4Cm--jQjdXKQ@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel] boolean short-circuit auth upon failure | expand |
> Pete Nelson <petiepooo@gmail.com> hat am 09.11.2021 20:47 geschrieben: > > > When evaluating authentication plugins, stop further evaluation > once the first failure is detected. > Since plugin_call is only a thin wrapper around plugin_call_ssl I think this would short-circuit ALL plugin calls. Doesn't sound like you intended that. Regards, Frank -- Frank Lichtenheld <!doctype html> <html> <head> <meta charset="UTF-8"> </head> <body> <div class="default-style"> <br> </div> <blockquote type="cite"> <div> Pete Nelson <petiepooo@gmail.com> hat am 09.11.2021 20:47 geschrieben: </div> <div> <br> </div> <div> <br> </div> <div dir="ltr"> When evaluating authentication plugins, stop further evaluation <br>once the first failure is detected. <br> </div> </blockquote> <div class="io-ox-signature"> <div class="default-style"> Since plugin_call is only a thin wrapper around plugin_call_ssl I think this would short-circuit ALL plugin calls. Doesn't sound like you intended that. <br> </div> <div class="default-style"> <br> </div> <div class="default-style"> Regards, <br> </div> <div class="default-style"> Frank <br> </div> <div class="default-style"> <br> </div> <div class="default-style"> -- </div> <div class="default-style"> Frank Lichtenheld </div> <div class="default-style"> <br> </div> </div> </body> </html>
Thank you, Frank. Amended patch so it applies only to auth attempts.
Not sure how patchwork handles this.. my intent is this amended patch
overwrites the patch from the OP.
-- Pete
---
src/openvpn/plugin.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c
index d5704e07..02b17378 100644
--- a/src/openvpn/plugin.c
+++ b/src/openvpn/plugin.c
@@ -818,26 +818,24 @@ plugin_call_ssl(const struct plugin_list *pl,
certdepth,
current_cert
);
- switch (status)
+ if (pr)
{
- case OPENVPN_PLUGIN_FUNC_SUCCESS:
- break;
-
- case OPENVPN_PLUGIN_FUNC_DEFERRED:
- deferred = true;
- break;
-
- default:
- error = true;
+ pr->n = i + 1;
+ }
+ if (status == OPENVPN_PLUGIN_FUNC_DEFERRED)
+ {
+ deferred = true;
+ }
+ else if (status != OPENVPN_PLUGIN_FUNC_SUCCESS)
+ {
+ error = true;
+ if (type == OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY)
+ {
break;
+ }
}
}
- if (pr)
- {
- pr->n = i;
- }
-
gc_free(&gc);
if (error)
Hi, On Thu, Nov 11, 2021 at 01:23:43PM +0000, Pete Nelson wrote: > Thank you, Frank. Amended patch so it applies only to auth attempts. > > Not sure how patchwork handles this.. my intent is this amended patch > overwrites the patch from the OP. > -- Pete The best way to do that is to send the patch with "git send-email" and add a "-v2" switch to that - it won't overwrite the patch in PW, but we can see "ah, it's a v2". Please add a note to the commit message as well what is new on v2, like v2: only apply shortcut logic to authentication plugin calls (if that's what is new) Please do also do "git commit -s", so we get the signed-off-by: line. gert
When evaluating authentication plugins, stop further evaluation
once the first failure is detected.
implementation note: refactoring from a switch-case to an
if-else block allows the break statement to break out of the
outer for loop without additional control variables.
v2: add check for auth plugin before breaking out of loop
Signed-off-by: Peter Nelson <petiepooo@gmail.com>
---
src/openvpn/plugin.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c
index d5704e07..02b17378 100644
--- a/src/openvpn/plugin.c
+++ b/src/openvpn/plugin.c
@@ -818,26 +818,24 @@ plugin_call_ssl(const struct plugin_list *pl,
certdepth,
current_cert
);
- switch (status)
+ if (pr)
{
- case OPENVPN_PLUGIN_FUNC_SUCCESS:
- break;
-
- case OPENVPN_PLUGIN_FUNC_DEFERRED:
- deferred = true;
- break;
-
- default:
- error = true;
+ pr->n = i + 1;
+ }
+ if (status == OPENVPN_PLUGIN_FUNC_DEFERRED)
+ {
+ deferred = true;
+ }
+ else if (status != OPENVPN_PLUGIN_FUNC_SUCCESS)
+ {
+ error = true;
+ if (type == OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY)
+ {
break;
+ }
}
}
- if (pr)
- {
- pr->n = i;
- }
-
gc_free(&gc);
if (error)
Should that have shown up as a separate entry in patchworks? I'm having to cut/paste the results of git format-patch as I can't git send-email from my dev box, and it looks like I may have missed editing the subject line. -- Pete On Thu, Nov 11, 2021 at 1:34 PM Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Thu, Nov 11, 2021 at 01:23:43PM +0000, Pete Nelson wrote: > > Thank you, Frank. Amended patch so it applies only to auth attempts. > > > > Not sure how patchwork handles this.. my intent is this amended patch > > overwrites the patch from the OP. > > -- Pete > > The best way to do that is to send the patch with "git send-email" and > add a "-v2" switch to that - it won't overwrite the patch in PW, but > we can see "ah, it's a v2". > > Please add a note to the commit message as well what is new on v2, > like > > v2: only apply shortcut logic to authentication plugin calls > > (if that's what is new) > > Please do also do "git commit -s", so we get the signed-off-by: > line. > > 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 > gert@greenie.muc.de > <div dir="ltr"><div dir="ltr">Should that have shown up as a separate entry in patchworks? I'm having to cut/paste the results of git format-patch as I can't git send-email from my dev box, and it looks like I may have missed editing the subject line.</div><div>-- Pete<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 11, 2021 at 1:34 PM Gert Doering <<a href="mailto:gert@greenie.muc.de">gert@greenie.muc.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br> <br> On Thu, Nov 11, 2021 at 01:23:43PM +0000, Pete Nelson wrote:<br> > Thank you, Frank. Amended patch so it applies only to auth attempts.<br> > <br> > Not sure how patchwork handles this.. my intent is this amended patch<br> > overwrites the patch from the OP.<br> > -- Pete<br> <br> The best way to do that is to send the patch with "git send-email" and<br> add a "-v2" switch to that - it won't overwrite the patch in PW, but<br> we can see "ah, it's a v2".<br> <br> Please add a note to the commit message as well what is new on v2,<br> like<br> <br> v2: only apply shortcut logic to authentication plugin calls<br> <br> (if that's what is new)<br> <br> Please do also do "git commit -s", so we get the signed-off-by:<br> line.<br> <br> gert<br> -- <br> "If was one thing all people took for granted, was conviction that if you <br> feed honest figures into a computer, honest figures come out. Never doubted <br> it myself till I met a computer with a sense of humor."<br> Robert A. Heinlein, The Moon is a Harsh Mistress<br> <br> Gert Doering - Munich, Germany <a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a><br> </blockquote></div></div>
Hi, On 11/11/2021 15:26, Pete Nelson wrote: > Should that have shown up as a separate entry in patchworks? I'm having > to cut/paste the results of git format-patch as I can't git send-email > from my dev box, and it looks like I may have missed editing the subject > line. It shows up when it is detected as a proper new patch (maybe using the exact same subject fooled the detection?). You can pass -v2 also to git format-patch, so it will edit the subject for you. In any case, I'd suggest fixing git-send-email as copy/paste may introduce all kind of formatting bugs and may make the patch unusable. Cheers,
Hi, On Thu, Nov 11, 2021 at 03:34:19PM +0100, Antonio Quartulli wrote: > In any case, I'd suggest fixing git-send-email as copy/paste may > introduce all kind of formatting bugs and may make the patch unusable. Indeed. Line wraps, tab/space issues, "helpful mail clients", so git-send-email is really the magic tool. As a side note, it needs not run on the dev box - so "git format-patch", copy the result to something that has e-mail, then "git send-email $file" on that second box - works fine. ("git send-email HEAD" is obviously more convenient, though :-) ) gert
diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c index d5704e07..c6c9a63f 100644 --- a/src/openvpn/plugin.c +++ b/src/openvpn/plugin.c @@ -818,24 +818,19 @@ plugin_call_ssl(const struct plugin_list *pl, certdepth, current_cert ); - switch (status) + if (pr) { - case OPENVPN_PLUGIN_FUNC_SUCCESS: - break; - - case OPENVPN_PLUGIN_FUNC_DEFERRED: - deferred = true; - break; - - default: - error = true; - break; + pr->n = i + 1; + } + if (status == OPENVPN_PLUGIN_FUNC_DEFERRED) + { + deferred = true; + } + else if (status != OPENVPN_PLUGIN_FUNC_SUCCESS) + { + error = true; + break; } - } - - if (pr) - { - pr->n = i; } gc_free(&gc);