[pve-devel] [PATCH common] fix #2023: try out all available yubico api endpoints

Thomas Lamprecht t.lamprecht at proxmox.com
Tue Apr 9 09:11:33 CEST 2019


While we moved to the new v2 API a bit ago we still only ever tried
a single hard coded endpoint, but yubico say:

> To ensure high-availability, configure your client to
> simultaneously issue requests to all five addresses and accept the
> first successful reply.
-- yubico server status site [0]

For now I ommitted the initial parallel requesting, it's a best
effort try, serialized.
That could be added on top of this later one, if desired, either by
just forking and starting connections in parallel, or some anyevent
like async request.

But lets hope we can save this complexity completely and wait out
until yubico sets up a single highly-available API endpoint. Until
then this is a cheap "one can only win" approach.

[0]: https://status.yubico.com/2018/11/26/deprecating-yubicloud-v1-protocol-plain-text-requests-and-old-tls-versions/

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---

best (re)viewed with git's ignore all space option (-w) to ignore indentation
only changes

 src/PVE/OTP.pm | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/src/PVE/OTP.pm b/src/PVE/OTP.pm
index 019076b..f9b6654 100644
--- a/src/PVE/OTP.pm
+++ b/src/PVE/OTP.pm
@@ -46,7 +46,7 @@ sub yubico_compute_param_sig {
 }
 
 sub yubico_verify_otp {
-    my ($otp, $keys, $url, $api_id, $api_key, $proxy) = @_;
+    my ($otp, $keys, $api_url, $api_id, $api_key, $proxy) = @_;
 
     die "yubico: missing password\n" if !defined($otp);
     die "yubico: missing API ID\n" if !defined($api_id);
@@ -55,7 +55,17 @@ sub yubico_verify_otp {
 
     die "yubico: wrong OTP length\n" if (length($otp) < 32) || (length($otp) > 48);
 
-    $url = 'https://api2.yubico.com/wsapi/2.0/verify' if !defined($url);
+    my $urls;
+    if (!defined($api_url)) {
+	# see https://status.yubico.com/2018/11/26/deprecating-yubicloud-v1-protocol-plain-text-requests-and-old-tls-versions/
+	$urls = [
+	    map { "https://api${_}.yubico.com/wsapi/2.0/verify" } (''), (2..5)
+	];
+    } elsif (!ref($api_url)) {
+	$urls = [ $api_url ];
+    } else {
+	$urls = $api_url;
+    }
 
     my $params = {
 	nonce =>  Digest::SHA::hmac_sha1_hex(time(), rand()),
@@ -68,8 +78,6 @@ sub yubico_verify_otp {
 
     $paramstr .= "&h=$sig" if $api_key;
 
-    my $req = HTTP::Request->new('GET' => "$url?$paramstr");
-
     my $ua = LWP::UserAgent->new(protocols_allowed => ['http', 'https'], timeout => 30);
 
     if ($proxy) {
@@ -78,13 +86,25 @@ sub yubico_verify_otp {
 	$ua->env_proxy;
     }
 
-    my $response = $ua->request($req);
-    my $code = $response->code;
+    my ($response, $lasterr);
+    for my $url (@$urls) {
+	my $req = HTTP::Request->new('GET' => "$url?$paramstr");
 
-    if ($code != 200) {
-	my $msg = $response->message || 'unknown';
-	die "Invalid response from server: $code $msg\n";
+	$response = $ua->request($req);
+	my $code = $response->code;
+
+	if ($code != 200) {
+	    $lasterr = $response->message || 'unknown';
+	    warn "Invalid response from '$url': $code $lasterr\n";
+	    next;
+	} else {
+	    $lasterr = undef;
+	}
     }
+    # just report last error to user, don't throw > 5, possible identical, errors
+    # at him, they're logged above for an admin already..
+    die "Could not connect to any yubico OTP API server specified: $lasterr\n"
+	if defined($lasterr);
 
     my $raw = $response->decoded_content;
 
-- 
2.20.1





More information about the pve-devel mailing list