Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SSL_peek #294

Merged
merged 1 commit into from Aug 18, 2015
Merged

Add support for SSL_peek #294

merged 1 commit into from Aug 18, 2015

Conversation

@mhils
Copy link
Contributor

@mhils mhils commented Jul 25, 2015

This PR adds support for OpenSSL's SSL_peek command, which is OpenSSL's (undocumented) equivalent of MSG_PEEK for normal sockets. I went for the straightforward way and mirrored the Python socket API - if you prefer a separate peek method, I'm happy to adjust the PR.
Tests may fail for now, as this requires the next cryptography release.

@codecov-io
Copy link

@codecov-io codecov-io commented Jul 26, 2015

Current coverage is 92.56%

Merging #294 into master will increase coverage by +0.02% as of 9727e1b

@@            master    #294   diff @@
======================================
  Files           14      14       
  Stmts         5347    5365    +18
  Branches       483     486     +3
  Methods          0       0       
======================================
+ Hit           4948    4966    +18
  Partial        157     157       
  Missed         242     242       

Review entire Coverage Diff as of 9727e1b

Powered by Codecov. Updated on successful CI builds.

@hynek
Copy link
Contributor

@hynek hynek commented Jul 26, 2015

Since pyOpenSSL mostly attempts to mirror OpenSSL APIs, I think the approach is okay. We’ll have to wait for the next cryptography release then I guess.

@mhils
Copy link
Contributor Author

@mhils mhils commented Aug 12, 2015

I cannot restart the tests, but I think we're good to go now. 😃

"""
server, client = self._loopback()
server.send(b('xy'))
self.assertEquals(client.recv(2, MSG_PEEK), b('xy'))

This comment has been minimized.

@hynek

hynek Aug 13, 2015
Contributor

please use assertEqual (no s) in new code, this variant is deprecated

@hynek
Copy link
Contributor

@hynek hynek commented Aug 13, 2015

sadly our pypy-builder is broken now :-/

meanwhile:

  • you’ll have to update doc/api/ssl.rst (sadly Connection isn’t on autodoc yet)
  • add a changelog entry
:return: The string read from the Connection
"""
buf = _ffi.new("char[]", bufsiz)
result = _lib.SSL_read(self._ssl, buf, bufsiz)
if flags and flags & socket.MSG_PEEK:

This comment has been minimized.

@hynek

hynek Aug 13, 2015
Contributor

please use flags is not None

@@ -2171,6 +2171,16 @@ def test_pending_wrong_args(self):
connection = Connection(Context(TLSv1_METHOD), None)
self.assertRaises(TypeError, connection.pending, None)

def test_peek(self):
"""
:py:obj:`Connection.recv` peeks into the connect if :py:obj:`socket.MSG_PEEK` is passed.

This comment has been minimized.

@hynek

hynek Aug 13, 2015
Contributor

what is a connect? :)

@hynek
Copy link
Contributor

@hynek hynek commented Aug 13, 2015

you probably also should rebase while you’re at it, thanks :)

@mhils mhils force-pushed the mhils:ssl_peek branch from 24093e7 to c8cfb99 Aug 17, 2015
@mhils
Copy link
Contributor Author

@mhils mhils commented Aug 17, 2015

Thanks for the feedback. I fixed alle the issues mentioned above and also added MSG_PEEK support to recv_into. 😃

ChangeLog Outdated
@@ -1,3 +1,8 @@
2015-08-17 Maximilian Hils <pyopenssl@maximilianhils.com>

* OpenSSL/SSL.py, : Add support for the ``MSG_PEEK`` flag to

This comment has been minimized.

@hynek

hynek Aug 17, 2015
Contributor

while we have shit terrible changlog format you’ll have to add OpenSSL/test/test_ssl.py here too (and no trailing comma please, it slid in the previous entry, feel free to remove it :))

This comment has been minimized.

@mhils

mhils Aug 17, 2015
Author Contributor

Fixed.

@mhils mhils force-pushed the mhils:ssl_peek branch from c8cfb99 to 1080568 Aug 17, 2015
@hynek
Copy link
Contributor

@hynek hynek commented Aug 17, 2015

Thanks, I’ll wait for #298 to be merged now. As soon as we have a green Travis again, please rebase and we’ll finally be able to merge. Sorry for the inconvenience.

@@ -2998,6 +3008,15 @@ def test_bytearray_really_doesnt_overfill(self):
"""
self._doesnt_overfill_test(bytearray)

def test_peek(self):

This comment has been minimized.

@hynek

hynek Aug 17, 2015
Contributor

oh and while we’re waiting, pyOpenSSL has sadly the gross code convention to leave two lines between methods on a class, so you’ll have to fix it here and in the read tests too. I’m sorry, I really wish it hadn’t.

This comment has been minimized.

@mhils

mhils Aug 17, 2015
Author Contributor

Alright. 😉
Fixed.

@mhils mhils force-pushed the mhils:ssl_peek branch from 1080568 to 84d1511 Aug 17, 2015
@hynek
Copy link
Contributor

@hynek hynek commented Aug 17, 2015

Rebase pls and report back. :)

@mhils mhils force-pushed the mhils:ssl_peek branch from 84d1511 to 1d95dea Aug 17, 2015
@mhils
Copy link
Contributor Author

@mhils mhils commented Aug 17, 2015

Here you go. 😉

hynek added a commit that referenced this pull request Aug 18, 2015
Add support for SSL_peek
@hynek hynek merged commit 2db996e into pyca:master Aug 18, 2015
2 checks passed
2 checks passed
codecov/commit 100.00% (target 100.00%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hynek
Copy link
Contributor

@hynek hynek commented Aug 18, 2015

thanks for trooping through :)

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Apr 20, 2016
Changes:
16.0.0 (2016-03-19)
-------------------
This is the first release under full stewardship of PyCA.
We have made *many* changes to make local development more pleasing.
The test suite now passes both on Linux and OS X with OpenSSL 0.9.8,
1.0.1, and 1.0.2.  It has been moved to `py.test <https://pytest.org/>`_,
all CI test runs are part of `tox <https://testrun.org/tox/>`_ and
the source code has been made fully `flake8
<https://flake8.readthedocs.org/>`_ compliant.

We hope to have lowered the barrier for contributions significantly
but are open to hear about any remaining frustrations.

Backward-incompatible changes:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
- Python 3.2 support has been dropped.
  It never had significant real world usage and has been dropped
  by our main dependency ``cryptography``.  Affected users should
  upgrade to Python 3.3 or later.

Deprecations:
^^^^^^^^^^^^^
- The support for EGD has been removed.
  The only affected function ``OpenSSL.rand.egd()`` now uses
  ``os.urandom()`` to seed the internal PRNG instead.  Please see
  `pyca/cryptography#1636
  <https://github.com/pyca/cryptography/pull/1636>`_ for more
  background information on this decision.  In accordance with our
  backward compatibility policy ``OpenSSL.rand.egd()`` will be
  *removed* no sooner than a year from the release of 16.0.0.
  Please note that you should `use urandom
  <http://sockpuppet.org/blog/2014/02/25/safely-generate-random-numbers/>`_
  for all your secure random number needs.
- Python 2.6 support has been deprecated.
  Our main dependency ``cryptography`` deprecated 2.6 in version
  0.9 (2015-05-14) with no time table for actually dropping it.
  pyOpenSSL will drop Python 2.6 support once ``cryptography``
  does.

Changes:
^^^^^^^^
- Fixed ``OpenSSL.SSL.Context.set_session_id``,
  ``OpenSSL.SSL.Connection.renegotiate``,
  ``OpenSSL.SSL.Connection.renegotiate_pending``, and
  ``OpenSSL.SSL.Context.load_client_ca``.
  They were lacking an implementation since 0.14.  `#422
  <https://github.com/pyca/pyopenssl/pull/422>`_
- Fixed segmentation fault when using keys larger than 4096-bit to sign data.
  `#428 <https://github.com/pyca/pyopenssl/pull/428>`_
- Fixed ``AttributeError`` when ``OpenSSL.SSL.Connection.get_app_data()``
  was called before setting any app data.
  `#304 <https://github.com/pyca/pyopenssl/pull/304>`_
- Added ``OpenSSL.crypto.dump_publickey()`` to dump ``OpenSSL.crypto.PKey``
  objects that represent public keys, and ``OpenSSL.crypto.load_publickey()``
  to load such objects from serialized representations.
  `#382 <https://github.com/pyca/pyopenssl/pull/382>`_
- Added ``OpenSSL.crypto.dump_crl()`` to dump a certificate revocation
  list out to a string buffer.
  `#368 <https://github.com/pyca/pyopenssl/pull/368>`_
- Added ``OpenSSL.SSL.Connection.get_state_string()`` using the
  OpenSSL binding ``state_string_long``.
  `#358 <https://github.com/pyca/pyopenssl/pull/358>`_
- Added support for the ``socket.MSG_PEEK`` flag to
  ``OpenSSL.SSL.Connection.recv()`` and
  ``OpenSSL.SSL.Connection.recv_into()``.
  `#294 <https://github.com/pyca/pyopenssl/pull/294>`_
- Added ``OpenSSL.SSL.Connection.get_protocol_version()`` and
  ``OpenSSL.SSL.Connection.get_protocol_version_name()``.
  `#244 <https://github.com/pyca/pyopenssl/pull/244>`_
- Switched to ``utf8string`` mask by default.
  OpenSSL formerly defaulted to a ``T61String`` if there were UTF-8
  characters present.  This was changed to default to ``UTF8String``
  in the config around 2005, but the actual code didn't change it
  until late last year.  This will default us to the setting that
  actually works.  To revert this you can call
  ``OpenSSL.crypto._lib.ASN1_STRING_set_default_mask_asc(b"default")``.
  `#234 <https://github.com/pyca/pyopenssl/pull/234>`_
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants