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

Remove EGD #1636

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@Sp1l
Contributor

Sp1l commented Jan 20, 2015

EGD was only necessary for some commercial UNIX systems, versions that
needed
it all reached end of life.
EGD needed until OS release date
IRIX 6.5.19 feb 2003
Solaris 2.6 jul 1997
AIX 5.2 oct 2002
Tru64 5.1B sep 2002
HP-UX 11i v2 sep 2003
https://en.wikipedia.org/wiki//dev/random#EGD_as_an_alternative

Remove EGD
EGD was only necessary for some commercial UNIX systems, versions that
needed
it all reached end of life.
EGD needed until        OS release date
IRIX 6.5.19   feb 2003
Solaris 2.6                     jul 1997
AIX     5.2                     oct 2002
Tru64   5.1B                    sep 2002
HP-UX   11i v2                  sep 2003
https://en.wikipedia.org/wiki//dev/random#EGD_as_an_alternative
@jenkins-cryptography

This comment has been minimized.

jenkins-cryptography commented Jan 20, 2015

Can one of the admins verify this patch?

@public

This comment has been minimized.

Member

public commented Jan 20, 2015

ok to test

@jenkins-cryptography

This comment has been minimized.

jenkins-cryptography commented Jan 20, 2015

Test PASSed.
Refer to this link for build results: https://jenkins.cryptography.io/job/cryptography-pr-experimental/2878/

@public

This comment has been minimized.

Member

public commented Jan 20, 2015

I am OK with this.

@Ayrx

This comment has been minimized.

Contributor

Ayrx commented Jan 20, 2015

This is technically a backwards compatibility break, no?

@coveralls

This comment has been minimized.

coveralls commented Jan 20, 2015

Coverage Status

Coverage remained the same when pulling 7c1874d on Sp1l:master into 7b672ca on pyca:master.

@Sp1l

This comment has been minimized.

Contributor

Sp1l commented Jan 20, 2015

Requires pyOpenSSL to remove EGD as well is what I was told on #dev-cryptography

@public

This comment has been minimized.

Member

public commented Jan 20, 2015

pyca/pyopenssl#187 in particular :)

@Ayrx

This comment has been minimized.

Contributor

Ayrx commented Jan 20, 2015

I'd make a note of it in the CHANGELOG as well.

@Ayrx

This comment has been minimized.

Contributor

Ayrx commented Jan 20, 2015

Otherwise +1 because EGD is completely stupid.

@Sp1l

This comment has been minimized.

Contributor

Sp1l commented Jan 20, 2015

Next point versions of Python won't always have EGD support. EGD support is built conditionally depending on the build system's support of EGD. This already breaks building with LibreSSL. See python/cpython@e3ec962 and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196827 and #928

Sp1l added some commits Jan 20, 2015

Revert "Remove EGD"
This reverts commit 7c1874d.
Make EGD support conditional
EGD was only necessary for some commercial UNIX systems, versions that
needed it all reached end of life.
EGD needed until OS release date
IRIX 6.5.19 feb 2003
Solaris 2.6 jul 1997
AIX 5.2 oct 2002
Tru64 5.1B sep 2002
HP-UX 11i v2 sep 2003
https://en.wikipedia.org/wiki//dev/random#EGD_as_an_alternative
@Sp1l

This comment has been minimized.

Contributor

Sp1l commented Jan 20, 2015

This seems to work as well... Don't know if this a clean way and if it adheres to your coding standards.
Not removing EGD completely still feels like dragging your great-great-grandmother along everywhere you go even though she died 10 years ago...

@jenkins-cryptography

This comment has been minimized.

jenkins-cryptography commented Jan 20, 2015

Test PASSed.
Refer to this link for build results: https://jenkins.cryptography.io/job/cryptography-pr-experimental/2880/

@reaperhulk

This comment has been minimized.

Member

reaperhulk commented Jan 21, 2015

We're going to need to coordinate with pyOpenSSL to determine how we want to proceed here. I'm -1 on merging anything that does conditional binding for Libre inside our OpenSSL backend at this time (I still think we're going to need to fork the backends due to the ENGINE and other changes) but I'm willing to remove these bindings if there's a solution on the pyOpenSSL side.

@public

This comment has been minimized.

Member

public commented Jan 21, 2015

Do we just need PyOpenSSL to implement a shim that returns the right type of error?

@Sp1l

This comment has been minimized.

Contributor

Sp1l commented Jan 21, 2015

pyOpenSSL implements 3 egd functions. I want to try doing an import of EGD from OpenSSL and then make the def egd-function's conditional.
try:
from OpenSSL import RAND_egd
except ImportError:
set somevariable

if not somevariable
def egd( etc.

@reaperhulk

This comment has been minimized.

Member

reaperhulk commented Jan 21, 2015

Presumably you'll want to implement methods in pyOpenSSL such that the absence of EGD will not cause failures for any software that happens to "use" it (It will just noop and raise deprecation warnings).

If that path is blessed by @exarkun then I'm happy to merge a PR that outright removes the bindings (e.g. your first commit on this PR).

@exarkun

This comment has been minimized.

Member

exarkun commented Jan 21, 2015

Presumably you'll want to implement methods in pyOpenSSL such that the absence of EGD will not cause failures for any software that happens to "use" it (It will just noop and raise deprecation warnings).

It would be nice if current versions of pyopenssl didn't suddenly break against the next release of cryptography, though. pyopenssl trustingly declares a >= cryptography dependency - so new installs are going to get the newest cryptography. I'm happy for cryptography to eventually delete these bindings but please wait until there's at least one new release of pyopenssl that doesn't depend on them.

@reaperhulk

This comment has been minimized.

Member

reaperhulk commented Jan 21, 2015

We definitely won't strip these bindings until there's a pyOpenSSL that doesn't require them. Possibly 2 or more releases (depending on your planned 2015 release cadence 😀 )

@public

This comment has been minimized.

Member

public commented Jan 21, 2015

I think a noop random bytes function is probably a bad idea. I think it needs to signal to the caller that EGD isn't available by returning -1.

Alternatively we could implement it such that it actually returns real random data from /dev/urandom but if you were actually using these methods presumably you don't trust your /dev/urandom much.

Fix build with LibreSSL 2.1.3
LibreSSL 2.1.3 contains ALPN already this fixes the definitions conflicting.
@jenkins-cryptography

This comment has been minimized.

jenkins-cryptography commented Jan 24, 2015

Test PASSed.
Refer to this link for build results: https://jenkins.cryptography.io/job/cryptography-pr-experimental/2890/

@reaperhulk

This comment has been minimized.

Member

reaperhulk commented Feb 19, 2015

Closing in favor of #1679.

@reaperhulk reaperhulk closed this Feb 19, 2015

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