Fixes #12 - multiple signing keys in metadata

Author: timlegge

lib/Net/SAML2/Binding/Redirect.pm

@@ -93,11 +93,15 @@ that the encoding should be in the standard uppercase (%2F not %2f)
 Specifies that the IdP response sent to the HTTP-Redirect is double encoded.
 The double encoding requires it to be decoded prior to processing.
 
+=item B<debug>
+
+Output extra debugging information
+
 =back
 
 =cut
 
-has 'cert' => (isa => 'Str', is => 'ro', required => 1);
+has 'cert' => (isa => 'ArrayRef[Str]', is => 'ro', required => 0, predicate => 'has_cert');
 has 'url'  => (isa => Uri, is => 'ro', required => 0, coerce => 1, predicate => 'has_url');
 has 'key'  => (isa => 'Str', is => 'ro', required => 0, predicate => 'has_key');
 
@@ -129,6 +133,12 @@ has 'sls_double_encoded_response' => (
     default  => 0
 );
 
+has debug => (
+   is => 'ro',
+   isa => 'Bool',
+   required => 0,
+);
+
 =for Pod::Coverage BUILD
 
 =cut
@@ -140,9 +150,32 @@ sub BUILD {
         croak("Need to have an URL specified") unless $self->has_url;
         croak("Need to have a key specified") unless $self->has_key;
     }
+    if ($self->param eq 'SAMLResponse') {
+        croak("Need to have a cert specified") unless $self->has_cert;
+    }
     # other params don't need to have these per-se
 }
 
+# BUILDARGS
+
+# Earlier versions expected the cert to be a string.  However, metadata
+# can include multiple signing certificates so the $idp->cert is now
+# expected to be an arrayref to the certificates.  To avoid breaking existing
+# applications this changes the the cert to an arrayref if it is not
+# already an array ref.
+
+around BUILDARGS => sub {
+    my $orig = shift;
+    my $self = shift;
+
+    my %params = @_;
+    if ($params{cert} && ref($params{cert}) ne 'ARRAY') {
+            $params{cert} = [$params{cert}];
+    }
+
+    return $self->$orig(%params);
+};
+
 =head2 sign( $request, $relaystate )
 
 Signs the given request, and returns the URL to which the user's
@@ -185,6 +218,37 @@ sub sign {
     return $u->as_string;
 }
 
+sub _verified {
+    my ($self, $sigalg, $signed, $sig) = @_;
+
+    foreach my $crt (@{$self->cert}) {
+        my $cert = Crypt::OpenSSL::X509->new_from_string($crt);
+        my $rsa_pub = Crypt::OpenSSL::RSA->new_public_key($cert->pubkey);
+
+        if ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256') {
+            $rsa_pub->use_sha256_hash;
+        } elsif ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha224') {
+            $rsa_pub->use_sha224_hash;
+        } elsif ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384') {
+            $rsa_pub->use_sha384_hash;
+        } elsif ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512') {
+            $rsa_pub->use_sha512_hash;
+        } elsif ($sigalg eq 'http://www.w3.org/2000/09/xmldsig#rsa-sha1') {
+            $rsa_pub->use_sha1_hash;
+        } else {
+            warn "Unsupported Signature Algorithim: $sigalg" if ($self->debug);
+        }
+
+        if ($rsa_pub->verify($signed, $sig)) {
+            return 1;
+        }
+
+        warn "Unable to verify with " . $cert->subject if ($self->debug);
+    }
+
+    die "bad sig";
+}
+
 =head2 verify( $url )
 
 Decode a Redirect binding URL.
@@ -200,9 +264,6 @@ sub verify {
     # verify the response
     my $sigalg = $u->query_param('SigAlg');
 
-    my $cert = Crypt::OpenSSL::X509->new_from_string($self->cert);
-    my $rsa_pub = Crypt::OpenSSL::RSA->new_public_key($cert->pubkey);
-
     my $signed;
     my $saml_request;
     my $sig = $u->query_param_delete('Signature');
@@ -242,21 +303,7 @@ sub verify {
 
     $sig = decode_base64($sig);
 
-    if ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256') {
-        $rsa_pub->use_sha256_hash;
-    } elsif ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha224') {
-        $rsa_pub->use_sha224_hash;
-    } elsif ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384') {
-        $rsa_pub->use_sha384_hash;
-    } elsif ($sigalg eq 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512') {
-        $rsa_pub->use_sha512_hash;
-    } elsif ($sigalg eq 'http://www.w3.org/2000/09/xmldsig#rsa-sha1') {
-        $rsa_pub->use_sha1_hash;
-    } else {
-        die "Unsupported Signature Algorithim: $sigalg";
-    }
-
-    die "bad sig" unless $rsa_pub->verify($signed, $sig);
+    $self->_verified($sigalg, $signed, $sig);
 
     # unpack the SAML request
     my $deflated = decode_base64($saml_request);

lib/Net/SAML2/IdP.pm

@@ -66,7 +66,7 @@ has 'cacert'   => (isa => 'Maybe[Str]',   is => 'ro', required => 1);
 has 'sso_urls' => (isa => 'HashRef[Str]', is => 'ro', required => 1);
 has 'slo_urls' => (isa => 'Maybe[HashRef[Str]]', is => 'ro');
 has 'art_urls' => (isa => 'Maybe[HashRef[Str]]', is => 'ro');
-has 'certs'    => (isa => 'HashRef[Str]',        is => 'ro', required => 1);
+has 'certs'    => (isa => 'HashRef[ArrayRef[Str]]', is => 'ro', required => 1);
 has 'sls_force_lcase_url_encoding'    => (isa => 'Bool', is => 'ro', required => 0);
 has 'sls_double_encoded_response' => (isa => 'Bool', is => 'ro', required => 0);
 
@@ -180,10 +180,15 @@ sub new_from_xml {
         }
     }
 
+    my @certs = ();
+
     for my $key (
         $xpath->findnodes('//md:EntityDescriptor/md:IDPSSODescriptor/md:KeyDescriptor'))
     {
-        my $use = $key->getAttribute('use') || 'signing';
+        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');
 
@@ -204,17 +209,20 @@ sub new_from_xml {
         $text = join "\n", @lines;
 
         # form a PEM certificate
-        $data->{Cert}->{$use}
-            = sprintf("-----BEGIN CERTIFICATE-----\n%s\n-----END CERTIFICATE-----\n",
-            $text);
+        for my $use (@uses) {
+            my $pem->{$use}
+                = sprintf("-----BEGIN CERTIFICATE-----\n%s\n-----END CERTIFICATE-----\n",
+                $text);
+            push (@certs, $pem);
+        }
     }
 
     my $self = $class->new(
         entityid => $xpath->findvalue('//md:EntityDescriptor/@entityID'),
         sso_urls => $data->{SSO},
         slo_urls => $data->{SLO} || {},
         art_urls => $data->{Art} || {},
-        certs                        => $data->{Cert},
+        certs                        => \@certs,
         cacert                       => $args{cacert},
         sls_force_lcase_url_encoding => $args{sls_force_lcase_url_encoding},
         sls_double_encoded_response  => $args{sls_double_encoded_response},
@@ -229,28 +237,50 @@ sub new_from_xml {
     return $self;
 }
 
-=head2 BUILD ( hashref of the parameters passed to the constructor )
-
-Called after the object is created to validate the IdP using the cacert
-
-=cut
-
-sub BUILD {
-    my($self) = @_;
+# BUILDARGS ( hashref of the parameters passed to the constructor )
+#
+# Called after the object is created to validate the IdP using the cacert
+#
+
+around BUILDARGS => sub {
+    my $orig = shift;
+    my $self = shift;
+
+    my %params = @_;
+
+    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});
+                ## BUGBUG this is failing for valid things ...
+                eval { $ca->verify($cert) };
+                if ($@) {
+                    $errors{$cert->fingerprint_sha256} = $@;
+                    next;
+                }
+                $verified = 1;
+                push @tmpcrt, $pem->{$use};
+
+                $certs{$use} = \@tmpcrt;
+            }
+        }
 
-    if ($self->cacert) {
-        my $ca = Crypt::OpenSSL::Verify->new($self->cacert, { strict_certs => 0, });
+        $params{certs} = \%certs;
 
-        for my $use (keys %{$self->certs}) {
-            my $cert = Crypt::OpenSSL::X509->new_from_string($self->certs->{$use});
-            ## BUGBUG this is failing for valid things ...
-            eval { $ca->verify($cert) };
-            if ($@) {
-                warn "Can't verify IdP '$use' cert: $@\n";
-            }
+        if (!$verified) {
+             warn "Can't verify IdP signing cert: ", %errors, "\n";
         }
     }
-}
+
+    return $self->$orig(%params);
+};
 
 =head2 sso_url( $binding )
 
@@ -290,7 +320,13 @@ sub art_url {
 
 =head2 cert( $use )
 
-Returns the IdP's certificate for the given use (e.g. C<signing>).
+Returns the IdP's certificates for the given use (e.g. C<signing>).
+
+IdP's are generated from the metadata it is possible for multiple certificates
+to be contained in the metadata and therefore possible for them to be there to
+be multiple verified certs in $self->certs.  At this point any certs in the IdP
+have been verified and are valid for the specified use.  All certs are of type
+$use are returned.
 
 =cut
 
@@ -310,6 +346,7 @@ sub binding {
     my($self, $name) = @_;
 
     my $bindings = {
+        post     => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
         redirect => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
         soap     => 'urn:oasis:names:tc:SAML:2.0:bindings:SOAP',
     };

t/01-create-idp.t

@@ -95,7 +95,11 @@ is(
     'Has correct art_url'
 );
 
-looks_like_a_cert($idp->cert('signing'));
+foreach my $use (keys %{$idp->certs}) {
+    for my $cert (@{$idp->cert($use)}) {
+        looks_like_a_cert($cert);
+    }
+};
 
 is(
     $idp->entityid,
@@ -152,7 +156,11 @@ is(
         'Has correct art_url'
     );
 
-    looks_like_a_cert($idp->cert('signing'), 'Looks like signing certificate');
+    foreach my $use (keys %{$idp->certs}) {
+        for my $cert (@{$idp->cert($use)}) {
+            looks_like_a_cert($cert);
+        }
+    };
 
     is(
         $idp->entityid,
@@ -275,7 +283,11 @@ XML
         'Has correct art_url'
     );
 
-    looks_like_a_cert($idp->cert('signing'), 'Looks like signing certificate');
+    foreach my $use (keys %{$idp->certs}) {
+        for my $cert (@{$idp->cert($use)}) {
+            looks_like_a_cert($cert);
+        }
+    };
 
     is(
         $idp->entityid,

t/05-soap-binding.t

@@ -24,8 +24,13 @@ is(
     'SLO url is correct'
 );
 
-my $idp_cert = $idp->cert('signing');
-looks_like_a_cert($idp_cert);
+my $idp_cert;
+foreach my $use (keys %{$idp->certs}) {
+    for my $cert (@{$idp->cert($use)}) {
+        $idp_cert = $cert;
+        looks_like_a_cert($cert);
+    }
+};
 
 my $nameid  = 'user-to-log-out';
 my $session = 'session-to-log-out';

t/06-redirect-binding.t

@@ -14,7 +14,6 @@ my $idp = Net::SAML2::IdP->new_from_xml(
     xml    => $metadata,
     cacert => 't/cacert.pem'
 );
-
 isa_ok($idp, "Net::SAML2::IdP");
 
 my $sso_url = $idp->sso_url($idp->binding('redirect'));
@@ -55,18 +54,30 @@ is($relaystate, 'http://return/url', "Relay state shows correct uri");
 lives_ok(
     sub {
         my $binding = Net::SAML2::Binding::Redirect->new(
-            cert  => $sp->cert,
+            cert  => $idp->cert('signing'),
             param => 'SAMLResponse',
         );
         isa_ok($binding, "Net::SAML2::Binding::Redirect");
     },
     "We can create a binding redirect without key/url for verification purposes"
 );
 
+lives_ok(
+    sub {
+        my $binding = Net::SAML2::Binding::Redirect->new(
+            param => 'SAMLRequest',
+            url   => 'https://foo.example.com',
+            key   => $sp->key,
+        );
+        isa_ok($binding, "Net::SAML2::Binding::Redirect");
+    },
+    "We do not need a cert to sign a SAMLRequest"
+);
+
 throws_ok(
     sub {
         Net::SAML2::Binding::Redirect->new(
-            cert  => $sp->cert,
+            cert  => $idp->cert('signing'),
             key   => $sp->key,
         );
     },
@@ -77,7 +88,6 @@ throws_ok(
 throws_ok(
     sub {
         Net::SAML2::Binding::Redirect->new(
-            cert  => $sp->cert,
             url   => 'https://foo.example.com',
         );
     },

t/11-more-metadata.t

@@ -30,7 +30,12 @@ is(
     'Found SSO artifact binding'
 );
 
-looks_like_a_cert($idp->cert('signing'));
+foreach my $use (keys %{$idp->certs}) {
+    for my $cert (@{$idp->cert($use)}) {
+        looks_like_a_cert($cert);
+    }
+};
+
 
 is(
     $idp->entityid,