Change in vdsm[master]: Implement SSL session cache

danken at redhat.com danken at redhat.com
Tue Oct 2 23:16:26 UTC 2012


Dan Kenigsberg has posted comments on this change.

Change subject: Implement SSL session cache
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(6 inline comments)

Thanks for this cool improvement. Few nitpicks and questions inside.j

....................................................
File tests/sslTests.py
Line 87:         simple SSL server.
Line 88:         """
Line 89: 
Line 90:         # Save the key to a file:
Line 91:         tmp = tempfile.NamedTemporaryFile(delete=False)
no need to change anything here, but I like

 with tempfile.NamedTemporaryFile(delete=False) as tmp:

which implicitly calls tmp.close().
Line 92:         tmp.write(KEY)
Line 93:         tmp.close()
Line 94:         self.keyfile = tmp.name
Line 95: 


Line 153:             "-connect", "%s:%d" % ADDRESS,
Line 154:         ]
Line 155:         if args:
Line 156:             command += args
Line 157:         print("command=%s" % str(command))
would you educate me about the nature of these python-3-style prints?

p.s. the str() call is not needed, %s does the Right Thing.
Line 158:         process = subprocess.Popen(command,
Line 159:             stdin=subprocess.PIPE,
Line 160:             stdout=subprocess.PIPE,
Line 161:             stderr=subprocess.PIPE)


Line 217:         connecting two times without stopping the server.
Line 218:         """
Line 219: 
Line 220:         # Create a temporary file to store the session details:
Line 221:         tmp = tempfile.NamedTemporaryFile(delete=False)
I prefer meaningful names, such as sessionDetailsFile.
Line 222: 
Line 223:         # Connect first time and save the session to a file:
Line 224:         rc, out = self.runSClient([
Line 225:             "-cert", self.certfile,


Line 289: """
Line 290: 
Line 291: 
Line 292: # Self signed certificate used for the tests:
Line 293: CERTIFICATE = """
note to future developers: do not freak out when unit test starts failing

      After : Sep 26 17:23:17 2022 GMT

just re-create certificate.
Line 294: -----BEGIN CERTIFICATE-----
Line 295: MIIC8zCCAdugAwIBAgIBADANBgkqhkiG9w0BAQUFADAUMRIwEAYDVQQDDAkxMjcu
Line 296: MC4wLjEwHhcNMTIwOTI4MTcyMzE3WhcNMjIwOTI2MTcyMzE3WjAUMRIwEAYDVQQD
Line 297: DAkxMjcuMC4wLjEwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDapPcH


....................................................
File vdsm/SecureXMLRPCServer.py
Line 1: # Copyright 2008 Red Hat, Inc. and/or its affiliates.
considerable code added. how about adding ",2012"?
Line 2: #
Line 3: # Redistribution and use in source and binary forms, with or without
Line 4: # modification, are permitted provided that the following conditions
Line 5: # are met:


Line 93: 
Line 94:     def accept(self):
Line 95:         # Get the client socket and address calling the accept method of
Line 96:         # the raw socket and replace the client socket with a wrapper:
Line 97:         client, address = self.raw.accept()
am I right that the caching is done here, by providing an existing context, instead of recreating it every time as ssl.SSLSocket()? I could use a bit of elaboration in the commit message regarding "in a way such that SSL sessions are cached".
Line 98:         client = SSLClientSocket(client, self.context)
Line 99:         return client, address
Line 100: 
Line 101: 


--
To view, visit http://gerrit.ovirt.org/8123
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic75adee4070b415b8855af1f2ea289825496fbc1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernandez at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernandez at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yaniv Kaul <ykaul at redhat.com>


More information about the vdsm-patches mailing list