Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

Fixes SSL certificate verification #49

Merged
merged 1 commit into from
Jan 22, 2017
Merged

Conversation

ares
Copy link
Contributor

@ares ares commented Aug 16, 2016

No description provided.

@mipearson
Copy link
Owner

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with VERIFY_NONE anyway.

Let me think about it.

@ares
Copy link
Contributor Author

ares commented Aug 16, 2016

I started with verify none as default but then realized that implicit
should be secure. I think major bump would be ok, maybe even minor since
it's really a security fix. With update instructions to achieve the
previous behavior it should be fine.

Anyway, leaving up to you.

Odesláno pomocí AquaMail pro Android
http://www.aqua-mail.com

Dne 16. srpna 2016 13:22:56 mipearson [email protected] napsal:

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with VERIFY_NONE
anyway.

Let me think about it.

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#49 (comment)

@mipearson
Copy link
Owner

Ah - if you're the contributor who originally added the VERIFY_NONE then
I'm happy to merge your fix.

On Wed, Aug 17, 2016 at 12:44 AM, Marek Hulán [email protected]
wrote:

I started with verify none as default but then realized that implicit
should be secure. I think major bump would be ok, maybe even minor since
it's really a security fix. With update instructions to achieve the
previous behavior it should be fine.

Anyway, leaving up to you.

Odesláno pomocí AquaMail pro Android
http://www.aqua-mail.com

Dne 16. srpna 2016 13:22:56 mipearson [email protected] napsal:

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with
VERIFY_NONE
anyway.

Let me think about it.

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#49
issuecomment-240074883


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#49 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAF8JXYYGYbnty9GE9PiIXZ_oGXiB6flks5qgczqgaJpZM4JlRyu
.

Michael Pearson

@ares
Copy link
Contributor Author

ares commented Aug 17, 2016

No, I'm not :-) I meant I started this PR with verify none, but then I
changed my mind and sent this version.

Odesláno pomocí AquaMail pro Android
http://www.aqua-mail.com

Dne 16. srpna 2016 11:52:16 PM mipearson [email protected] napsal:

Ah - if you're the contributor who originally added the VERIFY_NONE then
I'm happy to merge your fix.

On Wed, Aug 17, 2016 at 12:44 AM, Marek Hulán [email protected]
wrote:

I started with verify none as default but then realized that implicit
should be secure. I think major bump would be ok, maybe even minor since
it's really a security fix. With update instructions to achieve the
previous behavior it should be fine.

Anyway, leaving up to you.

Odesláno pomocí AquaMail pro Android
http://www.aqua-mail.com

Dne 16. srpna 2016 13:22:56 mipearson [email protected] napsal:

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with
VERIFY_NONE
anyway.

Let me think about it.

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#49
issuecomment-240074883


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#49 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAF8JXYYGYbnty9GE9PiIXZ_oGXiB6flks5qgczqgaJpZM4JlRyu
.

Michael Pearson

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#49 (comment)

@ares
Copy link
Contributor Author

ares commented Sep 22, 2016

any chance you have thought about it?

config.webpack.dev_server.https = false # note - this will use OpenSSL::SSL::VERIFY_NONE
config.webpack.dev_server.https = false
# change below option to OpenSSL::SSL::VERIFY_NONE if you don't care about security
config.webpack.dev_server.https_mode = 'OpenSSL::SSL::VERIFY_PEER'
Copy link
Owner

Choose a reason for hiding this comment

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

Change to config.webpack.dev_server.https_verify_peer, default to true. If true, use VERIFY_PEER otherwise VERIFY_NONE.

@ares
Copy link
Contributor Author

ares commented Sep 30, 2016

Thanks @mipearson, I updated the PR.

@ares
Copy link
Contributor Author

ares commented Oct 12, 2016

Anything else I could do to get this in?

@mipearson
Copy link
Owner

I'm going to be bringing this in to an API-breaking 1.0-pre branch shortly.

The delay on this (and other) pull requests is that I've been cautious to merge anything that could break functionality for existing users.

@ares
Copy link
Contributor Author

ares commented Oct 27, 2016

Sure, that I can definitely understand. Thanks for the update.

@mipearson mipearson merged commit 99f362e into mipearson:master Jan 22, 2017
@mipearson
Copy link
Owner

Merged in 0.9.10, but disabled by default

@ares
Copy link
Contributor Author

ares commented Jan 25, 2017

Thanks you 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants