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

[Thomas Lamprecht]

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>
---

changes v1 -> v2:
* add missing 'last' in request "success branch"

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

diff --git a/src/PVE/OTP.pm b/src/PVE/OTP.pm
index 019076b..de97636 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,26 @@ 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;
+	    last;
+	}
     }
+    # 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