This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: ssl: add set_msg_callback function
Type: enhancement Stage: resolved
Components: SSL Versions: Python 3.7
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: chris.jerdonek, christian.heimes, jcea, pitrou, tweksteen
Priority: normal Keywords: patch

Created on 2012-07-27 01:34 by tweksteen, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
ssl_msg_callback.patch tweksteen, 2012-07-27 01:34 set_msg_callback.patch review
ssl_msg_callback-0.2.patch tweksteen, 2012-07-30 00:10 review
Messages (11)
msg166532 - (view) Author: Thiébaud Weksteen (tweksteen) Date: 2012-07-27 01:34
I wrote a patch for Python 3 to expose the function
SSL_CTX_set_msg_callback in the module ssl.

Here is a description of this function:
"SSL_CTX_set_msg_callback() or SSL_set_msg_callback() can be used
to define a message callback function cb for observing all SSL/TLS
protocol messages (such as handshake messages) that are received or sent."

There is also a test case included in the patch.

Comments are welcomed.
msg166537 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-27 03:12
In your test, is there a reason you don't need to verify that your callback is actually called?

+            def cb(packet):
+              self.assertGreater(len(packet), 0)
+            ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
+            ctx.set_msg_callback(cb)
msg166538 - (view) Author: Thiébaud Weksteen (tweksteen) Date: 2012-07-27 03:57
I'm not sure what would be the best way to verify that. 
What about:

    def test_connect_with_msg_callback(self):
        with support.transient_internet("svn.python.org"):
            self.called = False
            def cb(packet):
                self.assertGreater(len(packet), 0)
                self.called = True
            ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
            ctx.set_msg_callback(cb)
            s = ctx.wrap_socket(socket.socket(socket.AF_INET))
            try:
                s.connect(("svn.python.org", 443))
                self.assertTrue(self.called)
            finally:
                s.close()
msg166591 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-27 17:46
That is one way to do it.  But it would be better to use a local variable rather than an attribute of the class (because otherwise you have to worry about resetting the value if more than one test uses the same pattern).  Something like this would be better

called = []
def cb(packet):
    called.append(1)
msg166837 - (view) Author: Thiébaud Weksteen (tweksteen) Date: 2012-07-30 00:10
I've updated the patch with this method of testing.
msg166957 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-31 03:59
I haven't commented on the content of your patch (I'm not knowledgeable enough in this area), but as for your test, I noticed that you removed the part that asserted something about the argument passed to cb().

Ideally when testing a callback, you want to test whatever is important about it, like: that it gets called at the right times (and as many times as it should), and that it is getting passed the right data.

So you might also want to make whatever assertions are appropriate about the packet(s) passed to cb().
msg167344 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-03 19:12
Thiébaud, I haven't reviewed the patch in detail, but why does the callback only receive the buffer contents? At the minimum, I think it should also receive write_p (whether the packet is an incoming or outgoing message is a rather useful piece of information).
msg167555 - (view) Author: Thiébaud Weksteen (tweksteen) Date: 2012-08-06 12:24
When I wrote this patch, I was focusing on a particular usage and the buffer was the only parameter that interested me. But you're right, the other parameters should be included. Which brings the following questions:

* write_p looks like a boolean, would it be appropriate to make it like that? Or keep it integer?
* version can be SSL2_VERSION , SSL3_VERSION or TLS1_VERSION. However, these constants are not used yet in _ssl. Should they be mapped to the current ones (with the tricky exception of PROTOCOL_SSLv23)?
* content_type could just be passed as a regular integer.

Thanks
msg203173 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-17 14:27
The patch won't be ready for 3.4 beta1 next weekend. Deferring to 3.5
msg301484 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-09-06 16:59
I'm reluctant to add new features to the ssl module unless they increase security. The message callback is a debugging hook to analyse handshake and other low level parts of the protocol.
msg301487 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-09-06 17:20
I talked with other ssl module maintainers. We agreed that the debug callback is out of scope for Python stdlib. The ssl does not try to be an all-encompassing wrapper of OpenSSL.
History
Date User Action Args
2022-04-11 14:57:33adminsetgithub: 59669
2017-09-06 17:20:43christian.heimessetstatus: open -> closed
resolution: rejected
messages: + msg301487

stage: patch review -> resolved
2017-09-06 16:59:27christian.heimessetassignee: christian.heimes ->
messages: + msg301484
components: - Extension Modules
2016-09-15 07:53:21christian.heimessetassignee: christian.heimes
components: + SSL
versions: - Python 3.6
2016-09-08 15:42:10christian.heimessetversions: + Python 3.6, Python 3.7, - Python 3.5
2016-06-12 11:25:12christian.heimessetassignee: christian.heimes -> (no value)
2013-11-17 14:27:48christian.heimessetmessages: + msg203173
versions: + Python 3.5, - Python 3.4
2013-08-14 11:25:49christian.heimessetassignee: christian.heimes
2013-06-14 13:17:56christian.heimessetnosy: + christian.heimes
2012-10-04 10:57:13jceasetnosy: + jcea
2012-08-06 12:24:40tweksteensetmessages: + msg167555
2012-08-03 19:12:40pitrousetmessages: + msg167344
stage: patch review
2012-07-31 03:59:15chris.jerdoneksetmessages: + msg166957
2012-07-30 00:10:23tweksteensetfiles: + ssl_msg_callback-0.2.patch

messages: + msg166837
2012-07-27 17:46:37chris.jerdoneksetmessages: + msg166591
2012-07-27 03:57:30tweksteensetmessages: + msg166538
2012-07-27 03:12:09chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg166537
2012-07-27 02:35:06r.david.murraysetnosy: + pitrou
2012-07-27 01:34:16tweksteencreate