trusteddomainproject/OpenDMARC

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

KIC-8462852 opened this issue · 10 comments

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

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.

@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.

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.

Is opendmarc being maintained any longer as part of EPEL?

glts commented

@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.

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.

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

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.

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.

I reproduced the issue in a dedicated lab environment

  • sender MTA + OpenARC able to sign with 1024/2048/3072/4096 keys
  • receiver MTA + OpenDMARC

Versions:

  • MTA: a recent postfix - version doesn't matter here ...
  • OpenARC build from trusteddomainproject/OpenARC/tree/develop + trusteddomainproject/OpenARC/pull/121
  • OpenDMARC build from trusteddomainproject/OpenARC/tree/master

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)