Re: dkimsign uses unexpected domain in signature on message from Ubuntu/Postfix relay client

2022-01-27 Thread Martijn van Duren
On Mon, 2022-01-24 at 16:20 +0100, Tim van der Molen wrote:
> Hi,
> 
> Martijn van Duren (2022-01-23 20:13 +0100):
> > >  From: r...@relayclient.example.com (Cron Daemon)
> > 
> > According to RFC5322 section 3.4[0] this is not a valid e-mail format.
> 
> Just to point out this actually is valid. Text in parentheses is a
> comment. See RFC 5322 section 3.2.2. Also this quote from section 3.4
> which describes the format used above:
> 
>   Note: Some legacy implementations used the simple form where the
>   addr-spec appears without the angle brackets, but included the
>   name of the recipient in parentheses as a comment following the
>   addr-spec.  Since the meaning of the information in a comment is
>   unspecified, implementations SHOULD use the full name-addr form of
>   the mailbox, instead of the legacy form, to specify the display
>   name associated with a mailbox.  Also, because some legacy
>   implementations interpret the comment, comments generally SHOULD
>   NOT be used in address fields to avoid confusing such
>   implementations.
> 
> Best,
> Tim

Thanks for pointing this one out, it made me take a closer look at the
spec. So when I originally responded I was only thinking in terms of
what characters are used in a domain name, but the " (Cron Daemon)" part
is not to be interpreted as domain characters, but a CFWS (or comment
folding whitespace). This means that it should not be returned by
osmtpd_mheader_from_domain() as part of the domain. So it is valid
syntax, but I shouldn't have returned it when comparing against the
known domain list. Similarly there were also a couple of FWS that I
could ignore.

As for Paul's remark "SHOULD NOT" when it comes to this syntax: It's
part of the current syntax (e.g. not obsolete), so I don't see any
reason not to. Sorry for the confusion.

I have the following changes lined up in my repo[0]:
- Fix a couple of memory leaks in error paths (pointed out by
  Peter)
- Add support for -D file, where file contains one domain per line. All
  other rules from -d apply. (requested by Mischa and Renaud)
- Fix FWS and CFWS issues when parsing a domain (pointed out by
  Paul/Tim)

If people could help me test the latest code (or even check the diffs
of revision 75-HEAD) that would help prepare for a new release.

martijn@

[0] http://imperialat.at/dev/filter-dkimsign/



Re: dkimsign uses unexpected domain in signature on message from Ubuntu/Postfix relay client

2022-01-27 Thread Paul Pace

On 2022-01-26 19:24, Ryan Kavanagh wrote:

On Wed, Jan 26, 2022 at 12:48:26PM -0800, Paul Pace wrote:

I'm trying to figure by going backwards where in the
Ubuntu/Debian/cron chain of packaging, configuring, compiling, or
coding this format actually comes from but I have a feeling this ends
up being a long goose chase.


Debsources [0] should help you catch the proverbial goose quickly
enough.

I'm guessing that the most likely source is line 507 of do_command.c in
the cron source package [1], unless it's getting set somewhere using
MAILFROM, in which case, likely [2]. There are a few other potential
matches [3].

Best,
Ryan

[0] https://sources.debian.org/
[1] 
https://sources.debian.org/src/cron/3.0pl1-137/do_command.c/?hl=507#L507
[2] 
https://sources.debian.org/src/cron/3.0pl1-137/debian/patches/features/Add-MAILFROM-environment-variable.patch/?hl=125#L125

[3] https://codesearch.debian.net/search?q=package%3Acron+Cron+Daemon

Very helpful!

I know very little about the depths of development work on these kinds 
of tools, and it looks like cron is maintained separately for each 
project, and had been somewhat neglected by Debian. That patch you 
linked was submitted with a bug (isn't that what every dev wishes for?) 
but then sat for three years.


Also learned that Debian uses GitLab for development work on "salsa".

Anyway, as best I can tell that patch made it to a salsa commit[1] and 
maybe it shows up one day in Ubuntu with the following new feature:


+If MAILFROM is defined, the sender email address is set to MAILFROM. 
Otherwise

+mail is sent as "root (Cron Daemon)".

So until then options are to fix it with the local MTA or just live with 
it.


Thank you,


Paul

[1] 
https://salsa.debian.org/debian/cron/-/commit/1d904fb79cfc732cdda0c45800258580c727555b