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.

Sign up for GitHub

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

Jump to bottom

Compatibility for OpenSSL >= 1.0.1i #193

Merged
merged 3 commits into from
Apr 15, 2015
Merged

Compatibility for OpenSSL >= 1.0.1i #193

merged 3 commits into from
Apr 15, 2015

Conversation

mrjefftang
Copy link
Contributor

ASN1_UTCTIME_cmp_time_t is returning -2 on OpenSSL 1.0.2

Addresses #192

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling ac77424 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling ac77424 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling ac77424 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling ac77424 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling ac77424 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.16%) to 94.67% when pulling bdedee7 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.16%) to 94.67% when pulling bdedee7 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@mrjefftang
Copy link
Contributor Author

Looks like TravisCI isn't accepting new requests for OS X test builds. I'm not sure how to get this tested as there's no Ubuntu OpenSSL 1.0.2 package either.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 9b20b34 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 9b20b34 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 9b20b34 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 1ef667f on mrjefftang:has_expired_bug into 496f40d on pyca:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 1ef667f on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 96c5e1b on mrjefftang:has_expired_bug into 496f40d on pyca:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 96c5e1b on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 96c5e1b on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 96c5e1b on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@mrjefftang mrjefftang mentioned this pull request Feb 17, 2015
Closed
@mrjefftang mrjefftang changed the title Perform the time comparison in python to fix #192 OpenSSL 1.0.2 Compatibility Feb 17, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 4bef4eb on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@mrjefftang
Copy link
Contributor Author

Final error arises from test_set_tmp_ecdh when it calls context.set_tmp_ecdh(curve) for EC curves Oakley-EC2N-4 and Oakley-EC2N-3.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 6a78aa2 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 94.52% when pulling 6a78aa2 on mrjefftang:has_expired_bug into 496f40d on pyca:master.

@mrjefftang
Copy link
Contributor Author

@exarkun It's not clear to me why the Context.set_tmp_ecdh() call for Oakley-EC2N-4 and Oakley-EC2N-3 cause an OpenSSL exception but it looks like it's also not being raised properly either since it's the TestCase.tearDown() cleanup that is discovering the exception.

@peterpramb peterpramb mentioned this pull request Feb 18, 2015
Closed
@mrjefftang mrjefftang changed the title OpenSSL 1.0.2 Compatibility Compatibility for OpenSSL >= 1.0.1i Feb 18, 2015
@mrjefftang mrjefftang mentioned this pull request Apr 14, 2015
Closed
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.2% when pulling 93af95d on mrjefftang:has_expired_bug into 468bd42 on pyca:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.2% when pulling 93af95d on mrjefftang:has_expired_bug into 468bd42 on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.2% when pulling 93af95d on mrjefftang:has_expired_bug into 468bd42 on pyca:master.

@reaperhulk reaperhulk added this to the 0.16 milestone Apr 14, 2015
@reaperhulk
Copy link
Member

@mrjefftang could we get you to rebase this now? Thanks for your work so far!

@mrjefftang
OpenSSL 1.0.2 Compatibility
fc18f7b 
- Perform the time comparison in python to fix #192
- Add root cert has_expired test
- Self sign test cert to fix issue in #149
- Change test case to verify digest of a valid certficate
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.21% when pulling fc18f7b on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.21% when pulling fc18f7b on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.21% when pulling fc18f7b on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.21% when pulling fc18f7b on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@mrjefftang
Copy link
Contributor Author

@reaperhulk Done.

hynek
hynek reviewed Apr 15, 2015
View reviewed changes
OpenSSL/crypto.py
notAfter = _lib.X509_get_notAfter(self._x509)
return _lib.ASN1_UTCTIME_cmp_time_t(
_ffi.cast('ASN1_UTCTIME*', notAfter), now) < 0
notAfter = self.get_notAfter().decode('utf-8')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a discussion with Paul I think we should not do that. pyOpenSSL should offer as little abstraction as necessary and since this bug has been fixed in 1.0.2a, we shouldn’t add baggage to our codebase.

@hynek
Copy link
Contributor

hynek commented Apr 15, 2015

Thanks for this and your responsiveness! Please revert the one change and we’re good to go if the CI stays green. :)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.21% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.21% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.21% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.21% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.21% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.36%) to 92.86% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.36%) to 92.86% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.36%) to 92.86% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.36%) to 92.86% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.36%) to 92.86% when pulling 05fe9fa on mrjefftang:has_expired_bug into fa0a04b on pyca:master.

@mrjefftang
Copy link
Contributor Author

@hynek Done

@reaperhulk
Copy link
Member

The fix that will stop the notAfter bug has not yet shipped (should be in 1.0.2b actually), but we'll merge this now since it shows the issue. Thanks again!

reaperhulk added a commit that referenced this pull request Apr 15, 2015
@reaperhulk
Merge pull request #193 from mrjefftang/has_expired_bug
cf7ede8 
Compatibility for OpenSSL >= 1.0.1i
@reaperhulk reaperhulk merged commit cf7ede8 into pyca:master Apr 15, 2015
@hynek
Copy link
Contributor

hynek commented Apr 16, 2015

Thanks to all involved and sorry for the delays!

@mrjefftang mrjefftang deleted the has_expired_bug branch April 16, 2015 11:52
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
16.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@mrjefftang @coveralls @reaperhulk @hynek