On Wed, Aug 14, 2019 at 02:15:40PM +0200, Thomas Lamprecht wrote:
> Am 7/18/19 um 4:44 PM schrieb Oguz Bektas:
> > this patch enables to pass totp codes during cluster join if tfa has been
> > enabled for root@pam (or any other user actually, but root seems to cause
> > the
> > most problems).
> >
> > u2f support is still not implemented.
> >
> > Co-developed-by: Thomas Lamprecht <t.lampre...@proxmox.com>
> > Co-developed-by: Wolfgang Bumiller <w.bumil...@proxmox.com>
> > Signed-off-by: Oguz Bektas <o.bek...@proxmox.com>
> > ---
> >
> > v2 -> v3:
> > implement thomas' suggestions, namely:
> > * factor out code and handle it outside of the login func
> > * pass $type and $challenge after detecting a two-factor-auth ticket
> > (the detection regex can be improved i think, but i'll leave it like
> > this for now)
> > * also if $type isn't detected, abort after doing a raise
> >
> > PVE/APIClient/LWP.pm | 30 ++++++++++++++++++++++++------
> > 1 file changed, 24 insertions(+), 6 deletions(-)
> >
> > diff --git a/PVE/APIClient/LWP.pm b/PVE/APIClient/LWP.pm
> > index c0e30ff..2cf7ac7 100755
> > --- a/PVE/APIClient/LWP.pm
> > +++ b/PVE/APIClient/LWP.pm
> > @@ -92,6 +92,23 @@ sub update_ticket {
> > $agent->default_header('Cookie', $cookie);
> > }
> >
> > +sub two_factor_auth_login {
> > + my ($self, $type, $challenge) = @_;
> > +
> > + if ($type eq 'PVE:tfa') {
> > + raise("TFA-enabled join currently works only with a TTY.") if !-t STDIN;
>
> This has nothing to do with a PVE node join, it hits us there, yes,
> but that's only the case because we currently only use the apiclient
> there.
> But we may ecxtend it's use in the future, and also other, external
> people and projects may already use it for different cases.
> So please adapt the error message(s) to something more general and
> fitting, i.e., you're doing a login which failed.
>
> > + print "\nEnter OTP code for user $self->{username}: ";
> > + my $tfa_response = <STDIN>;
> > + chomp $tfa_response;
> > + return $self->post('/api2/json/access/tfa', {response =>
> > $tfa_response});
> > + } elsif ($type eq 'PVE:u2f') {
> > + # TODO: implement u2f-enabled join
> > + raise("U2F-enabled join is currently not implemented.");
>
> same here
>
> > + } else {
> > + raise("Authentication type not recognized, aborting!");
>
> maybe include the unrecognised type in the error to help us understand
> why someone ran into this, if it happens.
>
> > + }
> > +}
> > +
> > sub login {
> > my ($self) = @_;
> >
> > @@ -129,15 +146,16 @@ sub login {
> > my $res = from_json($response->decoded_content, {utf8 => 1,
> > allow_nonref => 1});
> >
> > my $data = $extract_data->($res);
> > -
> > - # TODO: make it possible to use tfa
> > - if ($data->{ticket} =~ m/^PVE:tfa!/) {
> > - raise("Two Factor Auth is not yet implemented! Try disabling TFA for
> > the user '$username'.\n");
> > - }
> > -
> > $self->update_ticket($data->{ticket});
> > $self->update_csrftoken($data->{CSRFPreventionToken});
> >
> > + # handle two-factor login
> > + if ($data->{ticket} =~ m{^([^\s!]+)![^!]*(!([0-9a-zA-Z/.=_\-+]+))?$}) {
>
> maybe a
>
> my $tfa_ticket_re = qr/^([^\s!]+)![^!]*(!([0-9a-zA-Z/.=_\-+]+))?$/;
While the base64 pattern there is currently only used specifically with u2f
($data =~ /^u2f!...), whereas the 'tfa' case is actually matched as
`$data =~ /^tfa!(.*)$/` in the code currently. (Iow. the number of
"parameters" and restrictions on the content depends on the authentication
type.)
So rather thant restricting the tail, just end with `(?:!(.*))$`, also note
that you are including the '!' character in $2.
> if ($data->{ticket} =~ m/$tfa_ticket_re/) ...
No need to for the m//, jus use =~ $tfa_ticket_re ;-)
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
