Change in vdsm[master]: vdsm-reg: use web server CA extracted from SSL handshake

Alon Bar-Lev alonbl at redhat.com
Sat Oct 6 19:00:38 UTC 2012


Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-reg: use web server CA extracted from SSL handshake
......................................................................


Patch Set 1: (6 inline comments)

> I also have to say that I don't really see the benefit (from the security point of view) of downloading the CA certificate and then using it to check latter HTTPS connections. As we don't do any verification on the CA certificate we could well ignore the CA certificate, right?

No. If user approves the certificate hash one time, this CA becomes the trust anchor for future operations.

....................................................
File vdsm_reg/deployUtil.py.in
Line 1529:         if os.path.exists(engineWebCACert):
Line 1530:             ovirtfunctions.ovirt_safe_delete_config(engineWebCACert)
Line 1531: 
Line 1532: def getChainFromSSL(host):
Line 1533:     chain = {}
Done
Line 1534: 
Line 1535:     def _getChainFromSSL_Verify(conn, cert, errnum, depth, ok):
Line 1536:         chain[depth] = OpenSSL.crypto.dump_certificate(
Line 1537:             OpenSSL.crypto.FILETYPE_PEM,


Line 1531: 
Line 1532: def getChainFromSSL(host):
Line 1533:     chain = {}
Line 1534: 
Line 1535:     def _getChainFromSSL_Verify(conn, cert, errnum, depth, ok):
Done
Line 1536:         chain[depth] = OpenSSL.crypto.dump_certificate(
Line 1537:             OpenSSL.crypto.FILETYPE_PEM,
Line 1538:             cert
Line 1539:         ).decode("utf-8")


Line 1535:     def _getChainFromSSL_Verify(conn, cert, errnum, depth, ok):
Line 1536:         chain[depth] = OpenSSL.crypto.dump_certificate(
Line 1537:             OpenSSL.crypto.FILETYPE_PEM,
Line 1538:             cert
Line 1539:         ).decode("utf-8")
These are not separate objects embedded this time. I prefer it this way.
Line 1540:         return True
Line 1541: 
Line 1542:     ctx = OpenSSL.SSL.Context(OpenSSL.SSL.SSLv23_METHOD)
Line 1543:     ctx.set_verify(OpenSSL.SSL.VERIFY_PEER, _getChainFromSSL_Verify)


Line 1540:         return True
Line 1541: 
Line 1542:     ctx = OpenSSL.SSL.Context(OpenSSL.SSL.SSLv23_METHOD)
Line 1543:     ctx.set_verify(OpenSSL.SSL.VERIFY_PEER, _getChainFromSSL_Verify)
Line 1544:     ctx.set_verify_depth(10)
Done
Line 1545:     sock = OpenSSL.SSL.Connection(
Line 1546:         ctx,
Line 1547:         socket.socket(socket.AF_INET, socket.SOCK_STREAM)
Line 1548:     )


Line 1544:     ctx.set_verify_depth(10)
Line 1545:     sock = OpenSSL.SSL.Connection(
Line 1546:         ctx,
Line 1547:         socket.socket(socket.AF_INET, socket.SOCK_STREAM)
Line 1548:     )
Because it is not used in any statement and it wrapped.

There is no reason to make code more complex and be creating in inventing temporary variable names just for the sake of variable... it does not make code simpler, it just make it more complex.
Line 1549:     sock.connect(host)
Line 1550:     sock.do_handshake()
Line 1551:     sock.shutdown()
Line 1552:     sock.close()


Line 1550:     sock.do_handshake()
Line 1551:     sock.shutdown()
Line 1552:     sock.close()
Line 1553: 
Line 1554:     # elimintate duplicates
Done
Line 1555:     return [chain[depth] for depth in sorted(chain.keys())]
Line 1556: 
Line 1557: def getRhevmCert(IP, port):
Line 1558:     """Aquire CA certificate from SSL handshake


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab8727a167de19ac66712309868654ae00c9bf4d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernandez at redhat.com>


More information about the vdsm-patches mailing list