Skip to content

Commit 9226e04

Browse files
timleggetimlegge
authored
Merge pull request #84 from waterkip/GH-verify-xml-and_trust_anchors
BREAKING CHANGES: Verifying XML and trust anchors on SAML responses
2 parents 5fd7f45 + 4902c89 commit 9226e04

File tree

8 files changed

+257
-97
lines changed

8 files changed

+257
-97
lines changed

‎lib/Net/SAML2/Binding/POST.pm‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ use Net::SAML2::XML::Sig;
2828
use MIME::Base64 qw/ decode_base64 /;
2929
use Crypt::OpenSSL::Verify;
3030

31+
with 'Net::SAML2::Role::VerifyXML';
32+
3133
=head2 new( )
3234
3335
Constructor. Returns an instance of the POST binding.
@@ -59,27 +61,19 @@ sub handle_response {
5961

6062
# unpack and check the signature
6163
my $xml = decode_base64($response);
62-
my $xml_opts = { x509 => 1 };
63-
$xml_opts->{ cert_text } = $self->cert_text if ($self->cert_text);
64-
$xml_opts->{ exclusive } = 1;
65-
$xml_opts->{ no_xml_declaration } = 1;
66-
my $x = Net::SAML2::XML::Sig->new($xml_opts);
67-
my $ret = $x->verify($xml);
68-
die "signature check failed" unless $ret;
69-
70-
if ($self->cacert) {
71-
my $cert = $x->signer_cert
72-
or die "Certificate not provided and not in SAML Response, cannot validate";
73-
74-
my $ca = Crypt::OpenSSL::Verify->new($self->cacert, { strict_certs => 0, });
75-
if ($ca->verify($cert)) {
76-
return sprintf("%s (verified)", $cert->subject);
77-
} else {
78-
return 0;
79-
}
80-
}
81-
82-
return 1;
64+
65+
$self->verify_xml(
66+
$xml,
67+
no_xml_declaration => 1,
68+
$self->cert_text ? (
69+
cert_text => $self->cert_text
70+
) : (),
71+
$self->cacert ? (
72+
cacert => $self->cacert
73+
) : (),
74+
75+
);
76+
return $xml;
8377
}
8478

8579
__PACKAGE__->meta->make_immutable;

‎lib/Net/SAML2/Binding/SOAP.pm‎

Lines changed: 45 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
1-
use strict;
2-
use warnings;
31
package Net::SAML2::Binding::SOAP;
2+
use Moose;
3+
44
# VERSION
55

6-
use Moose;
76
use MooseX::Types::URI qw/ Uri /;
87
use Net::SAML2::XML::Util qw/ no_comments /;
8+
use Carp qw(croak);
9+
10+
with 'Net::SAML2::Role::VerifyXML';
911

10-
# ABSTRACT: Net::SAML2::Binding::Artifact - SOAP binding for SAML
12+
# ABSTRACT: Net::SAML2::Binding::SOAP - SOAP binding for SAML
1113

1214
=head1 NAME
1315
14-
Net::SAML2::Binding::Artifact - SOAP binding for SAML2
16+
Net::SAML2::Binding::SOAP - SOAP binding for SAML2
1517
1618
=head1 SYNOPSIS
1719
@@ -93,7 +95,18 @@ has 'url' => (isa => Uri, is => 'ro', required => 1, coerce => 1);
9395
has 'key' => (isa => 'Str', is => 'ro', required => 1);
9496
has 'cert' => (isa => 'Str', is => 'ro', required => 1);
9597
has 'idp_cert' => (isa => 'Str', is => 'ro', required => 1);
96-
has 'cacert' => (isa => 'Str', is => 'ro', required => 1);
98+
has 'cacert' => (
99+
is => 'ro',
100+
isa => 'Str',
101+
required => 0,
102+
predicate => 'has_cacert'
103+
);
104+
has 'anchors' => (
105+
is => 'ro',
106+
isa => 'HashRef',
107+
required => 0,
108+
predicate => 'has_anchors'
109+
);
97110

98111
=head2 request( $message )
99112
@@ -142,35 +155,16 @@ Accepts a string containing the complete SOAP response.
142155
sub handle_response {
143156
my ($self, $response) = @_;
144157

145-
# verify the response
146-
my $x = Net::SAML2::XML::Sig->new(
147-
{
148-
x509 => 1,
149-
cert_text => $self->idp_cert,
150-
exclusive => 1,
158+
my $saml = _get_saml_from_soap($response);
159+
$self->verify_xml(
160+
$saml,
151161
no_xml_declaration => 1,
152-
});
153-
154-
my $ret = $x->verify($response);
155-
die "bad SOAP response" unless $ret;
156-
157-
# verify the signing certificate
158-
my $cert = $x->signer_cert;
159-
my $ca = Crypt::OpenSSL::Verify->new($self->cacert, { strict_certs => 0, });
160-
$ret = $ca->verify($cert);
161-
die "bad signer cert" unless $ret;
162-
163-
my $subject = sprintf("%s (verified)", $cert->subject);
162+
cert_text => $self->idp_cert,
163+
cacert => $self->cacert,
164+
anchors => $self->anchors
165+
);
166+
return $saml;
164167

165-
# parse the SOAP response and return the payload
166-
my $dom = no_comments($response);
167-
168-
my $parser = XML::LibXML::XPathContext->new($dom);
169-
$parser->registerNs('soap-env', 'http://schemas.xmlsoap.org/soap/envelope/');
170-
$parser->registerNs('samlp', 'urn:oasis:names:tc:SAML:2.0:protocol');
171-
172-
my $saml = $parser->findnodes_as_string('/soap-env:Envelope/soap-env:Body/*');
173-
return ($subject, $saml);
174168
}
175169

176170
=head2 handle_request( $request )
@@ -184,29 +178,29 @@ Accepts a string containing the complete SOAP request.
184178
sub handle_request {
185179
my ($self, $request) = @_;
186180

187-
my $dom = no_comments($request);
181+
my $saml = _get_saml_from_soap($request);
182+
if (defined $saml) {
183+
$self->verify_xml(
184+
$saml,
185+
cert_text => $self->idp_cert,
186+
cacert => $self->cacert
187+
);
188+
return $saml;
189+
}
190+
191+
return;
192+
}
188193

194+
sub _get_saml_from_soap {
195+
my $soap = shift;
196+
my $dom = no_comments($soap);
189197
my $parser = XML::LibXML::XPathContext->new($dom);
190198
$parser->registerNs('soap-env', 'http://schemas.xmlsoap.org/soap/envelope/');
191199
$parser->registerNs('samlp', 'urn:oasis:names:tc:SAML:2.0:protocol');
192-
193-
my ($nodes) = $parser->findnodes('/soap-env:Envelope/soap-env:Body/*');
194-
my $saml = $nodes->toString;
195-
196-
if (defined $saml) {
197-
my $x = Net::SAML2::XML::Sig->new({ x509 => 1, cert_text => $self->idp_cert, exclusive => 1, });
198-
my $ret = $x->verify($saml);
199-
die "bad signature" unless $ret;
200-
201-
my $cert = $x->signer_cert;
202-
my $ca = Crypt::OpenSSL::Verify->new($self->cacert, { strict_certs => 0, });
203-
$ret = $ca->verify($cert);
204-
die "bad certificate in request: ".$cert->subject unless $ret;
205-
206-
my $subject = $cert->subject;
207-
return ($subject, $saml);
200+
my $set = $parser->findnodes('/soap-env:Envelope/soap-env:Body/*');
201+
if ($set->size) {
202+
return $set->get_node(1)->toString();
208203
}
209-
210204
return;
211205
}
212206

‎lib/Net/SAML2/Role/VerifyXML.pm‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package Net::SAML2::Role::VerifyXML;
2+
use Moose::Role;
3+
# VERSION
4+
5+
use Net::SAML2::XML::Sig;
6+
use Crypt::OpenSSL::Verify;
7+
use Crypt::OpenSSL::X509;
8+
use Carp qw(croak);
9+
use List::Util qw(none);
10+
11+
# ABSTRACT: A role to verify the SAML response XML
12+
13+
=head1 DESCRIPTION
14+
15+
=head1 SYNOPSIS
16+
17+
use Net::SAML2::Some::Module;
18+
19+
use Moose;
20+
with 'Net::SAML2::Role::VerifyXML';
21+
22+
sub do_something_with_xml {
23+
my $self = shift;
24+
my $xml = shift;
25+
26+
$self->verify_xml($xml,
27+
# Most of these options are passed to Net::SAML2::XML::Sig, except for the
28+
# cacert
29+
# Most options are optional
30+
cacert => $self->cacert,
31+
cert_text => $self->cert,
32+
no_xml_declaration => 1,
33+
);
34+
}
35+
36+
=cut
37+
38+
39+
=head1 METHODS
40+
41+
=head2 verify_xml($xml, %args)
42+
43+
$self->verify_xml($xml,
44+
# Most of these options are passed to Net::SAML2::XML::Sig, except for the
45+
# cacert
46+
# Most options are optional
47+
cert_text => $self->cert,
48+
no_xml_declaration => 1,
49+
50+
# Used for a trust model, if lacking, everything is trusted
51+
cacert => $self->cacert,
52+
# or check specific certificates based on subject/issuer or issuer hash
53+
anchors => {
54+
# one of the following is allowed
55+
subject => ["subject a", "subject b"],
56+
issuer => ["Issuer A", "Issuer B"],
57+
issuer_hash => ["Issuer A hash", "Issuer B hash"],
58+
},
59+
);
60+
61+
=cut
62+
63+
sub verify_xml {
64+
my $self = shift;
65+
my $xml = shift;
66+
my %args = @_;
67+
68+
my $cacert = delete $args{cacert};
69+
my $anchors = delete $args{anchors};
70+
71+
my $x = Net::SAML2::XML::Sig->new({
72+
x509 => 1,
73+
exclusive => 1,
74+
%args,
75+
});
76+
77+
croak("XML signature check failed") unless $x->verify($xml);
78+
79+
if (!$anchors && !$cacert) {
80+
return 1;
81+
}
82+
83+
my $cert = $x->signer_cert
84+
or die "Certificate not provided in SAML Response, cannot validate\n";
85+
86+
if ($cacert) {
87+
my $ca = Crypt::OpenSSL::Verify->new($cacert, { strict_certs => 0 });
88+
eval { $ca->verify($cert) };
89+
if ($@) {
90+
croak("Could not verify CA certificate: $@");
91+
}
92+
}
93+
94+
return 1 if !$anchors;
95+
96+
if (ref $anchors ne 'HASH') {
97+
croak("Unable to verify anchor trust");
98+
}
99+
100+
my ($key) = keys %$anchors;
101+
if (none { $key eq $_ } qw(subject issuer issuer_hash)) {
102+
croak("Unable to verify anchor trust, requires subject, issuer or issuer_hash");
103+
}
104+
105+
my $got = $cert->$key;
106+
my $want = $anchors->{$key};
107+
if (!ref $want) {
108+
$want = [ $want ];
109+
}
110+
111+
if (none { $_ eq $got } @$want) {
112+
croak("Could not verify trust anchors of certificate!");
113+
}
114+
return 1;
115+
116+
}
117+
118+
119+
1;
120+
121+
__END__
122+

‎lib/Net/SAML2/SP.pm‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -413,17 +413,13 @@ XXX UA
413413
sub soap_binding {
414414
my ($self, $ua, $idp_url, $idp_cert) = @_;
415415

416-
if (!$self->has_cacert) {
417-
croak("Unable to create SOAP binding, no CA certificate provided");
418-
}
419-
420416
return Net::SAML2::Binding::SOAP->new(
421417
ua => $ua,
422418
key => $self->key,
423419
cert => $self->cert,
424420
url => $idp_url,
425421
idp_cert => $idp_cert,
426-
cacert => $self->cacert,
422+
$self->has_cacert ? (cacert => $self->cacert) : (),
427423
);
428424
}
429425

‎lib/Net/SAML2/Util.pm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ package Net::SAML2::Util;
55

66
use Crypt::OpenSSL::Random qw(random_pseudo_bytes);
77

8-
# ABSTRACT: Utility functions for Net:SAML2
8+
# ABSTRACT: Utility functions for Net::SAML2
99

1010
use Exporter qw(import);
1111

‎lib/Net/SAML2/XML/Util.pm‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,11 @@ sub no_comments {
4444

4545
# Remove comments from XML to mitigate XML comment auth bypass
4646
my $dom = XML::LibXML->load_xml(
47-
string => $xml,
48-
no_network => 1,
49-
load_ext_dtd => 0,
50-
expand_entities => 0 );
47+
string => $xml,
48+
no_network => 1,
49+
load_ext_dtd => 0,
50+
expand_entities => 0
51+
);
5152

5253
for my $comment_node ($dom->findnodes('//comment()')) {
5354
$comment_node->parentNode->removeChild($comment_node);

‎t/04-response.t‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,23 +67,19 @@ my $sp = net_saml2_sp();
6767

6868
my $post = $sp->post_binding;
6969

70-
my $subject;
70+
my $response_xml;
7171

7272
lives_ok(
7373
sub {
74-
$subject = $post->handle_response($response);
74+
$response_xml = $post->handle_response($response);
7575
},
7676
'$sp->handle_response works'
7777
);
7878

7979

80-
ok(defined $subject, "->handle response verified something");
81-
like($subject, qr/verified/, "Matches the verified string");
82-
83-
## TODO: Move to t/03-assertion
84-
my $assertion_xml = decode_base64($response);
85-
my $assertion = Net::SAML2::Protocol::Assertion->new_from_xml(xml => $xml);
80+
is($xml, $response_xml, "We have the response XML as XML");
8681

82+
my $assertion = Net::SAML2::Protocol::Assertion->new_from_xml(xml => $response_xml);
8783
isa_ok($assertion, 'Net::SAML2::Protocol::Assertion');
8884

8985
done_testing;

0 commit comments

Comments
 (0)