[pve-devel] [RFC access-control 2/2] drop oathtool dependency

Fabian Grünbichler f.gruenbichler at proxmox.com
Thu Jun 30 15:27:33 CEST 2016


comment inline, rest looks good to me

> Wolfgang Bumiller <w.bumiller at proxmox.com> hat am 30. Juni 2016 um 14:43 geschrieben:
>  sub yubico_compute_param_sig {
> @@ -1278,20 +1296,21 @@ sub oath_verify_otp {
>      $digits = 6 if !$digits;
>  
>      my $found;
> -
> -    my $parser = sub {
> -	my $line = shift;
> -
> -	if ($line =~ m/^\d{6}$/) {
> -	    $found = 1 if $otp eq $line;
> -	}
> -    };
> -
>      foreach my $k (PVE::Tools::split_list($keys)) {
>  	# Note: we generate 3 values to allow small time drift
> -	my $now = localtime(time() - $step);
> -	my $cmd = ['oathtool', '--totp', '--digits', $digits, '-N', $now, '-s', $step, '-w', '2', '-b', $k];
> -	eval { run_command($cmd, outfunc => $parser, errfunc => sub {}); };
> +	my $binkey;
> +	if ($k =~ /^[A-Z2-7=]{32}$/) {
> +	    $binkey = MIME::Base32::decode_rfc3548($k);
> +	} elsif ($k =~ /^[A-Fa-f0-9]{40}$/) {
> +	    $binkey = pack('H*', $k);
> +	}
> +

there should probably be a final "else" branch here to provide a sane error message (and error out early) - up to this point, $keys is only known to not be just whitespace, so both REs might not match. alternatively, the whitespace check in line 1275 could be extended, but that is probably less readable ;)

> +	# force integer division for time/step
> +	use integer;
> +	my $now = time()/$step - 1;
> +	$found = 1 if $otp eq hotp($binkey, $now+0, $digits);
> +	$found = 1 if $otp eq hotp($binkey, $now+1, $digits);
> +	$found = 1 if $otp eq hotp($binkey, $now+2, $digits);
>  	last if $found;
>      }
>  
> diff --git a/control.in b/control.in
> index b74aaf1..758e9a0 100644
> --- a/control.in
> +++ b/control.in
> @@ -3,7 +3,7 @@ Version: @@VERSION@@-@@PKGRELEASE@@
>  Section: perl
>  Priority: optional
>  Architecture: @@ARCH@@
> -Depends: libc6 (>= 2.3), perl (>= 5.6.0-16), libcrypt-openssl-rsa-perl, libcrypt-openssl-random-perl, libjson-xs-perl, libjson-perl, libterm-readline-gnu-perl,libnet-ldap-perl, libpve-common-perl, pve-cluster, libauthen-pam-perl, libnet-ssleay-perl, liburi-perl, libwww-perl, oathtool, libmime-base32-perl
> +Depends: libc6 (>= 2.3), perl (>= 5.6.0-16), libcrypt-openssl-rsa-perl, libcrypt-openssl-random-perl, libjson-xs-perl, libjson-perl, libterm-readline-gnu-perl,libnet-ldap-perl, libpve-common-perl, pve-cluster, libauthen-pam-perl, libnet-ssleay-perl, liburi-perl, libwww-perl, libmime-base32-perl
>  Maintainer: Proxmox Support Team <support at proxmox.com>
>  Description: Proxmox VE access control library
>   This package contains the role based user management and access
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>



More information about the pve-devel mailing list