From a642722e9b65dbc8e56a7255c5105f28517517e3 Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 14:32:45 +0300 Subject: [PATCH 01/13] Fix for Assertion Attribute Values. The XML Oskari used to give an empty Attribute Value, this fixes that. --- lib/Net/SAML2/Protocol/Assertion.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Net/SAML2/Protocol/Assertion.pm b/lib/Net/SAML2/Protocol/Assertion.pm index fb2d9e4..11508f8 100644 --- a/lib/Net/SAML2/Protocol/Assertion.pm +++ b/lib/Net/SAML2/Protocol/Assertion.pm @@ -46,7 +46,7 @@ sub new_from_xml { my $attributes = {}; for my $node ($xpath->findnodes('//saml:Assertion/saml:AttributeStatement/saml:Attribute')) { - my @values = $node->findnodes('saml:AttributeValue'); + my @values = $xpath->findnodes('saml:AttributeValue', $node); $attributes->{$node->getAttribute('Name')} = [ map { $_->string_value } @values ]; From a76ab9c41dffbbe398b4d89a0238c069d4253e03 Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 14:49:12 +0300 Subject: [PATCH 02/13] Added default for the nameid_format (from Net::SAML2 v0.15). Added providername as a configurable thing. Added nameid_format into SYNOPSIS. --- lib/Net/SAML2/Protocol/AuthnRequest.pm | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/Net/SAML2/Protocol/AuthnRequest.pm b/lib/Net/SAML2/Protocol/AuthnRequest.pm index d496e58..f8533e5 100644 --- a/lib/Net/SAML2/Protocol/AuthnRequest.pm +++ b/lib/Net/SAML2/Protocol/AuthnRequest.pm @@ -16,6 +16,7 @@ Net::SAML2::Protocol::AuthnRequest - SAML2 AuthnRequest object issueinstant => DateTime->now, issuer => $self->{id}, destination => $destination, + nameid_format => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', # or 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' ); =head1 METHODS @@ -35,7 +36,8 @@ Arguments: has 'issuer' => (isa => Uri, is => 'ro', required => 1, coerce => 1); has 'destination' => (isa => Uri, is => 'ro', required => 1, coerce => 1); -has 'nameid_format' => (isa => NonEmptySimpleStr, is => 'ro', required => 1); +has 'nameid_format' => (isa => NonEmptySimpleStr, is => 'ro', required => 1, default => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'); +has 'providername' => (is => 'rw', required => 0, default => 'ProviderName'); =head2 as_xml() @@ -56,8 +58,13 @@ sub as_xml { { Destination => $self->destination, ID => $self->id, IssueInstant => $self->issue_instant, - ProviderName => "My SP's human readable name.", - Version => '2.0' }, + ProviderName => $self->providername(), + Version => '2.0', +# AssertionConsumerServiceURL => 'http://localhost/saml/consumer_post', +# IsPassive => 'false', +# ProtocolBinding => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST', +# ForceAuthn => "true", + }, $x->Issuer( $saml, $self->issuer, @@ -66,9 +73,9 @@ sub as_xml { $samlp, { AllowCreate => '1', Format => $self->nameid_format }, - ) + ), ) ); } -__PACKAGE__->meta->make_immutable; +1; From 1a31469ca8d5b21687f4dc07877db516b1af1cda Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 14:52:06 +0300 Subject: [PATCH 03/13] Default to KeyDescriptor use="signing" if attribute use is not present. For Shibboleth 2.x compatibility. --- lib/Net/SAML2/IdP.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Net/SAML2/IdP.pm b/lib/Net/SAML2/IdP.pm index 627cdf1..28b0913 100644 --- a/lib/Net/SAML2/IdP.pm +++ b/lib/Net/SAML2/IdP.pm @@ -101,7 +101,7 @@ sub new_from_xml { } for my $key ($xpath->findnodes('//md:EntityDescriptor/md:IDPSSODescriptor/md:KeyDescriptor')) { - my $use = $key->getAttribute('use'); + my $use = $key->getAttribute('use') || 'signing'; my ($text) = $key->findvalue('ds:KeyInfo/ds:X509Data/ds:X509Certificate') =~ /^\s*(.+?)\s*$/s; # rewrap the base64 data from the metadata; it may not From d57c755836b74ea3e8a9e9e6edf1ecaa1aa5d477 Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 14:54:10 +0300 Subject: [PATCH 04/13] Improved Synopsis, https://rt.cpan.org/Public/Bug/Display.html?id=71183 --- lib/Net/SAML2/Binding/Redirect.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Net/SAML2/Binding/Redirect.pm b/lib/Net/SAML2/Binding/Redirect.pm index 42d7c9b..07a66fb 100644 --- a/lib/Net/SAML2/Binding/Redirect.pm +++ b/lib/Net/SAML2/Binding/Redirect.pm @@ -13,6 +13,7 @@ Net::SAML2::Binding::Redirect key => 'sign-nopw-cert.pem', url => $sso_url, param => 'SAMLRequest', + cert => 'path/to/cert.ca', ); my $url = $redirect->sign($authnreq); From 698359eb2e7c86dfaca97c81c16f7dae278ef671 Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 14:55:28 +0300 Subject: [PATCH 05/13] Improved Synopsis, https://rt.cpan.org/Public/Bug/Display.html?id=71163 --- lib/Net/SAML2.pm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Net/SAML2.pm b/lib/Net/SAML2.pm index 81cd415..9c838e7 100644 --- a/lib/Net/SAML2.pm +++ b/lib/Net/SAML2.pm @@ -25,8 +25,9 @@ Net::SAML2 - SAML bindings and protocol implementation )->as_xml; my $redirect = Net::SAML2::Binding::Redirect->new( - key => 'sign-nopw-cert.pem', - url => $sso_url, + key => 'sign-nopw-cert.pem', + url => $sso_url, + param => 'SAMLRequest', ); my $url = $redirect->sign($authnreq); From 7b6e67bd775ad3e2f1e5d477b1f1f10ab16a5e8a Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 14:56:27 +0300 Subject: [PATCH 06/13] Accept an IdP without a Logout URL, https://rt.cpan.org/Public/Bug/Display.html?id=71161 --- lib/Net/SAML2/IdP.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Net/SAML2/IdP.pm b/lib/Net/SAML2/IdP.pm index 28b0913..6049b8d 100644 --- a/lib/Net/SAML2/IdP.pm +++ b/lib/Net/SAML2/IdP.pm @@ -33,7 +33,7 @@ Constructor has 'entityid' => (isa => Str, is => 'ro', required => 1); has 'cacert' => (isa => Str, is => 'ro', required => 1); has 'sso_urls' => (isa => HashRef[Str], is => 'ro', required => 1); -has 'slo_urls' => (isa => HashRef[Str], is => 'ro', required => 1); +has 'slo_urls' => (isa => 'HashRef[Str]|Undef', is => 'ro', required => 0); has 'art_urls' => (isa => HashRef[Str], is => 'ro', required => 1); has 'certs' => (isa => HashRef[Str], is => 'ro', required => 1); has 'formats' => (isa => HashRef[Str], is => 'ro', required => 1); From 1825df3d03c9adb557d718e750a2c35055bae7cc Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 15:18:27 +0300 Subject: [PATCH 07/13] Properly handle an XML document that has been signed and has another XML tree inside it that has also been signed. TODO: Should remove the outermost Signature only, now it removes the first it finds in the byte stream (with a regexp). TODO: Should not need the Response namespace for matching to make XML::Sig general again. --- lib/Net/SAML2/XML/Sig.pm | 73 +++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/lib/Net/SAML2/XML/Sig.pm b/lib/Net/SAML2/XML/Sig.pm index 036f4d9..0fca5b3 100644 --- a/lib/Net/SAML2/XML/Sig.pm +++ b/lib/Net/SAML2/XML/Sig.pm @@ -114,11 +114,17 @@ sub verify { $self->{ parser } = XML::XPath->new( xml => $xml ); $self->{ parser }->set_namespace('dsig', 'http://www.w3.org/2000/09/xmldsig#'); + # TODO: Find a more elegant way to match /saml2p:Response/dsig:Signature + $self->{ parser }->set_namespace('saml2p', 'urn:oasis:names:tc:SAML:2.0:protocol'); - my $signature = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:SignatureValue')); - my $signed_info_node = $self->_get_node('//dsig:Signature/dsig:SignedInfo'); + my $signature_nodeset = $self->{parser}->findnodes('/saml2p:Response/dsig:Signature'); + my $signature_node = $signature_nodeset->shift(); + + my $value = $self->{parser}->findvalue('dsig:SignatureValue', $signature_node); + + my $signature = _trim($self->{parser}->findvalue('dsig:SignatureValue', $signature_node)); + my $signed_info_node = $self->_get_node('dsig:SignedInfo', $signature_node); - my $signature_node = $self->_get_node('//dsig:Signature'); my $ns; if (defined $signature_node && ref $signature_node) { $ns = $signature_node->getNamespaces->[0]; @@ -141,14 +147,17 @@ sub verify { } else { # extract the certficate or key from the document - my $keyinfo_node; - if ($keyinfo_node = $self->{parser}->find('//dsig:Signature/dsig:KeyInfo/dsig:X509Data')) { - return 0 unless $self->_verify_x509($keyinfo_node,$signed_info_canon,$signature); - } - elsif ($keyinfo_node = $self->{parser}->find('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue')) { + my $keyinfo_nodeset; + if ($keyinfo_nodeset = $self->{parser}->findnodes('dsig:KeyInfo/dsig:X509Data', $signature_node)) { + my $keyinfo_node = $keyinfo_nodeset->shift(); + return 0 unless $self->_verify_x509($keyinfo_node,$signed_info_canon,$signature); + } + elsif ($keyinfo_nodeset = $self->{parser}->find('dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue', $signature_node)) { + my $keyinfo_node = $keyinfo_nodeset->shift(); return 0 unless $self->_verify_rsa($keyinfo_node,$signed_info_canon,$signature); } - elsif ($keyinfo_node = $self->{parser}->find('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue')) { + elsif ($keyinfo_nodeset = $self->{parser}->find('dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue', $signature_node)) { + my $keyinfo_node = $keyinfo_nodeset->shift(); return 0 unless $self->_verify_dsa($keyinfo_node,$signed_info_canon,$signature); } else { @@ -156,11 +165,13 @@ sub verify { } } - my $digest_method = $self->{parser}->findvalue('//dsig:Signature/dsig:SignedInfo/dsig:Reference/dsig:DigestMethod/@Algorithm'); - my $digest = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:SignedInfo/dsig:Reference/dsig:DigestValue')); + my $digest_method = $self->{parser}->findvalue('dsig:Reference/dsig:DigestMethod/@Algorithm', $signed_info_node); + my $digest = _trim($self->{parser}->findvalue('dsig:Reference/dsig:DigestValue', $signed_info_node)); - my $signed_xml = $self->_get_signed_xml(); - my $canonical = $self->_transform( $signed_xml ); + my $signed_xml = $self->_get_signed_xml( $signature_node ); + + my $canonical = $self->_transform( $signed_xml, $signature_node ); + my $digest_bin = sha1( $canonical ); return 1 if ($digest eq _trim(encode_base64($digest_bin))); @@ -183,7 +194,8 @@ sub _get_xml_to_sign { sub _get_signed_xml { my $self = shift; - my $id = $self->{parser}->findvalue('//dsig:Signature/dsig:SignedInfo/dsig:Reference/@URI'); + my ($context) = @_; + my $id = $self->{parser}->findvalue('dsig:SignedInfo/dsig:Reference/@URI', $context); $id =~ s/^#//; $self->{'sign_id'} = $id; my $xpath = "//*[\@ID='$id']"; @@ -192,8 +204,8 @@ sub _get_signed_xml { sub _transform { my $self = shift; - my ($xml) = @_; - foreach my $node ($self->{parser}->find('//dsig:Transform/@Algorithm')->get_nodelist) { + my ($xml, $transform_algorithm_context) = @_; + foreach my $node ($self->{parser}->find('dsig:SignedInfo/dsig:Reference/dsig:Transforms/dsig:Transform/@Algorithm', $transform_algorithm_context)->get_nodelist) { my $alg = $node->getNodeValue; if ($alg eq TRANSFORM_ENV_SIG) { $xml = $self->_transform_env_sig($xml); } elsif ($alg eq TRANSFORM_EXC_C14N) { $xml = $self->_canonicalize_xml($xml,0); } @@ -208,9 +220,9 @@ sub _verify_rsa { my ($context,$canonical,$sig) = @_; # Generate Public Key from XML - my $mod = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue/dsig:Modulus')); + my $mod = _trim($self->{parser}->findvalue('dsig:Modulus', $context)); my $modBin = decode_base64( $mod ); - my $exp = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue/dsig:Exponent')); + my $exp = _trim($self->{parser}->findvalue('dsig:Exponent', $context)); my $expBin = decode_base64( $exp ); my $n = Crypt::OpenSSL::Bignum->new_from_bin($modBin); my $e = Crypt::OpenSSL::Bignum->new_from_bin($expBin); @@ -239,12 +251,12 @@ sub _verify_x509 { confess "Crypt::OpenSSL::X509 needs to be installed so that we can handle X509 certificates" if $@; # Generate Public Key from XML - my $certificate = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:X509Data/dsig:X509Certificate')); + my $certificate = _trim($self->{parser}->findvalue('dsig:X509Certificate', $context)); # This is added because the X509 parser requires it for self-identification $certificate = $self->_clean_x509($certificate); - my $cert = Crypt::OpenSSL::X509->new_from_string($certificate); + return $self->_verify_x509_cert($cert, $canonical, $sig); } @@ -279,10 +291,10 @@ sub _verify_dsa { }; # Generate Public Key from XML - my $p = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:P'))); - my $q = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:Q'))); - my $g = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:G'))); - my $y = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:Y'))); + my $p = decode_base64(_trim($self->{parser}->findvalue('dsig:P', $context))); + my $q = decode_base64(_trim($self->{parser}->findvalue('dsig:Q', $context))); + my $g = decode_base64(_trim($self->{parser}->findvalue('dsig:G', $context))); + my $y = decode_base64(_trim($self->{parser}->findvalue('dsig:Y', $context))); my $dsa_pub = Crypt::OpenSSL::DSA->new(); $dsa_pub->set_p($p); $dsa_pub->set_q($q); @@ -298,8 +310,13 @@ sub _verify_dsa { sub _get_node { my $self = shift; - my ($xpath) = @_; - my $nodeset = $self->{parser}->find($xpath); + my ($xpath, $context) = @_; + my $nodeset; + if ($context) { + $nodeset = $self->{parser}->find($xpath, $context); + } else { + $nodeset = $self->{parser}->find($xpath); + } foreach my $node ($nodeset->get_nodelist) { return $node; } @@ -317,7 +334,9 @@ sub _transform_env_sig { if (defined $self->{dsig_prefix} && length $self->{dsig_prefix}) { $prefix = $self->{dsig_prefix} . ':'; } - $str =~ s/(<${prefix}Signature(.*?)>(.*?)\<\/${prefix}Signature>)//igs; + # This removes the first Signature tag from the XML - even if there is another XML tree with another Signature inside and that comes first. + # TODO: Remove the outermost Signature only. + $str =~ s/(<${prefix}Signature(.*?)>(.*?)\<\/${prefix}Signature>)//is; return $str; } From 35506510c5e2a7802265a4cd6db347c4b2dc258b Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Thu, 29 Sep 2011 15:18:27 +0300 Subject: [PATCH 08/13] Properly handle an XML document that has been signed and has another XML tree inside it that has also been signed. TODO: Should remove the outermost Signature only, now it removes the first it finds in the byte stream (with a regexp). TODO: Should not need the Response namespace for matching to make XML::Sig general again. --- lib/Net/SAML2/XML/Sig.pm | 73 +++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/lib/Net/SAML2/XML/Sig.pm b/lib/Net/SAML2/XML/Sig.pm index 036f4d9..0fca5b3 100644 --- a/lib/Net/SAML2/XML/Sig.pm +++ b/lib/Net/SAML2/XML/Sig.pm @@ -114,11 +114,17 @@ sub verify { $self->{ parser } = XML::XPath->new( xml => $xml ); $self->{ parser }->set_namespace('dsig', 'http://www.w3.org/2000/09/xmldsig#'); + # TODO: Find a more elegant way to match /saml2p:Response/dsig:Signature + $self->{ parser }->set_namespace('saml2p', 'urn:oasis:names:tc:SAML:2.0:protocol'); - my $signature = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:SignatureValue')); - my $signed_info_node = $self->_get_node('//dsig:Signature/dsig:SignedInfo'); + my $signature_nodeset = $self->{parser}->findnodes('/saml2p:Response/dsig:Signature'); + my $signature_node = $signature_nodeset->shift(); + + my $value = $self->{parser}->findvalue('dsig:SignatureValue', $signature_node); + + my $signature = _trim($self->{parser}->findvalue('dsig:SignatureValue', $signature_node)); + my $signed_info_node = $self->_get_node('dsig:SignedInfo', $signature_node); - my $signature_node = $self->_get_node('//dsig:Signature'); my $ns; if (defined $signature_node && ref $signature_node) { $ns = $signature_node->getNamespaces->[0]; @@ -141,14 +147,17 @@ sub verify { } else { # extract the certficate or key from the document - my $keyinfo_node; - if ($keyinfo_node = $self->{parser}->find('//dsig:Signature/dsig:KeyInfo/dsig:X509Data')) { - return 0 unless $self->_verify_x509($keyinfo_node,$signed_info_canon,$signature); - } - elsif ($keyinfo_node = $self->{parser}->find('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue')) { + my $keyinfo_nodeset; + if ($keyinfo_nodeset = $self->{parser}->findnodes('dsig:KeyInfo/dsig:X509Data', $signature_node)) { + my $keyinfo_node = $keyinfo_nodeset->shift(); + return 0 unless $self->_verify_x509($keyinfo_node,$signed_info_canon,$signature); + } + elsif ($keyinfo_nodeset = $self->{parser}->find('dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue', $signature_node)) { + my $keyinfo_node = $keyinfo_nodeset->shift(); return 0 unless $self->_verify_rsa($keyinfo_node,$signed_info_canon,$signature); } - elsif ($keyinfo_node = $self->{parser}->find('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue')) { + elsif ($keyinfo_nodeset = $self->{parser}->find('dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue', $signature_node)) { + my $keyinfo_node = $keyinfo_nodeset->shift(); return 0 unless $self->_verify_dsa($keyinfo_node,$signed_info_canon,$signature); } else { @@ -156,11 +165,13 @@ sub verify { } } - my $digest_method = $self->{parser}->findvalue('//dsig:Signature/dsig:SignedInfo/dsig:Reference/dsig:DigestMethod/@Algorithm'); - my $digest = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:SignedInfo/dsig:Reference/dsig:DigestValue')); + my $digest_method = $self->{parser}->findvalue('dsig:Reference/dsig:DigestMethod/@Algorithm', $signed_info_node); + my $digest = _trim($self->{parser}->findvalue('dsig:Reference/dsig:DigestValue', $signed_info_node)); - my $signed_xml = $self->_get_signed_xml(); - my $canonical = $self->_transform( $signed_xml ); + my $signed_xml = $self->_get_signed_xml( $signature_node ); + + my $canonical = $self->_transform( $signed_xml, $signature_node ); + my $digest_bin = sha1( $canonical ); return 1 if ($digest eq _trim(encode_base64($digest_bin))); @@ -183,7 +194,8 @@ sub _get_xml_to_sign { sub _get_signed_xml { my $self = shift; - my $id = $self->{parser}->findvalue('//dsig:Signature/dsig:SignedInfo/dsig:Reference/@URI'); + my ($context) = @_; + my $id = $self->{parser}->findvalue('dsig:SignedInfo/dsig:Reference/@URI', $context); $id =~ s/^#//; $self->{'sign_id'} = $id; my $xpath = "//*[\@ID='$id']"; @@ -192,8 +204,8 @@ sub _get_signed_xml { sub _transform { my $self = shift; - my ($xml) = @_; - foreach my $node ($self->{parser}->find('//dsig:Transform/@Algorithm')->get_nodelist) { + my ($xml, $transform_algorithm_context) = @_; + foreach my $node ($self->{parser}->find('dsig:SignedInfo/dsig:Reference/dsig:Transforms/dsig:Transform/@Algorithm', $transform_algorithm_context)->get_nodelist) { my $alg = $node->getNodeValue; if ($alg eq TRANSFORM_ENV_SIG) { $xml = $self->_transform_env_sig($xml); } elsif ($alg eq TRANSFORM_EXC_C14N) { $xml = $self->_canonicalize_xml($xml,0); } @@ -208,9 +220,9 @@ sub _verify_rsa { my ($context,$canonical,$sig) = @_; # Generate Public Key from XML - my $mod = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue/dsig:Modulus')); + my $mod = _trim($self->{parser}->findvalue('dsig:Modulus', $context)); my $modBin = decode_base64( $mod ); - my $exp = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue/dsig:Exponent')); + my $exp = _trim($self->{parser}->findvalue('dsig:Exponent', $context)); my $expBin = decode_base64( $exp ); my $n = Crypt::OpenSSL::Bignum->new_from_bin($modBin); my $e = Crypt::OpenSSL::Bignum->new_from_bin($expBin); @@ -239,12 +251,12 @@ sub _verify_x509 { confess "Crypt::OpenSSL::X509 needs to be installed so that we can handle X509 certificates" if $@; # Generate Public Key from XML - my $certificate = _trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:X509Data/dsig:X509Certificate')); + my $certificate = _trim($self->{parser}->findvalue('dsig:X509Certificate', $context)); # This is added because the X509 parser requires it for self-identification $certificate = $self->_clean_x509($certificate); - my $cert = Crypt::OpenSSL::X509->new_from_string($certificate); + return $self->_verify_x509_cert($cert, $canonical, $sig); } @@ -279,10 +291,10 @@ sub _verify_dsa { }; # Generate Public Key from XML - my $p = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:P'))); - my $q = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:Q'))); - my $g = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:G'))); - my $y = decode_base64(_trim($self->{parser}->findvalue('//dsig:Signature/dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue/dsig:Y'))); + my $p = decode_base64(_trim($self->{parser}->findvalue('dsig:P', $context))); + my $q = decode_base64(_trim($self->{parser}->findvalue('dsig:Q', $context))); + my $g = decode_base64(_trim($self->{parser}->findvalue('dsig:G', $context))); + my $y = decode_base64(_trim($self->{parser}->findvalue('dsig:Y', $context))); my $dsa_pub = Crypt::OpenSSL::DSA->new(); $dsa_pub->set_p($p); $dsa_pub->set_q($q); @@ -298,8 +310,13 @@ sub _verify_dsa { sub _get_node { my $self = shift; - my ($xpath) = @_; - my $nodeset = $self->{parser}->find($xpath); + my ($xpath, $context) = @_; + my $nodeset; + if ($context) { + $nodeset = $self->{parser}->find($xpath, $context); + } else { + $nodeset = $self->{parser}->find($xpath); + } foreach my $node ($nodeset->get_nodelist) { return $node; } @@ -317,7 +334,9 @@ sub _transform_env_sig { if (defined $self->{dsig_prefix} && length $self->{dsig_prefix}) { $prefix = $self->{dsig_prefix} . ':'; } - $str =~ s/(<${prefix}Signature(.*?)>(.*?)\<\/${prefix}Signature>)//igs; + # This removes the first Signature tag from the XML - even if there is another XML tree with another Signature inside and that comes first. + # TODO: Remove the outermost Signature only. + $str =~ s/(<${prefix}Signature(.*?)>(.*?)\<\/${prefix}Signature>)//is; return $str; } From c81f7571574ebe951ec25db25f29958d3207175b Mon Sep 17 00:00:00 2001 From: Chris Andrews Date: Wed, 12 Oct 2011 22:02:07 +0100 Subject: [PATCH 09/13] Switch saml2p prefix to samlp as used elsewhere. --- lib/Net/SAML2/XML/Sig.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Net/SAML2/XML/Sig.pm b/lib/Net/SAML2/XML/Sig.pm index 0fca5b3..7ece9c1 100644 --- a/lib/Net/SAML2/XML/Sig.pm +++ b/lib/Net/SAML2/XML/Sig.pm @@ -114,10 +114,10 @@ sub verify { $self->{ parser } = XML::XPath->new( xml => $xml ); $self->{ parser }->set_namespace('dsig', 'http://www.w3.org/2000/09/xmldsig#'); - # TODO: Find a more elegant way to match /saml2p:Response/dsig:Signature - $self->{ parser }->set_namespace('saml2p', 'urn:oasis:names:tc:SAML:2.0:protocol'); + # TODO: Find a more elegant way to match /samlp:Response/dsig:Signature + $self->{ parser }->set_namespace('samlp', 'urn:oasis:names:tc:SAML:2.0:protocol'); - my $signature_nodeset = $self->{parser}->findnodes('/saml2p:Response/dsig:Signature'); + my $signature_nodeset = $self->{parser}->findnodes('/samlp:Response/dsig:Signature'); my $signature_node = $signature_nodeset->shift(); my $value = $self->{parser}->findvalue('dsig:SignatureValue', $signature_node); From 15cb6479f0f61c726e07cac3bf87f9d68554f36f Mon Sep 17 00:00:00 2001 From: Chris Andrews Date: Fri, 14 Oct 2011 19:18:23 +0100 Subject: [PATCH 10/13] Remove references to saml elements and namespaces. --- lib/Net/SAML2/XML/Sig.pm | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/Net/SAML2/XML/Sig.pm b/lib/Net/SAML2/XML/Sig.pm index 7ece9c1..f6b99e3 100644 --- a/lib/Net/SAML2/XML/Sig.pm +++ b/lib/Net/SAML2/XML/Sig.pm @@ -114,11 +114,9 @@ sub verify { $self->{ parser } = XML::XPath->new( xml => $xml ); $self->{ parser }->set_namespace('dsig', 'http://www.w3.org/2000/09/xmldsig#'); - # TODO: Find a more elegant way to match /samlp:Response/dsig:Signature - $self->{ parser }->set_namespace('samlp', 'urn:oasis:names:tc:SAML:2.0:protocol'); + $self->{ parser }->set_namespace('ec', 'http://www.w3.org/2001/10/xml-exc-c14n#'); - my $signature_nodeset = $self->{parser}->findnodes('/samlp:Response/dsig:Signature'); - my $signature_node = $signature_nodeset->shift(); + my $signature_nodeset = $self->{parser}->findnodes('//dsig:Signature'); my $value = $self->{parser}->findvalue('dsig:SignatureValue', $signature_node); @@ -556,7 +554,7 @@ sub _signedinfo_xml { my $self = shift; my ($digest_xml) = @_; - return qq{ + return qq{ $digest_xml From e1150cd96c574a620d83af2bc90e13ec566dbaee Mon Sep 17 00:00:00 2001 From: Chris Andrews Date: Fri, 14 Oct 2011 19:23:44 +0100 Subject: [PATCH 11/13] Support InclusiveNamespaces element on canonicalization transform. Only supported when XML::CanonicalizeXML is used. --- lib/Net/SAML2/XML/Sig.pm | 50 ++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/lib/Net/SAML2/XML/Sig.pm b/lib/Net/SAML2/XML/Sig.pm index f6b99e3..3f59c91 100644 --- a/lib/Net/SAML2/XML/Sig.pm +++ b/lib/Net/SAML2/XML/Sig.pm @@ -202,17 +202,41 @@ sub _get_signed_xml { sub _transform { my $self = shift; - my ($xml, $transform_algorithm_context) = @_; - foreach my $node ($self->{parser}->find('dsig:SignedInfo/dsig:Reference/dsig:Transforms/dsig:Transform/@Algorithm', $transform_algorithm_context)->get_nodelist) { - my $alg = $node->getNodeValue; - if ($alg eq TRANSFORM_ENV_SIG) { $xml = $self->_transform_env_sig($xml); } - elsif ($alg eq TRANSFORM_EXC_C14N) { $xml = $self->_canonicalize_xml($xml,0); } - elsif ($alg eq TRANSFORM_EXC_C14N_COMMENTS) { $xml = $self->_canonicalize_xml($xml,1); } - else { die "Unsupported transform: $alg"; } + my ($xml, $context) = @_; + + my $transforms = $self->{parser}->find( + 'dsig:SignedInfo/dsig:Reference/dsig:Transforms/dsig:Transform', + $context + ); + + foreach my $node ($transforms->get_nodelist) { + my $alg = $node->getAttribute('Algorithm'); + + if ($alg eq TRANSFORM_ENV_SIG) { + $xml = $self->_transform_env_sig($xml); + } + elsif ($alg eq TRANSFORM_EXC_C14N) { + my $prefixlist = $self->_find_prefixlist($node); + $xml = $self->_canonicalize_xml($xml, 0, $prefixlist); + } + elsif ($alg eq TRANSFORM_EXC_C14N_COMMENTS) { + my $prefixlist = $self->_find_prefixlist($node); + $xml = $self->_canonicalize_xml($xml, 1, $prefixlist); + } + else { + die "Unsupported transform: $alg"; + } } return $xml; } +sub _find_prefixlist { + my $self = shift; + my ($node) = @_; + my $prefixlist = $self->{parser}->findvalue('ec:InclusiveNamespaces/@PrefixList', $node); + return $prefixlist; +} + sub _verify_rsa { my $self = shift; my ($context,$canonical,$sig) = @_; @@ -577,18 +601,26 @@ sub _reference_xml { sub _canonicalize_xml { my $self = shift; - my ($xml,$comments) = @_; + my ($xml,$comments,$prefixlist) = @_; $comments = 0 unless $comments; + $prefixlist = '' unless $prefixlist; if ( $self->{canonicalizer} eq 'XML::Canonical' ) { require XML::Canonical; + + # TODO - pass prefixlist in here if X::C supports it + my $xmlcanon = XML::Canonical->new( comments => $comments ); return $xmlcanon->canonicalize_string( $xml ); } elsif ( $self->{ canonicalizer } eq 'XML::CanonicalizeXML' ) { require XML::CanonicalizeXML; my $xpath = '(//. | //@* | //namespace::*)'; - return XML::CanonicalizeXML::canonicalize( $xml, $xpath, [], 1, $comments ); + + # adjust prefixlist from attribute for XML::CanonicalizeXML's format + $prefixlist =~ s/ /,/g; + + return XML::CanonicalizeXML::canonicalize( $xml, $xpath, $prefixlist, 1, $comments ); } else { confess "Unknown XML canonicalizer module."; From df1446c8801f5ecccc7ed3952ff7f4156c688bc0 Mon Sep 17 00:00:00 2001 From: Chris Andrews Date: Fri, 14 Oct 2011 19:24:38 +0100 Subject: [PATCH 12/13] Support multiple signatures. --- lib/Net/SAML2/XML/Sig.pm | 78 ++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/lib/Net/SAML2/XML/Sig.pm b/lib/Net/SAML2/XML/Sig.pm index 3f59c91..b46a4ba 100644 --- a/lib/Net/SAML2/XML/Sig.pm +++ b/lib/Net/SAML2/XML/Sig.pm @@ -118,62 +118,76 @@ sub verify { my $signature_nodeset = $self->{parser}->findnodes('//dsig:Signature'); - my $value = $self->{parser}->findvalue('dsig:SignatureValue', $signature_node); + while (my $signature_node = $signature_nodeset->shift()) { - my $signature = _trim($self->{parser}->findvalue('dsig:SignatureValue', $signature_node)); - my $signed_info_node = $self->_get_node('dsig:SignedInfo', $signature_node); + my $value = $self->{parser}->findvalue('dsig:SignatureValue', $signature_node); - my $ns; - if (defined $signature_node && ref $signature_node) { + my $signature = _trim($self->{parser}->findvalue('dsig:SignatureValue', $signature_node)); + my $signed_info_node = $self->_get_node('dsig:SignedInfo', $signature_node); + + my $ns; + if (defined $signature_node && ref $signature_node) { $ns = $signature_node->getNamespaces->[0]; $self->{dsig_prefix} = ($ns->getPrefix eq '#default') ? '' : $ns->getPrefix; - } - else { + } + else { die "no Signature node?"; - } + } - if (scalar @{ $signed_info_node->getNamespaces } == 0) { - $signed_info_node->appendNamespace($ns); - } + if (scalar @{ $signed_info_node->getNamespaces } == 0) { + $signed_info_node->appendNamespace($ns); + } - my $signed_info = XML::XPath::XMLParser::as_string($signed_info_node); - my $signed_info_canon = $self->_canonicalize_xml( $signed_info ); + my $signed_info = XML::XPath::XMLParser::as_string($signed_info_node); + my $signed_info_canon = $self->_canonicalize_xml( $signed_info ); - if (defined $self->{cert_obj}) { + if (defined $self->{cert_obj}) { # use the provided cert to verify - return 0 unless $self->_verify_x509_cert($self->{cert_obj},$signed_info_canon,$signature); - } - else { + unless ($self->_verify_x509_cert($self->{cert_obj},$signed_info_canon,$signature)) { + print STDERR "not verified by x509\n"; + return 0; + } + } + else { # extract the certficate or key from the document my $keyinfo_nodeset; if ($keyinfo_nodeset = $self->{parser}->findnodes('dsig:KeyInfo/dsig:X509Data', $signature_node)) { my $keyinfo_node = $keyinfo_nodeset->shift(); - return 0 unless $self->_verify_x509($keyinfo_node,$signed_info_canon,$signature); + unless ($self->_verify_x509($keyinfo_node,$signed_info_canon,$signature)) { + print STDERR "not verified by x509\n"; + return 0; + } } elsif ($keyinfo_nodeset = $self->{parser}->find('dsig:KeyInfo/dsig:KeyValue/dsig:RSAKeyValue', $signature_node)) { my $keyinfo_node = $keyinfo_nodeset->shift(); - return 0 unless $self->_verify_rsa($keyinfo_node,$signed_info_canon,$signature); + unless ($self->_verify_rsa($keyinfo_node,$signed_info_canon,$signature)) { + print STDERR "not verified by rsa\n"; + return 0; + } } elsif ($keyinfo_nodeset = $self->{parser}->find('dsig:KeyInfo/dsig:KeyValue/dsig:DSAKeyValue', $signature_node)) { my $keyinfo_node = $keyinfo_nodeset->shift(); - return 0 unless $self->_verify_dsa($keyinfo_node,$signed_info_canon,$signature); + unless ($self->_verify_dsa($keyinfo_node,$signed_info_canon,$signature)) { + print STDERR "not verified by dsa\n"; + return 0; + } } else { - die "Unrecognized key type or no KeyInfo in document"; + die "Unrecognized key type or no KeyInfo in document"; } - } + } - my $digest_method = $self->{parser}->findvalue('dsig:Reference/dsig:DigestMethod/@Algorithm', $signed_info_node); - my $digest = _trim($self->{parser}->findvalue('dsig:Reference/dsig:DigestValue', $signed_info_node)); + my $digest_method = $self->{parser}->findvalue('dsig:Reference/dsig:DigestMethod/@Algorithm', $signed_info_node); + my $refdigest = _trim($self->{parser}->findvalue('dsig:Reference/dsig:DigestValue', $signed_info_node)); - my $signed_xml = $self->_get_signed_xml( $signature_node ); - - my $canonical = $self->_transform( $signed_xml, $signature_node ); - - my $digest_bin = sha1( $canonical ); + my $signed_xml = $self->_get_signed_xml( $signature_node ); + my $canonical = $self->_transform( $signed_xml, $signature_node ); + my $digest = encode_base64(_trim(sha1( $canonical )), ''); - return 1 if ($digest eq _trim(encode_base64($digest_bin))); - return 0; + return 0 unless ($digest eq $refdigest); + } + + return 1; } sub signer_cert { @@ -358,7 +372,9 @@ sub _transform_env_sig { } # This removes the first Signature tag from the XML - even if there is another XML tree with another Signature inside and that comes first. # TODO: Remove the outermost Signature only. + $str =~ s/(<${prefix}Signature(.*?)>(.*?)\<\/${prefix}Signature>)//is; + return $str; } From fb01b8b145f7ce95b6635c9d5db04c77b7196a37 Mon Sep 17 00:00:00 2001 From: Oskari Okko Ojala Date: Fri, 4 Nov 2011 13:54:10 +0200 Subject: [PATCH 13/13] support for forceAuthn in AuthnRequest --- lib/Net/SAML2/Protocol/AuthnRequest.pm | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/Net/SAML2/Protocol/AuthnRequest.pm b/lib/Net/SAML2/Protocol/AuthnRequest.pm index f8533e5..de92853 100644 --- a/lib/Net/SAML2/Protocol/AuthnRequest.pm +++ b/lib/Net/SAML2/Protocol/AuthnRequest.pm @@ -16,7 +16,8 @@ Net::SAML2::Protocol::AuthnRequest - SAML2 AuthnRequest object issueinstant => DateTime->now, issuer => $self->{id}, destination => $destination, - nameid_format => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', # or 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' + nameid_format => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', # or 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', + forceAuthn => 1, ); =head1 METHODS @@ -38,6 +39,7 @@ has 'issuer' => (isa => Uri, is => 'ro', required => 1, coerce => 1); has 'destination' => (isa => Uri, is => 'ro', required => 1, coerce => 1); has 'nameid_format' => (isa => NonEmptySimpleStr, is => 'ro', required => 1, default => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'); has 'providername' => (is => 'rw', required => 0, default => 'ProviderName'); +has 'forceAuthn' => (is => 'rw', required => 0, default => 0); =head2 as_xml() @@ -55,15 +57,12 @@ sub as_xml { $x->xml( $x->AuthnRequest( $samlp, - { Destination => $self->destination, - ID => $self->id, - IssueInstant => $self->issue_instant, + { Destination => $self->destination(), + ID => $self->id(), + IssueInstant => $self->issue_instant(), ProviderName => $self->providername(), Version => '2.0', -# AssertionConsumerServiceURL => 'http://localhost/saml/consumer_post', -# IsPassive => 'false', -# ProtocolBinding => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST', -# ForceAuthn => "true", + ForceAuthn => $self->forceAuthn(), }, $x->Issuer( $saml, @@ -72,7 +71,7 @@ sub as_xml { $x->NameIDPolicy( $samlp, { AllowCreate => '1', - Format => $self->nameid_format }, + Format => $self->nameid_format() }, ), ) );