Skip to content

[Mailer] stream_socket_enable_crypto php warning about crypto type #31105

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
Closed
plandolt opened this issue Apr 14, 2019 · 6 comments
Closed

[Mailer] stream_socket_enable_crypto php warning about crypto type #31105

plandolt opened this issue Apr 14, 2019 · 6 comments
Labels
Mailer

Comments

@plandolt
Copy link
Contributor

plandolt commented Apr 14, 2019 •
edited
Loading

Symfony version(s) affected:
4.3-dev

Description
When using the EsmtpTransport transport with tls encryption I receive a PHP warning:

PHP Warning:  stream_socket_enable_crypto(): When enabling encryption you must specify the crypto type in .../vendor/symfony/mailer/Transport/Smtp/Stream/SocketStream.php on line 156

How to reproduce

$transport = (new EsmtpTransport('example.com', 587, 'tls'))
    ->setUsername('...')
    ->setPassword('...');

Possible Solution
Honestly I have no clue on how stream_socket_enable_crypto should work, sorry.
@plandolt plandolt changed the title stream_socket_enable_crypto php warning about crypto type [Mailer] stream_socket_enable_crypto php warning about crypto type Apr 14, 2019
@plandolt
Copy link
Contributor Author

After reading the documentation and doing a test the third parameter should be STREAM_CRYPTO_METHOD_ANY_CLIENT.

@sstok
Copy link
Contributor

sstok commented May 26, 2019

Some background:

https://wiki.php.net/rfc/improved-tls-defaults
zendframework/zend-http#105

New Constants

STREAM_CRYPTO_METHOD_ANY_CLIENT

Crypto method interpreted as “any client crypto method we can possibly support.” Applications may use this method for maximum compatibility with SSLv2, SSLv3, TLSv1, TLSv1.1 and TLSv1.2 servers.

STREAM_CRYPTO_METHOD_ANY_SERVER

Crypto method interpreted as “any server crypto method we can possibly support.” Applications may use this method for maximum compatibility with SSLv2, SSLv3, TLSv1, TLSv1.1 and TLSv1.2 clients.

STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT

Crypto method flag allowing specific TLSv1 usage in encrypted client streams.

STREAM_CRYPTO_METHOD_TLSv1_0_SERVER

Crypto method flag allowing specific TLSv1 usage in encrypted server streams.

STREAM_CRYPTO_METHOD_ANY_CLIENT basically accepts any client which is extreme risky as this makes it possible to perform downgrade attacks.

Better to use STREAM_CRYPTO_METHOD_TLS_CLIENT or even STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT (which however may not be supported everywhere).

@fabpot
Copy link
Member

fabpot commented May 28, 2019

My reasoning for not passing the third parameter is that it allows one to configure it from the outside via the SSL context options.

@bpolaszek
Copy link
Contributor

@fabpot it looks like the 3rd parameter is mandatory as soon as $enabled is true, or an ErrorException will be raised because of the warning, and the message won't be sent.

I'm not an expert at this level but is there a way to ensure it's properly configured from the outside before calling the function?

@fabpot
Copy link
Member

fabpot commented Jun 4, 2019

@bpolaszek Looks like you're right. Let's find the "best" option here. Would you like to work on a PR for this?

@bpolaszek bpolaszek mentioned this issue Jun 4, 2019
Merged
@bpolaszek
Copy link
Contributor

@fabpot something like this?

@fabpot fabpot closed this as completed Jun 5, 2019
fabpot added a commit that referenced this issue Jun 5, 2019
@fabpot
bug #31846 [Mailer] Set default crypto method (bpolaszek)
036d7b6 
This PR was merged into the 4.3 branch.

Discussion
----------

[Mailer] Set default crypto method

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #31105
| License | MIT
| Doc PR | -

This PR fixes #31105 by providing `STREAM_CRYPTO_METHOD_TLS_CLIENT | STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT | STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT` as  default crypto method when none is defined in user options and TLS is enabled.

Commits
-------

4f0ad25 Set default crypto method - Fix #31105
fabpot added a commit that referenced this issue Jun 5, 2019
@fabpot
Merge branch '4.3' into 4.4
7207849 
* 4.3:
  [Console] Add check for Konsole/Yakuake to disable hyperlinks
  [HttpClient] work around PHP 7.3 bug related to json_encode()
  [VarDumper] fix dumping the cloner itself
  Rename the Symfony Mailer service config to avoid conflict with SwitMailer
  Set default crypto method - Fix #31105
  [Form] add missing symfony/service-contracts dependency
  [HttpClient] Don't throw InvalidArgumentException on bad Location header
fabpot added a commit that referenced this issue Jun 5, 2019
@fabpot
Merge branch '4.4'
d2909ca 
* 4.4:
  [Console] Add check for Konsole/Yakuake to disable hyperlinks
  [HTTP Foundation] Deprecate passing argument to method Request::isMethodSafe()
  [HttpClient] work around PHP 7.3 bug related to json_encode()
  [VarDumper] fix dumping the cloner itself
  Rename the Symfony Mailer service config to avoid conflict with SwitMailer
  Set default crypto method - Fix #31105
  [Form] add missing symfony/service-contracts dependency
  [HttpClient] Don't throw InvalidArgumentException on bad Location header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Mailer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @javiereguiluz @sstok @plandolt @bpolaszek