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

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

Open
KIC-8462852 opened this issue Jun 25, 2021 · 10 comments
Open
Labels
critical High Priority Fix Neexed

Comments

@KIC-8462852
Copy link

KIC-8462852 commented Jun 25, 2021

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
Copy link

sfsumn commented Aug 16, 2021

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 [email protected]

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
Copy link
Author

KIC-8462852 commented Aug 19, 2021

@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
Copy link

sfsumn commented Sep 21, 2021

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
Copy link

sfsumn commented Jan 18, 2022

Is opendmarc being maintained any longer as part of EPEL?

@glts
Copy link
Contributor

glts commented Jan 18, 2022

@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
Copy link

sfsumn commented Jan 18, 2022

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 added a commit to abokth/OpenDMARC that referenced this issue Jul 5, 2022
abokth added a commit to abokth/OpenDMARC that referenced this issue Jul 6, 2022
abokth added a commit to abokth/OpenDMARC that referenced this issue Jul 6, 2022
@abokth
Copy link

abokth commented Jul 6, 2022

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
Copy link

abokth commented Jul 7, 2022

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
Copy link
Collaborator

thegushi commented Jan 5, 2023

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.

@thegushi thegushi added the critical High Priority Fix Neexed label Jan 5, 2023
@andreasschulze
Copy link

andreasschulze commented Jan 13, 2023

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
critical High Priority Fix Neexed
Projects
None yet
Development

No branches or pull requests

6 participants