Merge ~roadmr/canonical-identity-provider:call-me-saml-sha2-maybe into canonical-identity-provider:master

Proposed by Daniel Manrique
Status: Merged
Approved by: Daniel Manrique
Approved revision: 73e7807f2a812b4a20681863517b6d08a3c5bf1f
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~roadmr/canonical-identity-provider:call-me-saml-sha2-maybe
Merge into: canonical-identity-provider:master
Diff against target: 201 lines (+104/-16)
5 files modified
requirements.txt (+3/-1)
requirements_devel.txt (+3/-0)
src/identityprovider/tests/cert/certificate-2048bit.pem (+21/-0)
src/identityprovider/tests/cert/private-key-2048bit.pem (+27/-0)
src/ubuntu_sso_saml/tests/test_processors.py (+50/-15)
Reviewer Review Type Date Requested Status
Maximiliano Bertacchini Approve
Review via email: mp+388613@code.launchpad.net

Commit message

Update saml2idp to 0.21 for proper, tested, working sha2 digest/signature support in SAML.

Also update the tests so we're sure the correct identifiers are used at the SSO level; correct signing itself is tested thoroughly in the saml2idp project proper.

Had a bit more repercussions than I expected (required adding bs4 and updating m2crypto which required a custom wheel instead of system package) but it works well in local tests....

To post a comment you must log in.
Revision history for this message
Maximiliano Bertacchini (maxiberta) wrote :

LGTM

review: Approve
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/requirements.txt b/requirements.txt
2index ad319b6..e250bd7 100644
3--- a/requirements.txt
4+++ b/requirements.txt
5@@ -34,7 +34,7 @@ python-openid==2.2.5
6 raven==6.10.0
7 requests==2.23.0
8 requests-oauthlib==0.4.2
9-saml2idp==0.20
10+saml2idp==0.21
11 six==1.10.0
12 ssoclient==2.1.1
13 talisker[django,gunicorn,raven]==0.18.0
14@@ -56,6 +56,7 @@ enum34==1.1.6
15 idna==2.1
16 ipaddress==1.0.16
17 libnacl==1.5.0
18+M2Crypto==0.36.0 # saml2-idp
19 paste==2.0.1
20 psycopg2==2.7.4
21 pyasn1==0.1.9
22@@ -63,4 +64,5 @@ pycparser==2.14
23 six==1.10.0
24 statsd==3.3.0
25 testresources==0.2.7
26+typing==3.7.4.3 # M2crypto
27 ua_parser==0.10.0
28diff --git a/requirements_devel.txt b/requirements_devel.txt
29index a0e461e..08893ff 100644
30--- a/requirements_devel.txt
31+++ b/requirements_devel.txt
32@@ -9,6 +9,7 @@
33 # an import statement.
34
35 coverage==3.6
36+beautifulsoup4==4.9.1
37 fixtures==0.3.12
38 flake8==3.5.0
39 flake8-import-order==0.16
40@@ -35,6 +36,7 @@ wsgi-intercept==1.6.0
41 #
42 # If a dependency is needed by several requirement files, add it to all.
43
44+backports.functools_lru_cache==1.6.1 # beautifulsoup4
45 cookies==2.2.1
46 cssselect==0.7.1
47 ecdsa==0.11
48@@ -50,5 +52,6 @@ pyflakes==1.6.0
49 pygpgme==0.3
50 python-mimeparse==0.1.4
51 selenium==2.53.6
52+soupsieve==1.9.6 # beautifulsoup4
53 sqlparse==0.1.4
54 txfixtures==0.1.4
55diff --git a/src/identityprovider/tests/cert/certificate-2048bit.pem b/src/identityprovider/tests/cert/certificate-2048bit.pem
56new file mode 100644
57index 0000000..e956879
58--- /dev/null
59+++ b/src/identityprovider/tests/cert/certificate-2048bit.pem
60@@ -0,0 +1,21 @@
61+-----BEGIN CERTIFICATE-----
62+MIIDazCCAlOgAwIBAgIUNWtKq9Rqg37GmIsRLnQlF9PPwaowDQYJKoZIhvcNAQEL
63+BQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
64+GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMDA4MDMyMDMyMzBaFw0zMDA4
65+MDEyMDMyMzBaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
66+HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB
67+AQUAA4IBDwAwggEKAoIBAQDSPBU6+OtAXmonyipl+g5bxsLXjxie8Dor+jhdSECw
68+yBJjVTFJzMGlCweEBBhUggjHRoslpkP9PToaF/3XsR/5V6HKJ28BjVsPeeueGC3M
69+RTA43YE8lqydSjNdLT3/ILFD+cPhFxfBU2lqalRvgDRduW5V93Q0OkZzca3hDbUn
70+S81P5Q1xnToxO1mzAVf7+DC9TA0SDDrCnL6XLWAzSOjTHwQEEoFrlwQ2o+1owh15
71+FKJfh6wcJr1EjnujFQNpgWX+0Lt6dGFneFBxtfpfg5i3yh0iAGFuXFRBDsQwP8yi
72+hIvZz+DYKWq2tYPj2YpjjWzjXB+EYvRRvkJVcSM/WiCtAgMBAAGjUzBRMB0GA1Ud
73+DgQWBBQWV7lLFypOCMYkhDbJr83T+Cp8izAfBgNVHSMEGDAWgBQWV7lLFypOCMYk
74+hDbJr83T+Cp8izAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCA
75+BSfvMpbCVKdxiJBr5zW6TOA93sLhXrLbUMzXrTO01v1D1HAQ/F6dQloDXCBMJK/h
76+qwRpyTClu8QoKUBovTCQ7XSp7CYPL+x8a+tYiCn7GnUOSbI3+q2OWjP5745bA09O
77+zikC5eJ6HXCmNPMMrbGaksC6WvfV87uCQSes19bqZxgia/zP1Ky6g83TulOOVIOx
78+3LNSnY4D2Fag3i4LN19GCO1q8vkZ/krkRp17h7TfUnvtLJW897XfYp8PBE6NGJeh
79+AW3BxlDaRn0PHGtzlZfgkA6PI4Fa2Z7KHnuBXpv13B5j/5on1OLxNEMg8Ls6TN0z
80+ofV1SfjKsL4tAjufuXea
81+-----END CERTIFICATE-----
82diff --git a/src/identityprovider/tests/cert/private-key-2048bit.pem b/src/identityprovider/tests/cert/private-key-2048bit.pem
83new file mode 100644
84index 0000000..b50eaad
85--- /dev/null
86+++ b/src/identityprovider/tests/cert/private-key-2048bit.pem
87@@ -0,0 +1,27 @@
88+-----BEGIN RSA PRIVATE KEY-----
89+MIIEpAIBAAKCAQEA0jwVOvjrQF5qJ8oqZfoOW8bC148YnvA6K/o4XUhAsMgSY1Ux
90+SczBpQsHhAQYVIIIx0aLJaZD/T06Ghf917Ef+VehyidvAY1bD3nrnhgtzEUwON2B
91+PJasnUozXS09/yCxQ/nD4RcXwVNpampUb4A0XbluVfd0NDpGc3Gt4Q21J0vNT+UN
92+cZ06MTtZswFX+/gwvUwNEgw6wpy+ly1gM0jo0x8EBBKBa5cENqPtaMIdeRSiX4es
93+HCa9RI57oxUDaYFl/tC7enRhZ3hQcbX6X4OYt8odIgBhblxUQQ7EMD/MooSL2c/g
94+2ClqtrWD49mKY41s41wfhGL0Ub5CVXEjP1ogrQIDAQABAoIBAQC5lqyvOJqEYlSX
95+95HiIBKY1ieenwX/GNZhiCxFvMjOxm/lHIm8OnMfvVLPKcZIo9oYAKBJhjMy1N/7
96+tP4zcHtnZALBMHqABkdCrEBhebrEPrfJG1FBKUoMODqLoKrAFB4ogc53YClqUrYW
97+aPNM0wJnUpCidyYwDNmwg8QZjj5DBr8dmWbj2KzeZGtfWovyEf+UQjoqglOHoVzb
98+dVYvp8Zuf5ExXWivrCnuRDk3V4BSwKv+NJ5G8MyuY+PeJSa+5joomj3Woslh0AVy
99+BF5AMMi8kuK9ZP2zDHgJ7RZgRu1cWJ6h7HXPm7Z9nA0ahLp12Bb1gvDbxTiSX0kl
100+TXl9ieDBAoGBAPgWj81sdOHZrqNisstLV8qK4FLREqIzZKWQu+EEMXcGVuaj1DPJ
101+JAuhEIpzkeBNJBkCDWQ/TR9uDOLqEvAKZITEu12Rbc/d1c3ZjaNluXAJOiqanFdi
102+BYg7jnxuxsbWI0zZgg5mlYjM8YXSYUn60O/Lvz9emJGaAiEQuQRyYmPlAoGBANjw
103+eohKnP3x2+kfLDlbXgJTOmWDWwqm7s0QKyIUOTug8sW/0/1YBJssDJ1A9TGzes21
104+cU4IctX+by3KJVIbpWAajLSOLdXUUkoWezn0XnX4eBvfyxLIaCEIEki0m64pT/eC
105+dD/T4lGBCvKPlwJnigOqPyLdFGnIHdovCMiHF40pAoGBAOr/fCILTUAukeWEdXZq
106+nPNnz4vDLZJlej32tnE0JqLnZ2O75iNTsYgufluXk6PTjpD9x6+g4q4V8bD0JxIn
107+1gWJ0S2Vt15CdOcz353amuhMZUO8BsIjQLhGKfcme7YXW4LNOgvoElduQo4PtUZ9
108+hnLPHg6pzX2GU+P8UjWrIBYdAoGAZ5GnbhnxCWvGSW2Qd3GfWAzT/FLRzCwWJeBq
109+N0LgcA9O2AnU80ZqLIrDGvBAHxJItpzBEzgFWxS1j27KkoCQW2lRc4HNcCCFiMli
110+2zBHSJru/J/DG5yB2gM4d22CRYuDme62ASLvEWpCB7t1pLg5s7Y7njFd5YKcfeWm
111+k9Wq8MECgYADI/UrQH3jZM+K4qtwAcuwroHHsRXACkoW3tQFkFbTAjZGbFRi/+oQ
112+0uu860zpgPv5Oyr8Y1j5G9CMJPGYSUSGFY8mqb+nWIK0Cc4ns1EV4U669FN3DFrf
113+ifL9nXlgW+3x71HnJ/86Ry49UA222Dcns6lVrVNZSujp8vpD5B2Mtw==
114+-----END RSA PRIVATE KEY-----
115diff --git a/src/ubuntu_sso_saml/tests/test_processors.py b/src/ubuntu_sso_saml/tests/test_processors.py
116index 6117468..d92ac21 100644
117--- a/src/ubuntu_sso_saml/tests/test_processors.py
118+++ b/src/ubuntu_sso_saml/tests/test_processors.py
119@@ -2038,11 +2038,20 @@ class CanonicalProcessorTestCase(CanonicalProcessorBaseTestCase):
120 data = self.get_request_data()
121 response = self.client.get('/+saml', data=data, follow=True)
122 samlresponse = get_saml_response(response)
123- # Ensure sha1 mentioned in the assertion
124- self.assertIn("sha1", samlresponse)
125- # Ensure sha256 or sha512 NOT mentioned in the assertion
126- self.assertNotIn("sha256", samlresponse)
127- self.assertNotIn("sha512", samlresponse)
128+ # Ensure digest and signature methods are sha1
129+ self.assertIn(
130+ "http://www.w3.org/2000/09/xmldsig#rsa-sha1", samlresponse)
131+ self.assertIn(
132+ "http://www.w3.org/2000/09/xmldsig#sha1", samlresponse)
133+ # Ensure sha256 or sha512 identifiers NOT mentioned in the assertion
134+ self.assertNotIn(
135+ "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", samlresponse)
136+ self.assertNotIn(
137+ "http://www.w3.org/2001/04/xmlenc#sha256", samlresponse)
138+ self.assertNotIn(
139+ "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512", samlresponse)
140+ self.assertNotIn(
141+ "http://www.w3.org/2001/04/xmlenc#sha512", samlresponse)
142
143 def test_sha256_in_assertion(self):
144 self.setup_saml_sp(signing_algorithm='sha256')
145@@ -2050,20 +2059,46 @@ class CanonicalProcessorTestCase(CanonicalProcessorBaseTestCase):
146 data = self.get_request_data()
147 response = self.client.get('/+saml', data=data, follow=True)
148 samlresponse = get_saml_response(response)
149- # Ensure sha256 mentioned in the assertion
150- self.assertIn("sha256", samlresponse)
151- # Ensure sha1 or 512 NOT mentioned in the assertion
152- self.assertNotIn("sha1", samlresponse)
153- self.assertNotIn("sha512", samlresponse)
154+ # Ensure digest and signature methods are sha256
155+ self.assertIn(
156+ "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", samlresponse)
157+ self.assertIn(
158+ "http://www.w3.org/2001/04/xmlenc#sha256", samlresponse)
159+ # Ensure sha1 or sha512 identifiers NOT mentioned in the assertion
160+ self.assertNotIn(
161+ "http://www.w3.org/2000/09/xmldsig#rsa-sha1", samlresponse)
162+ self.assertNotIn(
163+ "http://www.w3.org/2000/09/xmldsig#sha1", samlresponse)
164+ self.assertNotIn(
165+ "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512", samlresponse)
166+ self.assertNotIn(
167+ "http://www.w3.org/2001/04/xmlenc#sha512", samlresponse)
168
169 def test_sha512_in_assertion(self):
170+ # sha512 requires a larger key, the default one is 256-bit or so.
171+ cert_file = find_cert_path('certificate-2048bit.pem')
172+ priv_file = find_cert_path('private-key-2048bit.pem')
173+ test_config = {
174+ 'certificate_file': cert_file,
175+ 'private_key_file': priv_file,
176+ }
177+ saml2idp_metadata.SAML2IDP_CONFIG.update(test_config)
178 self.setup_saml_sp(signing_algorithm='sha512')
179
180 data = self.get_request_data()
181 response = self.client.get('/+saml', data=data, follow=True)
182 samlresponse = get_saml_response(response)
183- # Ensure sha512 mentioned in the assertion
184- self.assertIn("sha512", samlresponse)
185- # Ensure sha1 or 256 NOT mentioned in the assertion
186- self.assertNotIn("sha1", samlresponse)
187- self.assertNotIn("sha256", samlresponse)
188+ # Ensure digest and signature methods are sha512
189+ self.assertIn(
190+ "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512", samlresponse)
191+ self.assertIn(
192+ "http://www.w3.org/2001/04/xmlenc#sha512", samlresponse)
193+ # Ensure sha1 or sha256 identifiers NOT mentioned in the assertion
194+ self.assertNotIn(
195+ "http://www.w3.org/2000/09/xmldsig#rsa-sha1", samlresponse)
196+ self.assertNotIn(
197+ "http://www.w3.org/2000/09/xmldsig#sha1", samlresponse)
198+ self.assertNotIn(
199+ "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", samlresponse)
200+ self.assertNotIn(
201+ "http://www.w3.org/2001/04/xmlenc#sha256", samlresponse)

Subscribers

People subscribed via source and target branches