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

Implement missing methods #422

Merged
merged 4 commits into from Mar 18, 2016

Conversation

Projects
None yet
5 participants
@hynek
Contributor

hynek commented Feb 13, 2016

First shot at #388.

  • It would be nice if @tiran (or anyone else understand the matter) could suggest a better docstring.
  • If anyone knows what could be checked in the test case I’m all ears. There doesn’t seem to be a get_session_id?
  • We also need to find a way to simulate errors. :-/

N.B. only the -cryptographyMaster envs are supposed to pass. Once this and the other missing methods are signed off, @reaperhulk will release a cryptography 1.2 point release with the necessary bindings.

  • set cryptography requirement to >=1.3 in setup.py once 1.3 is available.
buf,
len(buf),
) != 1:
_raise_current_error()

This comment has been minimized.

@reaperhulk

reaperhulk Feb 13, 2016

Member

You can get around the lack of coverage for this case by making a new function:

def _pyopenssl_assert(ok):
    if not ok:
        _raise_current_error()

then you can change the above to

        res = _lib.SSL_CTX_set_session_id_context(
                self._context,
                buf,
                len(buf),
        )
        _pyopenssl_assert(res == 1)

Coverage on _pyopenssl_assert is, of course, trivial then. This is the approach we use in cryptography when we have response codes we expect to always be a specific value unless something catastrophic has occurred.

Edit: I meant res == 1. Comment updated ;)

This comment has been minimized.

@reaperhulk

reaperhulk Feb 13, 2016

Member

That said, in this case you could trigger the error by passing a session ID context value greater than SSL_MAX_SSL_SESSION_ID_LENGTH, which appears to be 32 (bytes?)

This comment has been minimized.

@hynek

hynek Feb 14, 2016

Contributor

Good idea, done! I’ve taken a, much higher number in the hopes that this number won't change significantly in the next OpenSSL release…

@hynek

This comment has been minimized.

Contributor

hynek commented Feb 14, 2016

(FYI as soon as this is signed off [but no sooner], I’ll tackle the next one. It would be nice tho if someone else could start working on them too.)

@hynek hynek changed the title from [WIP] Implement Context.set_session_id to [WIP] Implement missing methods Feb 27, 2016

@hynek

This comment has been minimized.

Contributor

hynek commented Feb 27, 2016

FYI I’ll fix #387 and #305 here too. It’s cumbersome to work on them separately.

@hynek hynek changed the title from [WIP] Implement missing methods to Implement missing methods Feb 27, 2016

@hynek

This comment has been minimized.

Contributor

hynek commented Feb 27, 2016

Relevant builders are green.

Please tear it apart so we can have a 16.0.0 soon!

@hynek hynek referenced this pull request Feb 28, 2016

Closed

Bindings-only point-release #2715

@hynek hynek added this to the 16.0.0 milestone Feb 28, 2016

hynek added a commit to hynek/pyopenssl that referenced this pull request Mar 11, 2016

Fix set_cipher_list on modern OpenSSL
Also port forward a few changes from pyca#422.
@alex

This comment has been minimized.

Member

alex commented Mar 11, 2016

@hynek this needs a rebase

@hynek hynek force-pushed the hynek:set_session_id branch from 5458d14 to f8dee05 Mar 11, 2016

@hynek

This comment has been minimized.

Contributor

hynek commented Mar 11, 2016

done, all relevant builds are green.

# """
# """
# server, client = self._loopback()
def test_renegotiate(self):

This comment has been minimized.

@reaperhulk

reaperhulk Mar 12, 2016

Member

Should this test call renegotiate_pending during and after to confirm it properly returns True/False at the expected time?

This comment has been minimized.

@hynek

hynek Mar 13, 2016

Contributor

I’ve added some asserts and the builders are still green.

hynek added a commit to hynek/pyopenssl that referenced this pull request Mar 13, 2016

reaperhulk added a commit that referenced this pull request Mar 13, 2016

Merge pull request #442 from hynek/more-422
Pluck more unrelated bits from #422

@hynek hynek force-pushed the hynek:set_session_id branch 3 times, most recently from 40d0843 to 5222c88 Mar 13, 2016

@hynek hynek force-pushed the hynek:set_session_id branch from 0218864 to 947427b Mar 16, 2016

@hynek hynek force-pushed the hynek:set_session_id branch from 947427b to 73412e5 Mar 16, 2016

hynek added some commits Mar 16, 2016

@codecov-io

This comment has been minimized.

codecov-io commented Mar 18, 2016

Current coverage is 87.95%

Merging #422 into master will increase coverage by +0.11% as of 6d16da2

@@            master    #422   diff @@
======================================
  Files            7       7       
  Stmts         2032    2042    +10
  Branches       372     369     -3
  Methods          0       0       
======================================
+ Hit           1785    1796    +11
+ Partial        117     115     -2
- Missed         130     131     +1

Review entire Coverage Diff as of 6d16da2

Powered by Codecov. Updated on successful CI builds.

reaperhulk added a commit that referenced this pull request Mar 18, 2016

Merge pull request #422 from hynek/set_session_id
Implement missing methods

@reaperhulk reaperhulk merged commit 9dff5c4 into pyca:master Mar 18, 2016

2 of 3 checks passed

codecov/patch 82.00% of diff hit (target 100.00%)
Details
codecov/project 87.95% (+0.11%) compared to b98d569 at 87.84%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@glyph

This comment has been minimized.

Contributor

glyph commented Mar 19, 2016

This is amazing and all you guys are awesome.

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Apr 20, 2016

leot
Update security/py-OpenSSL to 16.0.0.
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>`_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment