-
-
Notifications
You must be signed in to change notification settings - Fork 73
Allow set stream context #30
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
Conversation
| private $timeout; | ||
|
|
||
| /** @var $context */ | ||
| private $context; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@var array
There is: http://php.net/manual/en/function.stream-context-set-default.php. So there should be something like |
|
👍 Yikes, I overlooked it in that bunch of PHP functions. |
src/Mail/SmtpMailer.php
Outdated
| $errno, $error, $this->timeout, STREAM_CLIENT_CONNECT, $context | ||
| ); | ||
| if (!$this->connection) { | ||
| if (!$error) $error = error_get_last()['message']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function error_get_last() doesn't return OpenSSL warning here, but...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is missing { ... }
|
@ZZromanZZ Setting |
|
Of course, my primary intention is to allow ignoring verification via However, I wouldn't like see this in README, docs or other tutorials, because this is basically bad practice. Next, why allowing modify only some context options, what if someone needs fine-tune other options , not related ony to SSL/TLS. |
|
Why would anyone conserned about security ever want to disable certificate verification? I don't like the idea with stream_context_get_default either, it's configuration by global environment... |
|
It provides same behaviour as before, thus no BC break if anyone used |
|
Exactly, otherwise it breaks #19 |
|
I have still problem with |
|
@ZZromanZZ Why do you need to ignore verification? Just create your own ca bundle with the certificate you need. For example this is what I use https://gist.github.com/JanTvrdik/9ad909f81d37697e580d3c06d9365852 |
|
@JanTvrdik I am aware of workarounds. And I do not take lightly skipping security checks at all. However, there are circumstances when e.g. certificate is expired, has weak signature, etc... and you do not control server side to replace it. And, of course, show must go on (business must go on). |
|
So I dropped touching SmtpException completely. Is there anything else what can I do ? |
|
And what if you want to bypass default context and have no context options? (I don't now if it is useful, just thinking). It can be done with: /** @var resource */
private $context;
function __constructor()
{
$this->context = isset($options['context']) ? stream_context_create($options['context']) : stream_context_get_default(); |
|
I changed the implementation accordingly, it might be useful in later use cases, when stream context needs to be bypassed. |
|
Great |
|
I just noticed, should not be 'context' also in $defaults array in MailExtension ? |
In current implementation, there is no way to modify stream context parameters.
Which means there is no way to connect (since php 5.6) to SSL/TLS server with self-signed, expired, missmatched CN, etc..
See this problem on forum, for example.
If this is acceptable I would like to discuss possible approaches:
Also there is problem how to catch warnings from underlying Openssl library and provide it to SmtpException as error message.
There is MR, more like experiment, implementation is vague.
We can find inspiration for example in Ruby.