search

trusteddomainproject / OpenDMARC

This is the Trusted Domain Project's impementation of the DMARC protocol libary and mail filter, called OpenDMARC. A "milter" connects to unix-based mailers (originally, sendmail, but now many) and provides a standard filtering API.
Other
101 stars 53 forks source link

Segmentation fault with 3072-bit key signatures in ARC-Seal headers #183

Open KIC-8462852 opened 3 years ago

KIC-8462852 commented 3 years ago

Hi, I was having some crashes and segfaults with some ARC-Seal headers. Upon investigation, I found that opendmarc crashes when trying to parse invalid ARC-Seal headers like this one:

"ARC-Seal: i=1; none"

opendmarc-arcseal.c:98: opendmarc_arcseal_strip_whitespace: Assertion `string != NULL' failed.
Aborted

This is due to the lack of a guard validation for the NULL value before calling the opendmarc_arcseal_strip_whitespace() function. I fixed this by putting the proper guard for the NULL value before calling the opendmarc_arcseal_strip_whitespace() function.

Another issue with ARC-Seal headers - this time with valid ones - causes opendmarc to crash with a segmentation fault, e.g.:

"ARC-Seal: i=1; a=rsa-sha256; d=example.com; s=default; t=1624542288; cv=none; b=QUJDREVGR0hJS ... 512-byte or more ... XowMTIzNDU2Nz"
Segmentation fault

This is due to signatures produced by keys >= 3072-bit. The 512-byte value defined for "OPENDMARC_ARCSEAL_MAX_TOKEN_LEN" and "OPENDMARC_ARCSEAL_MAX_LONG_VALUE_LEN" is not enough to contain these signatures. When the opendmarc_arcseal_strip_whitespace() function (again) gets a token value with a length greater than this, it returns NULL. Without any guard, this NULL value is then passed to strlcpy function that then segfaults. I fixed this by putting the proper guard for the NULL value before strlcpy.

As IETF RFC 8301 states that verifiers MUST be able to validate signatures with keys ranging from 1024-bit to 4096-bit, I also changed the defines for "OPENDMARC_ARCSEAL_MAX_LONG_VALUE_LEN" and "OPENDMARC_ARCSEAL_MAX_TOKEN_LEN" (this one is used by opendmarc_arcseal_strip_whitespace() function) from 512-byte to 768-byte. This will be enough for signatures produced by 4096-bit keys.

The patch: opendmarc-arcseal.patch.txt

sfsumn commented 3 years ago

I believe we ran into this same issue last week and had to revert opendmarc to a previous version (again), specifically downgrading from version 1.4.1.1 (release 3.el7) back to version 1.3.2 (release 1.el7). I was able to duplicate the problem by including the following headers:

ARC-Seal: i=1; s=TST001; d=umn.edu; t=1628613129; a=rsa-sha256; cv=none; b=mDLsoivuVv5CEgQxP69jJWgeBENSRzydSVl2ZlRJjx9ABqfnVj8CtBf+Eh5F1kEvhRUbnM Q9cpmOIdhWkClTA99VfBSYNnJkhFIjDusPFJ9Y9bmf3uuiwzX51Y+H1sWWrsxDpNUV4pS9 j2Hunygn0J/AyjbAL6Im3IrTBnNC1ezrAYCSJk+qzjiY34sgiC2yadnluCg0ixdabX4JCQ Cv+tGmujSywxFM+DGY4uz+Xl2X1x65jh/msJETdFkLmjxddYTsBFVFs4ib2aD4rMcL88Cn Hz6FDz4SKU8qViAliHePGuZaGKFtLGPUM3q1gCR/RZHjgCUAY9q8RWHdj5hBgan4Tn4KMz AmgCqpLWj91q//Z0LD6DD+4pvtYq0gzPL9QrA81QDR/71TSg9YE46So2oC3M1b4fPzScT1 orWgVFoMOQjI9pamW5Ket5a9Upe+otnLrfU/xGQo/bTnMPvHiT6KVg0KlhbQNjeHAP1wEU og8/jfhuJkIxyyC9XR+S/J From: John Doe John.Doe@umn.edu

When using the above headers in an inbound message (or just these headers alone), opendmarc will crash.

Is the mentioned patch in the EPEL pipeline? This is the second time we've upgraded to a 1.4.1 variant of opendmarc only to downgrade again due to crash issues. The first issue had to do with domain-less addresses in the "From:" header.

KIC-8462852 commented 3 years ago

@Steve-Siirila "Is the patch in the EPEL pipeline?" I don't think so. I posted the patch on Jun 25 and have not received any comment so far. I can only say that the patch solves the issue and that I've been using it in production environment for two months now. Cheers.

sfsumn commented 3 years ago

Can anyone else comment on what the status is for getting this critical patch in the EPEL pipeline? We are having to run an older version of opendmarc in the meantime which presents its own problems. Running the latest version is NOT an option as it simply keep crashing.

sfsumn commented 2 years ago

Is opendmarc being maintained any longer as part of EPEL?

glts commented 2 years ago

@Steve-Siirila This here is the upstream repository. For questions about the EPEL packages better ask the maintainers at the EPEL package repository.

By the way, we have applied this patch in Debian.

sfsumn commented 2 years ago

Thanks for the feedback. I was beginning to think that opendmarc was really not being used enough for anyone to care to apply the fix. Good to hear that this patch at least made it to Debian. We are running Redhat and incorporate EPEL packages so are anxious to see EPEL updated.

abokth commented 2 years ago

I believe this issue has become more urgent recently due to an increased volume of email from important providers with this kind of header.

abokth commented 2 years ago

I made a COPR for Fedora/EPEL at https://copr.fedorainfracloud.org/coprs/abo/opendmarc/ to not burden the Fedora maintainers with pre-release patches.

thegushi commented 1 year ago

Let me check with @mskucherawy and see if it makes sense to apply all of these. They're fairly trivial, but I'd like an overall option to just turn arc-validation off as well, and see if that should be on the radar as well.

andreasschulze commented 1 year ago

I reproduced the issue in a dedicated lab environment

Versions:

OpenARC signing with 1024 and 2048 was no issue for OpenDMARC. But messages signed with 3072/4096 let OpenDMARC crash. I applied the patch mentioned above and then also message with ArcSeal Keys up to 4k pass OpenDMARC as expected.

So, the patch solve the issue (for me)