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.

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

[Symfony/Mailer] support STMP over an unix stream socket #48154

Closed
emmanuel-deloget opened this issue Nov 8, 2022 · 10 comments
Closed

[Symfony/Mailer] support STMP over an unix stream socket #48154

emmanuel-deloget opened this issue Nov 8, 2022 · 10 comments

Comments

@emmanuel-deloget
Copy link

emmanuel-deloget commented Nov 8, 2022

Description

While this looks like an odd use case, unix socket are actually quite useful to add access control to a socket interface (something that cannot be done easily with TCP connexions).

Issues with the current implementation

  • in the Dsn class, parse_url() cannot be used to parse unix:// URI (or anything in the form of scheme:///path/to/something unless scheme === "file") which prevent the use of this function to get information about a unix socket URI. It is quite straigntforward to implement a parse_url() like function which is able to understand the URI.

  • the Dsn class requires a host and unix socket does not have one (it has a path). Some generalization might be needed here

  • the SocketStream class require a host and a port - none of them exist when dealing with a unix domain socket.

Example

Consider this socat tunnel :

socat \
  UNIX-LISTEN:/var/run/mail-socket,user=mail,group=mail,mode=700 \
  OPENSSL:mail.example.com:625,method=TLS1.2,verify=1,cert=/root/cert.pem,cafile=/root/ca.pem

When run as root, this tunnel connects a TLS 1.2 server (with server and client verification ; both the cert and the ca are only readable by root:root) to a unix socket which is only accessible to user mail from group mail.

Using such construct allows an administrator to easily identify and control who is sending a specific mail while still maintaining the security of his platform.

@emmanuel-deloget
Copy link
Author

As a side note, I noticed that Symfony proposed 3 different Dsn classes in its code base:

  • src/Symfony/Component/Translation/Provider/Dsn.php
  • src/Symfony/Component/Mailer/Transport/Dsn.php
  • src/Symfony/Component/Notifier/Transport/Dsn.php

The classes are very similar, with only a handfull of meaningfull differences. May I propose an additional Dsn component that could be reused by Translation, Mailer and Notifier?

@norkunas
Copy link
Contributor

norkunas commented Nov 10, 2022

There was already a suggestion for DSN component in #32546 :)

@emmanuel-deloget
Copy link
Author

It seems this has been mentionned a good number of times, and some even proposed various PR (see #36999 for example) but it also seems that there isn't enough traction to merge this kind of change. Am I right? If this is the case then I'm not sure I'll ever be able to support unix sockets in the mailer :)

@xabbuh
Copy link
Member

xabbuh commented Nov 10, 2022

I fail to see how a potential Dsn component would help implementing the requested feature. To achieve this there is only one Dsn class that would have to be modified, right?

@emmanuel-deloget
Copy link
Author

But does that make any sense since there is already 3 slightly different Dsn classes ? If that's enough, I might be able to put up a PR with the changes needed to implement the functionnality I want to use.

But this feels very wrong to me w.r.t. the general architecture of Symfony.

Since @norkunas pointed me to the #36999 PR, I gave a look to the various discussions about Dsn and there is clearly a need for something here, although I'm not sure what exactly. It might be a Dsn component, or an Uri component, or something that might be a mix between the two. The concept of Dsn exists in many projects and it's curious to me that Symfony did not use its strength in the PHP area to propose a related package that would then be used by other projects.

Packagist shows 13 active library projects with the "dsn" tag (20 packages, 2 abandonned, 5 deal with Delivery Status Notification which is a different beast). The two most used packages were downloaded about 17 million times (the second one, with more tha 5M downloads, is nyholm/dsn which originated from the PR cited above). So there is a clear interest in defining what a DSN is [1] and in using more-or-less standardized code to parse them.

But then, all this discussion only have a meaning if the community is on board. If the community think that it's ok to have multiple separate, specialized Dsn classes that will only grow more and more differrent with each change, then I'm ok with it too :)


[1]: as the name implies, a DSN is a Data(base) Source Name. It was originaly designed to database connection information so that ODBC drivers would be able to locate the real database backend. Using the syntax similar to the ones used in DSN in other contexts is of course of interest, but the resource you try to describe this way is no longer a data source - for example, the DSN strings used in symfony/mailer are in reality service locators, not data source. My mind tells me that even if DSN is a widely known, recognized and used name, it's still incorrect. A better name I would propose is "unified service locator" (USL), a subtype of URI - such a name would then encompass DSN and other strings that encode information in a syntaxically similar way. Describing it this way would help to provide a precise definition that could leverage RFC 3896

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@emmanuel-deloget
Copy link
Author

I (sadly) worked around this by implementing support for unix sockets in an outside package (I had to run over the @internal class definitions). I can provide a basic patch to at least make this easier for the next who would want to try this.

@carsonbot carsonbot removed the Stalled label May 14, 2023
@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@carsonbot
Copy link

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

@carsonbot
Copy link

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

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

No branches or pull requests

4 participants