[PATCH] Juniper: support password and 2FA fields in the same form

Ash Holland ash at sorrel.sh
Wed Jun 24 17:52:59 EDT 2020


On Wed Jun 24, 2020 at 10:45 PM BST, Daniel Lenski wrote:
> On Wed, Jun 24, 2020 at 2:27 PM Ash Holland <ash at sorrel.sh> wrote:
> >  auth-juniper.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/auth-juniper.c b/auth-juniper.c
> > index 19d439784..f4d9733fa 100644
> > --- a/auth-juniper.c
> > +++ b/auth-juniper.c
> > @@ -74,6 +74,18 @@ static int oncp_can_gen_tokencode(struct openconnect_info *vpninfo,
> >             vpninfo->token_bypassed)
> >                 return -EINVAL;
> >
> > +       if (!strcmp(form->auth_id, "frmLogin")) {
> > +               // The first "password" input in frmLogin is likely to be a password, not 2FA token
> > +               struct oc_form_opt **p = &form->opts;
> > +               while (*p) {
> > +                       if ((*p)->type == OC_FORM_OPT_PASSWORD) {
> > +                               return can_gen_tokencode(vpninfo, form, opt);
> > +                       }
> > +                       p = &(*p)->next;
> > +               }
>
> 1) It appears to me that you haven't actually implemented the
> skip-this-field-if-it-is-the-first-password-input behavior.

This might be more closely described as "it's only a 2FA field if it's
not the first password input", but they're equivalent, I think.

> 2) Why `**p`? Everything here could be simplified by using `struct
> oc_form_opt *p = form->opts`.

Because I copy-pasted it from the bit in parse_input_node where there's
a similar loop. Can you tell I don't write much C? :P

I can send another patch with this corrected if you'd like.

> > I've tested this against the VPN I mentioned in [1], and it seems to
> > work well
>
> Hrm… are you sure? It appears the code that you sent should cause the
> 2FA token to fill the *first* password. I don't understand how this
> would work, and actually prompt you for the first password rather than
> auto-fill the token code, from the CLI.

It definitely works, I've just checked again and compared the master
branch to mine. On master I'm prompted for "password" and "password#2";
with this patch I get:

    OK to generate INITIAL tokencode
    frmLogin
    password:
    Generating OATH HOTP token code
    POST https://webvpn.york.ac.uk/dana-na/auth/url_default/login.cgi

The logic goes something like:

- if this is a frmLogin form:
  - loop through all previously-seen fields
  - if a password field has already been seen, this is a 2FA field
    (subject to the checks in can_gen_tokencode)
  - otherwise, this is the first password field, so it's not a 2FA field



More information about the openconnect-devel mailing list