Merge lp:~dobey/ubuntuone-storage-protocol/update-3-0 into lp:ubuntuone-storage-protocol/stable-3-0

Proposed by dobey
Status: Merged
Approved by: Roberto Alsina
Approved revision: 158
Merged at revision: 158
Proposed branch: lp:~dobey/ubuntuone-storage-protocol/update-3-0
Merge into: lp:ubuntuone-storage-protocol/stable-3-0
Diff against target: 314 lines (+263/-12)
4 files modified
data/ValiCert_Class_2_VA.pem (+18/-0)
setup.py (+1/-0)
tests/test_context.py (+197/-0)
ubuntuone/storageprotocol/context.py (+47/-12)
To merge this branch: bzr merge lp:~dobey/ubuntuone-storage-protocol/update-3-0
Reviewer Review Type Date Requested Status
Roberto Alsina (community) Approve
Review via email: mp+109915@code.launchpad.net

Commit message

[Alejandro Cura]

    Add the ValiCert certificate to the custom certificates (LP: #882062).
    Be more strict when validating the SSL certificate

[Rodney Dawes]

    Patch get_certificates to use the uninstalled certificates when testing

To post a comment you must log in.
Revision history for this message
Roberto Alsina (ralsina) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'data/ValiCert_Class_2_VA.pem'
2--- data/ValiCert_Class_2_VA.pem 1970-01-01 00:00:00 +0000
3+++ data/ValiCert_Class_2_VA.pem 2012-06-12 20:02:24 +0000
4@@ -0,0 +1,18 @@
5+-----BEGIN CERTIFICATE-----
6+MIIC5zCCAlACAQEwDQYJKoZIhvcNAQEFBQAwgbsxJDAiBgNVBAcTG1ZhbGlDZXJ0
7+IFZhbGlkYXRpb24gTmV0d29yazEXMBUGA1UEChMOVmFsaUNlcnQsIEluYy4xNTAz
8+BgNVBAsTLFZhbGlDZXJ0IENsYXNzIDIgUG9saWN5IFZhbGlkYXRpb24gQXV0aG9y
9+aXR5MSEwHwYDVQQDExhodHRwOi8vd3d3LnZhbGljZXJ0LmNvbS8xIDAeBgkqhkiG
10+9w0BCQEWEWluZm9AdmFsaWNlcnQuY29tMB4XDTk5MDYyNjAwMTk1NFoXDTE5MDYy
11+NjAwMTk1NFowgbsxJDAiBgNVBAcTG1ZhbGlDZXJ0IFZhbGlkYXRpb24gTmV0d29y
12+azEXMBUGA1UEChMOVmFsaUNlcnQsIEluYy4xNTAzBgNVBAsTLFZhbGlDZXJ0IENs
13+YXNzIDIgUG9saWN5IFZhbGlkYXRpb24gQXV0aG9yaXR5MSEwHwYDVQQDExhodHRw
14+Oi8vd3d3LnZhbGljZXJ0LmNvbS8xIDAeBgkqhkiG9w0BCQEWEWluZm9AdmFsaWNl
15+cnQuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDOOnHK5avIWZJV16vY
16+dA757tn2VUdZZUcOBVXc65g2PFxTXdMwzzjsvUGJ7SVCCSRrCl6zfN1SLUzm1NZ9
17+WlmpZdRJEy0kTRxQb7XBhVQ7/nHk01xC+YDgkRoKWzk2Z/M/VXwbP7RfZHM047QS
18+v4dk+NoS/zcnwbNDu+97bi5p9wIDAQABMA0GCSqGSIb3DQEBBQUAA4GBADt/UG9v
19+UJSZSWI4OB9L+KXIPqeCgfYrx+jFzug6EILLGACOTb2oWH+heQC1u+mNr0HZDzTu
20+IYEZoDJJKPTEjlbVUjP9UNV+mWwD5MlM/Mtsq2azSiGM5bUMMj4QssxsodyamEwC
21+W/POuZ6lcg5Ktz885hZo+L7tdEy8W9ViH0Pd
22+-----END CERTIFICATE-----
23
24=== modified file 'setup.py'
25--- setup.py 2012-05-18 20:26:48 +0000
26+++ setup.py 2012-06-12 20:02:24 +0000
27@@ -76,6 +76,7 @@
28 extra_path='ubuntuone-storage-protocol',
29 data_files=[(ssl_cert_location,
30 ['data/UbuntuOne-Go_Daddy_CA.pem',
31+ 'data/ValiCert_Class_2_VA.pem',
32 'data/UbuntuOne-Go_Daddy_Class_2_CA.pem'])],
33
34 cmdclass={
35
36=== added file 'tests/test_context.py'
37--- tests/test_context.py 1970-01-01 00:00:00 +0000
38+++ tests/test_context.py 2012-06-12 20:02:24 +0000
39@@ -0,0 +1,197 @@
40+# tests.test_context - test ssl context creation
41+#
42+# Copyright 2012 Canonical Ltd.
43+#
44+# This program is free software: you can redistribute it and/or modify it
45+# under the terms of the GNU Affero General Public License version 3,
46+# as published by the Free Software Foundation.
47+#
48+# This program is distributed in the hope that it will be useful, but
49+# WITHOUT ANY WARRANTY; without even the implied warranties of
50+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
51+# PURPOSE. See the GNU Affero General Public License for more details.
52+#
53+# You should have received a copy of the GNU Affero General Public License
54+# along with this program. If not, see <http://www.gnu.org/licenses/>.
55+#
56+# In addition, as a special exception, the copyright holders give
57+# permission to link the code of portions of this program with the
58+# OpenSSL library under certain conditions as described in each
59+# individual source file, and distribute linked combinations
60+# including the two.
61+# You must obey the GNU General Public License in all respects
62+# for all of the code used other than OpenSSL. If you modify
63+# file(s) with this exception, you may extend this exception to your
64+# version of the file(s), but you are not obligated to do so. If you
65+# do not wish to do so, delete this exception statement from your
66+# version. If you delete this exception statement from all source
67+# files in the program, then also delete it here.
68+
69+import os
70+
71+from OpenSSL import crypto, SSL
72+from twisted.internet import defer, error, reactor, ssl
73+from twisted.trial import unittest
74+from twisted.web import client, resource, server
75+
76+from ubuntuone.storageprotocol import context
77+
78+
79+class FakeCerts(object):
80+ """CA and Server certificate."""
81+
82+ def __init__(self, testcase, common_name="fake.domain"):
83+ """Initialize this fake instance."""
84+ self.cert_dir = os.path.join(testcase.mktemp(), 'certs')
85+ if not os.path.exists(self.cert_dir):
86+ os.makedirs(self.cert_dir)
87+
88+ ca_key = self._build_key()
89+ ca_req = self._build_request(ca_key, "Fake Cert Authority")
90+ self.ca_cert = self._build_cert(ca_req, ca_req, ca_key)
91+
92+ server_key = self._build_key()
93+ server_req = self._build_request(server_key, common_name)
94+ server_cert = self._build_cert(server_req, self.ca_cert, ca_key)
95+
96+ self.server_key_path = self._save_key(server_key, "server_key.pem")
97+ self.server_cert_path = self._save_cert(server_cert, "server_cert.pem")
98+
99+ def _save_key(self, key, filename):
100+ """Save a certificate."""
101+ data = crypto.dump_privatekey(crypto.FILETYPE_PEM, key)
102+ return self._save(filename, data)
103+
104+ def _save_cert(self, cert, filename):
105+ """Save a certificate."""
106+ data = crypto.dump_certificate(crypto.FILETYPE_PEM, cert)
107+ return self._save(filename, data)
108+
109+ def _save(self, filename, data):
110+ """Save a key or certificate, and return the full path."""
111+ fullpath = os.path.join(self.cert_dir, filename)
112+ if os.path.exists(fullpath):
113+ os.unlink(fullpath)
114+ with open(fullpath, 'wt') as fd:
115+ fd.write(data)
116+ return fullpath
117+
118+ def _build_key(self):
119+ """Create a private/public key, save it in a temp dir."""
120+ key = crypto.PKey()
121+ key.generate_key(crypto.TYPE_RSA, 1024)
122+ return key
123+
124+ def _build_request(self, key, common_name):
125+ """Create a new certificate request."""
126+ request = crypto.X509Req()
127+ request.get_subject().CN = common_name
128+ request.set_pubkey(key)
129+ request.sign(key, "md5")
130+ return request
131+
132+ def _build_cert(self, request, ca_cert, ca_key):
133+ """Create a new certificate."""
134+ certificate = crypto.X509()
135+ certificate.set_serial_number(1)
136+ certificate.set_issuer(ca_cert.get_subject())
137+ certificate.set_subject(request.get_subject())
138+ certificate.set_pubkey(request.get_pubkey())
139+ certificate.gmtime_adj_notBefore(0)
140+ certificate.gmtime_adj_notAfter(3600) # valid for one hour
141+ certificate.sign(ca_key, "md5")
142+ return certificate
143+
144+
145+class FakeResource(resource.Resource):
146+ """A fake resource."""
147+
148+ isLeaf = True
149+
150+ def render(self, request):
151+ """Render this resource."""
152+ return "ok"
153+
154+
155+class SSLContextTestCase(unittest.TestCase):
156+ """Tests for the context.get_ssl_context function."""
157+
158+ @defer.inlineCallbacks
159+ def setUp(self):
160+ yield super(SSLContextTestCase, self).setUp()
161+ self.patch(context, "get_certificates", self.get_certificates)
162+
163+ def get_certificates(self):
164+ """Get the uninstalled certificates, for testing."""
165+ ca_1 = ssl.Certificate.loadPEM(file(os.path.abspath(os.path.join(
166+ os.pardir, os.pardir, 'data',
167+ 'UbuntuOne-Go_Daddy_Class_2_CA.pem')), 'r').read())
168+ ca_2 = ssl.Certificate.loadPEM(file(os.path.abspath(os.path.join(
169+ os.pardir, os.pardir, 'data',
170+ 'UbuntuOne-Go_Daddy_CA.pem')), 'r').read())
171+ return [ca_1.original, ca_2.original]
172+
173+ @defer.inlineCallbacks
174+ def verify_context(self, server_context, client_context):
175+ """Verify a client context with a given server context."""
176+ site = server.Site(FakeResource())
177+ port = reactor.listenSSL(0, site, server_context)
178+ self.addCleanup(port.stopListening)
179+ url = "https://localhost:%d" % port.getHost().port
180+ result = yield client.getPage(url, contextFactory=client_context)
181+ self.assertEqual(result, "ok")
182+
183+ @defer.inlineCallbacks
184+ def test_no_verify(self):
185+ """Test the no_verify option."""
186+ certs = FakeCerts(self, "localhost")
187+ server_context = ssl.DefaultOpenSSLContextFactory(
188+ certs.server_key_path, certs.server_cert_path)
189+ client_context = context.get_ssl_context(no_verify=True,
190+ hostname="localhost")
191+
192+ yield self.verify_context(server_context, client_context)
193+
194+ def test_no_hostname(self):
195+ """Test that calling without hostname arg raises proper error."""
196+ self.assertRaises(error.CertificateError,
197+ context.get_ssl_context, False)
198+
199+ @defer.inlineCallbacks
200+ def test_fails_certificate(self):
201+ """A wrong certificate is rejected."""
202+ certs = FakeCerts(self, "localhost")
203+ server_context = ssl.DefaultOpenSSLContextFactory(
204+ certs.server_key_path, certs.server_cert_path)
205+ client_context = context.get_ssl_context(no_verify=False,
206+ hostname="localhost")
207+
208+ d = self.verify_context(server_context, client_context)
209+ e = yield self.assertFailure(d, SSL.Error)
210+ self.assertEqual(e[0][0][1], "SSL3_GET_SERVER_CERTIFICATE")
211+
212+ @defer.inlineCallbacks
213+ def test_fails_hostname(self):
214+ """A wrong hostname is rejected."""
215+ certs = FakeCerts(self, "thisiswronghost.net")
216+ server_context = ssl.DefaultOpenSSLContextFactory(
217+ certs.server_key_path, certs.server_cert_path)
218+ self.patch(context, "get_certificates", lambda: [certs.ca_cert])
219+ client_context = context.get_ssl_context(no_verify=False,
220+ hostname="localhost")
221+
222+ d = self.verify_context(server_context, client_context)
223+ e = yield self.assertFailure(d, SSL.Error)
224+ self.assertEqual(e[0][0][1], "SSL3_GET_SERVER_CERTIFICATE")
225+
226+ @defer.inlineCallbacks
227+ def test_matches_all(self):
228+ """A valid certificate passes checks."""
229+ certs = FakeCerts(self, "localhost")
230+ server_context = ssl.DefaultOpenSSLContextFactory(
231+ certs.server_key_path, certs.server_cert_path)
232+ self.patch(context, "get_certificates", lambda: [certs.ca_cert])
233+ client_context = context.get_ssl_context(no_verify=False,
234+ hostname="localhost")
235+
236+ yield self.verify_context(server_context, client_context)
237
238=== modified file 'ubuntuone/storageprotocol/context.py'
239--- ubuntuone/storageprotocol/context.py 2012-03-29 20:28:09 +0000
240+++ ubuntuone/storageprotocol/context.py 2012-06-12 20:02:24 +0000
241@@ -33,7 +33,8 @@
242 import sys
243
244 from OpenSSL import SSL
245-from twisted.internet import ssl
246+from twisted.internet import error, ssl
247+from twisted.python import log
248
249 if sys.platform == "win32":
250 # diable pylint warning, as it may be the wrong platform
251@@ -58,18 +59,52 @@
252 ssl_cert_location = '/etc/ssl/certs'
253
254
255-def get_ssl_context(no_verify):
256- """ Get the ssl context """
257+class HostnameVerifyContextFactory(ssl.CertificateOptions):
258+ """Does hostname checks in addition to certificate checks."""
259+
260+ def __init__(self, hostname, *args, **kwargs):
261+ """Initialize this instance."""
262+ super(HostnameVerifyContextFactory, self).__init__(*args, **kwargs)
263+ self.expected_hostname = hostname
264+
265+ def verify_server_hostname(self, conn, cert, errno, depth, preverifyOK):
266+ """Verify the server hostname."""
267+ if depth == 0:
268+ # No extra checks because U1 certs have the right commonName
269+ if self.expected_hostname != cert.get_subject().commonName:
270+ log.err("Host name does not match certificate. "
271+ "Expected %s but got %s." % (self.expected_hostname,
272+ cert.get_subject().commonName))
273+ return False
274+ return preverifyOK
275+
276+ def getContext(self):
277+ """The context returned will verify the hostname too."""
278+ ctx = super(HostnameVerifyContextFactory, self).getContext()
279+ flags = SSL.VERIFY_PEER | SSL.VERIFY_FAIL_IF_NO_PEER_CERT
280+ ctx.set_verify(flags, self.verify_server_hostname)
281+ return ctx
282+
283+
284+def get_certificates():
285+ """Get a list of certificate paths."""
286+ ca_file = ssl.Certificate.loadPEM(file(os.path.join(ssl_cert_location,
287+ 'UbuntuOne-Go_Daddy_Class_2_CA.pem'), 'r').read())
288+ ca_file_2 = ssl.Certificate.loadPEM(file(os.path.join(ssl_cert_location,
289+ 'UbuntuOne-Go_Daddy_CA.pem'), 'r').read())
290+ ca_file_3 = ssl.Certificate.loadPEM(file(os.path.join(ssl_cert_location,
291+ 'ValiCert_Class_2_VA.pem'), 'r').read())
292+ return [ca_file.original, ca_file_2.original, ca_file_3.original]
293+
294+
295+def get_ssl_context(no_verify, hostname=None):
296+ """Get the ssl context."""
297 if no_verify:
298 ctx = ssl.ClientContextFactory()
299 else:
300- ca_file = ssl.Certificate.loadPEM(file(
301- os.path.join(ssl_cert_location,
302- 'UbuntuOne-Go_Daddy_Class_2_CA.pem'), 'r').read())
303- ca_file_2 = ssl.Certificate.loadPEM(file(
304- os.path.join(ssl_cert_location,
305- 'UbuntuOne-Go_Daddy_CA.pem'), 'r').read())
306- ctx = ssl.CertificateOptions(verify=True,
307- caCerts=[ca_file.original, ca_file_2.original],
308- method=SSL.SSLv23_METHOD)
309+ if hostname is None:
310+ raise error.CertificateError(
311+ 'No hostname specified. Unable to verify SSL certificate.')
312+ ctx = HostnameVerifyContextFactory(hostname, verify=True,
313+ caCerts=get_certificates(), method=SSL.SSLv23_METHOD)
314 return ctx

Subscribers

People subscribed via source and target branches

to all changes: