On Fri, Nov 08, 2019 at 10:03:55PM +0100, Moviuro wrote:

> Hi all,
> 
> # ecc.key is: (note the EC PARAMETERS object)
> -----BEGIN EC PARAMETERS-----
> ...
> -----END EC PARAMETERS-----
> -----BEGIN EC PRIVATE KEY-----
> ...
> -----END EC PRIVATE KEY-----

> According to the documentation, smtpd_tls_chain_files is the preferred
> way of handling TLS files now, so I use:
> 
> smtpd_tls_chain_files =
>   /usr/local/etc/ssl/mail.domain.tld/rsa.key,
>   /usr/local/etc/ssl/mail.domain.tld/rsa.fullchain.cer

Yes, but that file format is Postfix-specific, and requires that
the key precede its certificate chain, and *only* supports with
keys and associated certificates, and no other objects.

    http://www.postfix.org/postconf.5.html#smtpd_tls_chain_files

The documentation does not mention support for ignoring unrelated
PEM objects.

> Mail log also has an error:
> 
>   Nov  8 21:32:25 mail postfix/smtps/smtpd[3146]: warning:
>     error loading /usr/local/etc/ssl/mail.domain.tld/ecc.key:
>     unexpected PEM type: EC PARAMETERS

This is expected.

> Which looks like it's working correctly. So the EC PARAMETERS object
> breaks an otherwise fine key file for Postfix.

Yes, the chain files are not expected to hold spurious objects.

> With my little understanding of C code, I suspect that fixing this bug
> should be made around lines 407..418 of src/tls/tls_certkey.c [2], to
> silently ignore the EC PARAMETERS object if encountered (though ignoring
> those parameters may have consequences I know nothing of).

For the record, there is no bug, the file format does not support
spurious objects.  This file format is also used for SNI, where all
the PEM blobs are loaded into memory or "compiled" into an indexed
file, and should not really drag along unintended data.

> I could also edit my key file, but as it's valid, it seems weird to do
> it. And because the key was generated by a third-party tool ([0]), more
> people will encounter the issue if they use it too, same as I did to get
> my certificates.

Your key + chain file should be constructed atomically from the
pieces delivered by certbot et. al. and moved (rename(2)) into place
once it is verified to hold a matching key and certificate.

A sensible feature request would be to ask for (or better yet
contribute) a new "postfix tls" script sub-command that creates a
"key + chain" file from a standard PEM file with a key, a standard
PEM file with the cert + optional chain and an optional file with
even more of the chain.  The script would verify that the all bits
are sane, and then combine the (key, cert, chain) triple into a
single file with just the expected objects.

It is of course also possible to change to the code to ignore
unexpected PEM objects in the chain file, perhaps that's less
surprising, but it also leads to surprised if the objects were
supposed to be used, but were not recognized.

So there's a trade-off here, and I could perhaps be persuaded
that a more liberal parser is needed.

-- 
        Viktor.

Reply via email to