Fix bug where two keys with different usage fails

The problem is that the typing is defined as a hashref of arrays, eg { signing => [ pem, pem, pem], } and the input was: [ { signing => [ pem, pem, pem ] } ] This fails and breaks the IdP code. Fixes: #116 Signed-off-by: Wesley Schwengle <[email protected]>
perl-net-saml2 · Sep 1, 2022 · e95e7c2 · e95e7c2
1 parent 66a4146
commit e95e7c2
Show file tree

Hide file tree

Showing 4 changed files with 178 additions and 76 deletions.
diff --git a/lib/Net/SAML2/IdP.pm b/lib/Net/SAML2/IdP.pm
@@ -1,9 +1,8 @@
-use strict;
-use warnings;
 package Net::SAML2::IdP;
+use Moose;
+
 # VERSION
 
-use Moose;
 use MooseX::Types::URI qw/ Uri /;
 
 # ABSTRACT: Net::SAML2::IdP - SAML Identity Provider object
@@ -121,36 +120,24 @@ sub new_from_xml {
 
     my $data;
 
-    for my $sso (
-        $xpath->findnodes(
-            '//md:EntityDescriptor/md:IDPSSODescriptor/md:SingleSignOnService')
-        )
-    {
+    my $basepath  = '//md:EntityDescriptor/md:IDPSSODescriptor';
+
+    for my $sso ($xpath->findnodes("$basepath/md:SingleSignOnService")) {
         my $binding = $sso->getAttribute('Binding');
         $data->{SSO}->{$binding} = $sso->getAttribute('Location');
     }
 
-    for my $slo (
-        $xpath->findnodes(
-            '//md:EntityDescriptor/md:IDPSSODescriptor/md:SingleLogoutService')
-        )
-    {
+    for my $slo ($xpath->findnodes("$basepath/md:SingleLogoutService")) {
         my $binding = $slo->getAttribute('Binding');
         $data->{SLO}->{$binding} = $slo->getAttribute('Location');
     }
 
-    for my $art (
-        $xpath->findnodes(
-            '//md:EntityDescriptor/md:IDPSSODescriptor/md:ArtifactResolutionService')
-        )
-    {
+    for my $art ($xpath->findnodes("$basepath/md:ArtifactResolutionService")) {
         my $binding = $art->getAttribute('Binding');
         $data->{Art}->{$binding} = $art->getAttribute('Location');
     }
 
-    for my $format (
-        $xpath->findnodes('//md:EntityDescriptor/md:IDPSSODescriptor/md:NameIDFormat'))
-    {
+    for my $format ($xpath->findnodes("$basepath/md:NameIDFormat")) {
         $format = $format->string_value;
         $format =~ s/^\s+//g;
         $format =~ s/\s+$//g;
@@ -164,50 +151,26 @@ sub new_from_xml {
         }
     }
 
-    my @certs = ();
-
-    for my $key (
-        $xpath->findnodes('//md:EntityDescriptor/md:IDPSSODescriptor/md:KeyDescriptor'))
-    {
-        my @uses;
-        push (@uses, $key->getAttribute('use') || 'signing');
-        push (@uses, 'encryption') if !$key->getAttribute('use');
-
-
-        $key->setNamespace('http://www.w3.org/2000/09/xmldsig#', 'ds');
-
-        my ($text)
-            = $key->findvalue("ds:KeyInfo/ds:X509Data/ds:X509Certificate", $key)
-            =~ /^\s*(.+?)\s*$/s;
-
-        # rewrap the base64 data from the metadata; it may not
-        # be wrapped at 64 characters as PEM requires
-        $text =~ s/\n//g;
-
-        my @lines;
-        while(length $text > 64) {
-            push @lines, substr $text, 0, 64, '';
+    my %certs = ();
+    for my $key ($xpath->findnodes("$basepath/md:KeyDescriptor")) {
+        my $use = $key->getAttribute('use');
+        my $pem = $class->_get_pem_from_keynode($key);
+        if (!$use) {
+            push(@{$certs{signing}}, $pem);
+            push(@{$certs{encryption}}, $pem);
         }
-        push @lines, $text;
-
-        $text = join "\n", @lines;
-
-        # form a PEM certificate
-        for my $use (@uses) {
-            my $pem->{$use}
-                = sprintf("-----BEGIN CERTIFICATE-----\n%s\n-----END CERTIFICATE-----\n",
-                $text);
-            push (@certs, $pem);
+        else {
+            push(@{$certs{$use}}, $pem);
         }
     }
 
-    my $self = $class->new(
+    return $class->new(
         entityid => $xpath->findvalue('//md:EntityDescriptor/@entityID'),
         sso_urls => $data->{SSO},
         slo_urls => $data->{SLO} || {},
         art_urls => $data->{Art} || {},
-        certs                        => \@certs,
-        cacert                       => $args{cacert},
+        certs    => \%certs,
+        cacert   => $args{cacert},
         $data->{DefaultFormat}
         ? (
             default_format => $data->{DefaultFormat},
@@ -216,9 +179,34 @@ sub new_from_xml {
         : (),
     );
 
-    return $self;
 }
 
+sub _get_pem_from_keynode {
+    my $self = shift;
+    my $node = shift;
+
+    $node->setNamespace('http://www.w3.org/2000/09/xmldsig#', 'ds');
+
+    my ($text)
+        = $node->findvalue("ds:KeyInfo/ds:X509Data/ds:X509Certificate", $node)
+        =~ /^\s*(.+?)\s*$/s;
+
+    # rewrap the base64 data from the metadata; it may not
+    # be wrapped at 64 characters as PEM requires
+    $text =~ s/\n//g;
+
+    my @lines;
+    while(length $text > 64) {
+        push @lines, substr $text, 0, 64, '';
+    }
+    push @lines, $text;
+
+    $text = join "\n", @lines;
+
+    return "-----BEGIN CERTIFICATE-----\n$text\n-----END CERTIFICATE-----\n";
+}
+
+
 # BUILDARGS ( hashref of the parameters passed to the constructor )
 #
 # Called after the object is created to validate the IdP using the cacert
@@ -233,32 +221,22 @@ around BUILDARGS => sub {
     if ($params{cacert}) {
         my $ca = Crypt::OpenSSL::Verify->new($params{cacert}, { strict_certs => 0, });
 
-        my $verified = 0;
-        my %errors;
-        my %certs;
-
-        for my $pem (@{ $params{certs} }) {
-            for my $use (keys %{$pem}) {
-                my @tmpcrt;
-                my $cert = Crypt::OpenSSL::X509->new_from_string($pem->{$use});
+        my %certificates;
+        for my $use (keys %{$params{certs}}) {
+            my $certs = $params{certs}{$use};
+            for my $pem (@{$certs}) {
+                my $cert = Crypt::OpenSSL::X509->new_from_string($pem);
                 ## BUGBUG this is failing for valid things ...
                 eval { $ca->verify($cert) };
                 if ($@) {
-                    $errors{$cert->fingerprint_sha256} = $@;
+                    warn "Can't verify IdP cert: $@";
                     next;
                 }
-                $verified = 1;
-                push @tmpcrt, $pem->{$use};
-
-                $certs{$use} = \@tmpcrt;
+                push(@{$certificates{$use}}, $pem);
             }
         }
 
-        $params{certs} = \%certs;
-
-        if (!$verified) {
-             warn "Can't verify IdP signing cert: ", %errors, "\n";
-        }
+        $params{certs} = \%certificates;
     }
 
     return $self->$orig(%params);

diff --git a/t/01-create-idp.t b/t/01-create-idp.t
@@ -387,4 +387,22 @@ XML
     );
 }
 
+{
+    my $xml = path('t/data/idp-metadata-signing-encryption.xml')->slurp;
+    my $idp = Net::SAML2::IdP->new_from_xml(
+        xml => $xml,
+    );
+
+    isa_ok($idp, "Net::SAML2::IdP");
+    is(@{$idp->cert('signing')}, 1, 'Got one signing cert');
+    is(@{$idp->cert('encryption')}, 1, 'Got one encryption cert');
+}
+
+{
+    my $xml = path('t/data/idp-metadata-multiple-invalid-use.xml')->slurp;
+    my $idp = Net::SAML2::IdP->new_from_xml(xml => $xml);
+    is(@{$idp->cert('signing')}, 1, 'Got one signing cert');
+    is(@{$idp->cert('encryption')}, 2, 'Got two encryption certs');
+}
+
 done_testing;
diff --git a/t/data/idp-metadata-multiple-invalid-use.xml b/t/data/idp-metadata-multiple-invalid-use.xml
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://accounts.google.com/o/saml2?idpid=C01nccos6" validUntil="2022-04-20T00:56:59.000Z">
+  <md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+    <md:KeyDescriptor>
+      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+        <ds:X509Data>
+          <ds:X509Certificate>MIIDdDCCAlygAwIBAgIGAVuOAyTCMA0GCSqGSIb3DQEBCwUAMHsxFDASBgNVBAoTC0dvb2dsZSBJ
+bmMuMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MQ8wDQYDVQQDEwZHb29nbGUxGDAWBgNVBAsTD0dv
+b2dsZSBGb3IgV29yazELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWEwHhcNMTcwNDIx
+MDA1NjU5WhcNMjIwNDIwMDA1NjU5WjB7MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEWMBQGA1UEBxMN
+TW91bnRhaW4gVmlldzEPMA0GA1UEAxMGR29vZ2xlMRgwFgYDVQQLEw9Hb29nbGUgRm9yIFdvcmsx
+CzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
+MIIBCgKCAQEAsziYPBZSqLzs5pFfoZPTVyoHwM4fmI9qQIScWJB0435ss5HDRDKBTvO8rg2sGbfM
+SYVINLvesTeTNkPkMfoSplne6uWjAWkoskvls+0bzp8jAXb+UJ1cZplQxhWW25TsEcpcMX+p4J0v
+/syO9lyDubNSpchPGIuzitKXdwFkgcOiCFk+01MF3COKaO6HoLr5Jqf7I6qjJy72h3ZC7rGUtwjC
+QPaAfeLIM2uGmdOcm7Ql/89hBSjJYvJOXOyEZqppYd2tat7UPh4q8UTmPhwETaVssThfNZUpt9iZ
+cCFcWaVh/mljxD/V20VNMAlzN8VXM/WjbBSAePS30VRF6xXlzwIDAQABMA0GCSqGSIb3DQEBCwUA
+A4IBAQAKxc4M+il/bQQcmm/JNKF5ypeC0YH+oRjSwec84pznP2qqjWosR2uR9hL6CN2dOZU/uCaX
+1nZviGIlsw9n7JUEfxMs4azYWfdTz7C7l9x6HpF/QK5NwBe1z3iPZAiKR8hBqt7IeN3nlT4RsaMr
+0E9UbG5pimxEG0uves5BoHS+3Xfb8sd33dNRyCm22nb2wWGG+jTKy2G8/24gHB2c0X6/AiD3dZbY
+qf7pFRWrXefkMaUMaCMnQh+owjjxVNEAAQ8QU/2Hwhz6pR0eEGL6UwWufJX8uSGaxqn+397MdrI4
+CE0gQAZXrB1L3PU9tdyiap2hWQPo8T2STpOtvcovFPHs</ds:X509Certificate>
+        </ds:X509Data>
+      </ds:KeyInfo>
+    </md:KeyDescriptor>
+    <md:KeyDescriptor use="encryption">
+      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+        <ds:X509Data>
+          <ds:X509Certificate>MIIDdDCCAlygAwIBAgIGAWj+rlteMA0GCSqGSIb3DQEBCwUAMHsxFDASBgNVBAoTC0dvb2dsZSBJ
+bmMuMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MQ8wDQYDVQQDEwZHb29nbGUxGDAWBgNVBAsTD0dv
+b2dsZSBGb3IgV29yazELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWEwHhcNMTkwMjE4
+MDMzNzQ1WhcNMjQwMjE3MDMzNzQ1WjB7MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEWMBQGA1UEBxMN
+TW91bnRhaW4gVmlldzEPMA0GA1UEAxMGR29vZ2xlMRgwFgYDVQQLEw9Hb29nbGUgRm9yIFdvcmsx
+CzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
+MIIBCgKCAQEAubE4/cl70Lc2f7VV+ZJyzYIzuAMj6ejlbtRnym2kgyYjaaO0MVU/r38oRC7UqdQI
+XwA0/S1Eu0k6EYcCUiyTGWz3HKv/OSOSnSDpN4wEWaZbmJYvu8SjWZQCVdcM4fx1kzrtE+LEBTOK
+gj0k2G1qUMNI7xaqiJONO9aIeCic5zbACNpc+IOZoRS4RaY5Ie7W1ZIXAJ0xWL3snVdqklaJzzU2
+Myt5QX6W1Sd411Hzo+Ihi5ksq18ML1tgMTFIqLkY2Luf5JJdZFRcCgvHrFF2CQywE0ftZtSSg5wD
++hp3PJpL5bxxF1vSdyKQbUia1Buc8+Cy6NTIbmLLIrcyULyHSwIDAQABMA0GCSqGSIb3DQEBCwUA
+A4IBAQAXKjWrRzhgUW1+pK3V51MGl2b/yf33Ac4fm7GQql0Ag0Neye1EmdLjD2N9gVeFawMcfRT4
+GABBHtS5bu01M8QHHAGjbBKfqOaPJc39v0Y/RCSd/FzXg99hNT5UggAWVR+vC6a1IrVSUa5eKe82
+yBAubbdftvGtKHG90HIAsb1iyMKK2rGnTupgJfJIUTWhWnWuemIVwduErFCxng//jYXViyEloz73
+0faMIp6eNSD2+2cCssVGFb6FxhCvVuNh6tgXv4vErVSWerFk/GcIh5n/biaDy/gEtAqgK154AfOi
+fpDP3l3ZV/celj1wSwcLF90e84XaVIkzb3veTcWhqaaq</ds:X509Certificate>
+        </ds:X509Data>
+      </ds:KeyInfo>
+    </md:KeyDescriptor>
+    <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://accounts.google.com/o/saml2/idp?idpid=C01nccos6"/>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://accounts.google.com/o/saml2/idp?idpid=C01nccos6"/>
+  </md:IDPSSODescriptor>
+</md:EntityDescriptor>
+
diff --git a/t/data/idp-metadata-signing-encryption.xml b/t/data/idp-metadata-signing-encryption.xml
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://accounts.google.com/o/saml2?idpid=C01nccos6" validUntil="2022-04-20T00:56:59.000Z">
+  <md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+    <md:KeyDescriptor use="signing">
+      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+        <ds:X509Data>
+          <ds:X509Certificate>MIIDdDCCAlygAwIBAgIGAVuOAyTCMA0GCSqGSIb3DQEBCwUAMHsxFDASBgNVBAoTC0dvb2dsZSBJ
+bmMuMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MQ8wDQYDVQQDEwZHb29nbGUxGDAWBgNVBAsTD0dv
+b2dsZSBGb3IgV29yazELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWEwHhcNMTcwNDIx
+MDA1NjU5WhcNMjIwNDIwMDA1NjU5WjB7MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEWMBQGA1UEBxMN
+TW91bnRhaW4gVmlldzEPMA0GA1UEAxMGR29vZ2xlMRgwFgYDVQQLEw9Hb29nbGUgRm9yIFdvcmsx
+CzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
+MIIBCgKCAQEAsziYPBZSqLzs5pFfoZPTVyoHwM4fmI9qQIScWJB0435ss5HDRDKBTvO8rg2sGbfM
+SYVINLvesTeTNkPkMfoSplne6uWjAWkoskvls+0bzp8jAXb+UJ1cZplQxhWW25TsEcpcMX+p4J0v
+/syO9lyDubNSpchPGIuzitKXdwFkgcOiCFk+01MF3COKaO6HoLr5Jqf7I6qjJy72h3ZC7rGUtwjC
+QPaAfeLIM2uGmdOcm7Ql/89hBSjJYvJOXOyEZqppYd2tat7UPh4q8UTmPhwETaVssThfNZUpt9iZ
+cCFcWaVh/mljxD/V20VNMAlzN8VXM/WjbBSAePS30VRF6xXlzwIDAQABMA0GCSqGSIb3DQEBCwUA
+A4IBAQAKxc4M+il/bQQcmm/JNKF5ypeC0YH+oRjSwec84pznP2qqjWosR2uR9hL6CN2dOZU/uCaX
+1nZviGIlsw9n7JUEfxMs4azYWfdTz7C7l9x6HpF/QK5NwBe1z3iPZAiKR8hBqt7IeN3nlT4RsaMr
+0E9UbG5pimxEG0uves5BoHS+3Xfb8sd33dNRyCm22nb2wWGG+jTKy2G8/24gHB2c0X6/AiD3dZbY
+qf7pFRWrXefkMaUMaCMnQh+owjjxVNEAAQ8QU/2Hwhz6pR0eEGL6UwWufJX8uSGaxqn+397MdrI4
+CE0gQAZXrB1L3PU9tdyiap2hWQPo8T2STpOtvcovFPHs</ds:X509Certificate>
+        </ds:X509Data>
+      </ds:KeyInfo>
+    </md:KeyDescriptor>
+    <md:KeyDescriptor use="encryption">
+      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+        <ds:X509Data>
+          <ds:X509Certificate>MIIDdDCCAlygAwIBAgIGAWj+rlteMA0GCSqGSIb3DQEBCwUAMHsxFDASBgNVBAoTC0dvb2dsZSBJ
+bmMuMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MQ8wDQYDVQQDEwZHb29nbGUxGDAWBgNVBAsTD0dv
+b2dsZSBGb3IgV29yazELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWEwHhcNMTkwMjE4
+MDMzNzQ1WhcNMjQwMjE3MDMzNzQ1WjB7MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEWMBQGA1UEBxMN
+TW91bnRhaW4gVmlldzEPMA0GA1UEAxMGR29vZ2xlMRgwFgYDVQQLEw9Hb29nbGUgRm9yIFdvcmsx
+CzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
+MIIBCgKCAQEAubE4/cl70Lc2f7VV+ZJyzYIzuAMj6ejlbtRnym2kgyYjaaO0MVU/r38oRC7UqdQI
+XwA0/S1Eu0k6EYcCUiyTGWz3HKv/OSOSnSDpN4wEWaZbmJYvu8SjWZQCVdcM4fx1kzrtE+LEBTOK
+gj0k2G1qUMNI7xaqiJONO9aIeCic5zbACNpc+IOZoRS4RaY5Ie7W1ZIXAJ0xWL3snVdqklaJzzU2
+Myt5QX6W1Sd411Hzo+Ihi5ksq18ML1tgMTFIqLkY2Luf5JJdZFRcCgvHrFF2CQywE0ftZtSSg5wD
++hp3PJpL5bxxF1vSdyKQbUia1Buc8+Cy6NTIbmLLIrcyULyHSwIDAQABMA0GCSqGSIb3DQEBCwUA
+A4IBAQAXKjWrRzhgUW1+pK3V51MGl2b/yf33Ac4fm7GQql0Ag0Neye1EmdLjD2N9gVeFawMcfRT4
+GABBHtS5bu01M8QHHAGjbBKfqOaPJc39v0Y/RCSd/FzXg99hNT5UggAWVR+vC6a1IrVSUa5eKe82
+yBAubbdftvGtKHG90HIAsb1iyMKK2rGnTupgJfJIUTWhWnWuemIVwduErFCxng//jYXViyEloz73
+0faMIp6eNSD2+2cCssVGFb6FxhCvVuNh6tgXv4vErVSWerFk/GcIh5n/biaDy/gEtAqgK154AfOi
+fpDP3l3ZV/celj1wSwcLF90e84XaVIkzb3veTcWhqaaq</ds:X509Certificate>
+        </ds:X509Data>
+      </ds:KeyInfo>
+    </md:KeyDescriptor>
+    <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://accounts.google.com/o/saml2/idp?idpid=C01nccos6"/>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://accounts.google.com/o/saml2/idp?idpid=C01nccos6"/>
+  </md:IDPSSODescriptor>
+</md:EntityDescriptor>
+