feat: RFC 9990 generator/pass-disposition + RFC 9989 obsolete-tag cleanup

DMARCBIS-REMAINING-WORK.md

@@ -20,9 +20,50 @@ specifically (all resolved) and are not duplicated here.
 - **RFC 9989 `t=`/`pct=`**: `t=` parsing, fetch accessor, and enforcement
   step-down (reject->quarantine->none). `pct=` deliberately kept for POLA,
   with `DMARCbisIgnorePct` for operators who want strict compliance. (#434)
+- **`!NNNk` RUA size-suffix syntax**: RFC 9989's ABNF marks this obsolete
+  (`obs-dmarc-uri`/`obs-dmarc-report-size`) and says reporters MUST ignore
+  it. Unlike the `pct=` POLA call above, `opendmarc-reports` now drops the
+  enforcement PR #392 had restored -- the suffix is still stripped from
+  the destination address so delivery isn't broken, but it no longer
+  overrides `report_maxbytes`.
 - **RFC 9990 aggregate reporting**: `np`/`testing`/`discovery_method` in
   `policy_published`, namespace bumped to `dmarc-2.0`, `<pct>` removed
   (branch `feat/rfc9990-aggregate-reporting`).
+- **RFC 9990 `generator` element** (S3.1.1.3): `report_metadata` now
+  includes `<generator>` identifying the report-generating software
+  (`$progname v$version`), matching the string already used for
+  `--version` output and the `X-Mailer` header. (Unrelated: the separate
+  `contrib/dmarc-report-totext.pl` *consumer* tool already parses this
+  field from other senders' reports — that's reading, not writing.)
+- **`rf=`/`ri=` cleanup**: both tags were removed from the DMARC record
+  format by RFC 9989. Removed the parsing (`opendmarc_policy.c`), the
+  now-dead `DMARC_POLICY_T` fields (`rf`, `ri`), the orphaned
+  `opendmarc_policy_fetch_rf()` (never declared in the public header, so
+  unreachable by consumers anyway) and its stale doc page, and the
+  `RF=`/`RI=` lines from `opendmarc_policy_to_buf()`'s dump. Unrecognized
+  tags are already silently ignored by the parser per spec, so `rf=`/`ri=`
+  now fall through to that same path rather than being validated.
+  Verified via a real `./configure && make check` on quark (this sandbox
+  lacks milter headers) — all 10 `libopendmarc` tests pass, including
+  `test_dmarc_parse`'s negative cases updated to expect `DMARC_PARSE_OKAY`
+  now that these are unknown tags rather than validated ones.
+- **`pass` disposition value** (S3.1.1.9): `ActionDispositionType` now
+  emits `pass` alongside `none`/`quarantine`/`reject`. New
+  `DMARC_RESULT_PASS` (`opendmarc.h`) is set in `opendmarc.c`'s
+  `enforce_policy` switch specifically when alignment passed *and* the
+  unadjusted published `p`/`sp` (whichever applies) was `reject` or
+  `quarantine` -- under `p=none`/`sp=none` the disposition stays `none`,
+  since RFC 9990 reserves `pass` for "passing DMARC w/enforcing policy".
+  Deliberately keyed off the unadjusted policy rather than the
+  `t=y`-downgraded `enforce_policy`, matching the existing rule that `t=`
+  doesn't affect report generation. Also added a `dis=pass` case to the
+  Authentication-Results `dis=` tag mapping for consistency with the
+  existing `dmarc=pass` `aresult`. Flows through `messages.disp` (a plain
+  unconstrained `TINYINT`, so no schema migration) into
+  `opendmarc-reports.in`'s disposition mapping. Verified with a clean
+  `./configure && make check` on quark: full project builds with no
+  warnings, `libopendmarc` suite still 10/10 (unaffected, but a good
+  regression signal since this touches the same enforcement path).
 - **RFC 9991 failure reporting**: `Identity-Alignment` and
   `DKIM-Domain`/`-Identity`/`-Selector` ARF headers, `ruf=` external
   destination verification + rate-limiting in `opendmarc-reports
@@ -56,18 +97,10 @@ specifically (all resolved) and are not duplicated here.
 
 ### RFC 9990 aggregate reporting
 
-- **`pass` disposition value** (S3.1.1.9): `ActionDispositionType` now
-  includes `pass` (message passed DMARC under an *enforcing* policy)
-  alongside `none`/`quarantine`/`reject`. Current code only ever emits the
-  original three. Not touched by the `np`/`testing`/`discovery_method` work.
 - **`policy_test_mode` reason type** (S3.1.6): a `<reason><type>` value a
   report record should carry when `t=y` caused a policy step-down. Directly
   adjacent to work already done — `t=` enforcement and `<testing>` in
   `policy_published` both shipped, but this per-record annotation didn't.
-- **`generator` element** (S3.1.1.3): identifies the report-generating
-  software. Not implemented in `opendmarc-reports`. (Unrelated: the
-  separate `contrib/dmarc-report-totext.pl` *consumer* tool already parses
-  this field from other senders' reports — that's reading, not writing.)
 - **`error` element** (S3.1.1.3/S3.1.5): describes processing errors
   encountered while evaluating the DMARC Policy Record. Not implemented.
 - **DKIM signature priority + 100-signature cap** (S3.1.3): defines which
@@ -77,10 +110,6 @@ specifically (all resolved) and are not duplicated here.
 - **Extension mechanism** (S3.2, S5): `<extension>` at file level,
   namespaced elements at record level. Low priority — only matters if
   extensions are actually adopted by report consumers.
-- **`rf=`/`ri=` cleanup**: both tags were removed from the DMARC record
-  format by RFC 9989. OpenDMARC still parses them into unused
-  `DMARC_POLICY_T` fields (`rf`, `ri`). Minor; safe to remove.
-
 ### RFC 9991 failure reporting
 
 Both RFC 6591 fields originally tracked here (`DKIM-Canonicalized-Header`/
@@ -135,14 +164,3 @@ in this section.
   startup instead of a compile error until someone actually parses a
   config file containing it, which apparently hadn't happened for three
   of these four since they were introduced.
-
-### Open decision, not just missing code
-
-- **`!NNNk` RUA size-suffix syntax**: RFC 9990 Appendix C says this syntax
-  is obsolete and receivers (i.e. OpenDMARC, acting as report generator)
-  MUST ignore it. The restored PR #392 code (`check_size_restriction`)
-  actively parses and enforces it, faithfully reproducing pre-regression
-  behavior rather than the RFC 9990 text. Same shape of question as the
-  `pct=` POLA decision: keep honoring it for senders who rely on it, or
-  drop it now that it's back in front of you. Needs an explicit call, not
-  a default.

libopendmarc/dmarc.h.in

@@ -47,9 +47,6 @@ extern "C" {
 #define DMARC_RECORD_P_NONE		('n')		/* p and sp */
 #define DMARC_RECORD_P_QUARANTINE	('q')		/* p and sp */
 #define DMARC_RECORD_P_REJECT		('r')		/* p and sp */
-#define DMARC_RECORD_RF_UNSPECIFIED	(0x0)		/* rf, a bitmap */
-#define DMARC_RECORD_RF_AFRF		(0x1)		/* rf, a bitmap */
-#define DMARC_RECORD_RF_IODEF		(0x2)		/* rf, a bitmap */
 #define DMARC_RECORD_FO_UNSPECIFIED	(0x0)		/* fo, a bitmap */
 #define DMARC_RECORD_FO_0		(0x1)		/* fo, a bitmap */
 #define DMARC_RECORD_FO_1		(0x2)		/* fo, a bitmap */

libopendmarc/docs/Makefile.am

@@ -13,7 +13,6 @@ dist_doc_DATA = index.html \
 	opendmarc_policy_fetch_fo.html \
 	opendmarc_policy_fetch_p.html \
 	opendmarc_policy_fetch_pct.html \
-	opendmarc_policy_fetch_rf.html \
 	opendmarc_policy_fetch_rua.html \
 	opendmarc_policy_fetch_ruf.html \
 	opendmarc_policy_fetch_sp.html \

libopendmarc/docs/index.html

@@ -160,10 +160,6 @@ <h2>Functions</h2>
   <td> <a href="opendmarc_policy_fetch_fo.html"> <tt>opendmarc_policy_fetch_fo()</tt> </a> </td>
   <td> Fetch a bitmap that shows how/when that RUF reports should be sent. </td>
  </tr>
- <tr>
-  <td> <a href="opendmarc_policy_fetch_rf.html"> <tt>opendmarc_policy_fetch_rf()</tt> </a> </td>
-  <td> Fetch a bitmap that shows which format (arfrf versus iodef) should be used. </td>
- </tr>
  <tr>
   <td> <a href="opendmarc_policy_fetch_alignment.html"> <tt>opendmarc_policy_fetch_alignment()</tt> </a> </td>
   <td> Fetch the result of SPF and DKIM alignment checks.

libopendmarc/opendmarc_internal.h

@@ -174,8 +174,6 @@ typedef struct dmarc_policy_t {
 	int		psd;		/* RFC 9989: DMARC_RECORD_PSD_* */
 	int		t;		/* RFC 9989: DMARC_RECORD_T_*, test mode */
 	int		pct;
-	int		rf;
-	uint32_t	ri;
 	int		rua_cnt;
 	u_char **	rua_list;
 	int		ruf_cnt;

libopendmarc/opendmarc_policy.c

@@ -1336,7 +1336,6 @@ opendmarc_policy_parse_dmarc(DMARC_POLICY_T *pctx, u_char *domain, u_char *recor
 	 * Set the defaults to detect missing required items.
 	 */
 	pctx->pct = -1;
-	pctx->ri  = -1;
 
 	(void) memset((char *)copy, '\0', sizeof copy);
 	(void) strlcpy((char *)copy, (char *)record, sizeof copy);
@@ -1498,65 +1497,6 @@ opendmarc_policy_parse_dmarc(DMARC_POLICY_T *pctx, u_char *domain, u_char *recor
 				return DMARC_PARSE_ERROR_BAD_VALUE;
 			}
 		}
-		else if (strcasecmp((char *)cp, "ri") == 0)
-		{
-			char *xp;
-
-			for (xp = (char *)vp; *xp != '\0'; ++xp)
-			{
-				if (! isdigit((int)*xp))
-					return DMARC_PARSE_ERROR_BAD_VALUE;
-			}
-			errno = 0;
-			pctx->ri = strtoul((char *)vp, NULL, 10);
-			if (errno == EINVAL || errno == ERANGE)
-			{
-				return DMARC_PARSE_ERROR_BAD_VALUE;
-			}
-		}
-		else if (strcasecmp((char *)cp, "rf") == 0)
-		{
-			char *xp, *yp;
-
-			/*
-			 * The list may be a comma delimilted list of choices.
-			 */
-			for (xp = (char *)vp; *xp != '\0'; )
-			{
-				u_char xbuf[32];
-
-				yp = strchr(xp, ',');
-				if (yp != NULL)
-					*yp = '\0';
-
-				xp = (char *)opendmarc_util_cleanup((u_char *)xp, xbuf, sizeof xbuf);
-				if (xp != NULL && strlen((char *)xp) > 0)
-				{
-					/*
-					 * Be generous. Accept, for example, "rf=a, aspf=afrf or any
-					 * left match of "afrf".
-					 */
-					if (strncasecmp((char *)xp, "afrf", strlen((char *)xp)) == 0)
-						pctx->rf |= DMARC_RECORD_RF_AFRF;
-					else if (strncasecmp((char *)xp, "iodef", strlen((char *)xp)) == 0)
-						pctx->aspf |= DMARC_RECORD_RF_IODEF;
-					else
-					{
-						/* A totaly unknown value */
-						return DMARC_PARSE_ERROR_BAD_VALUE;
-					}
-				}
-				else
-				{
-					return DMARC_PARSE_ERROR_BAD_VALUE;
-				}
-
-				if (yp != NULL)
-					xp = yp+1;
-				else
-					break;
-			}
-		}
 		else if (strcasecmp((char *)cp, "rua") == 0)
 		{
 			char *xp, *yp;
@@ -1696,10 +1636,6 @@ opendmarc_policy_parse_dmarc(DMARC_POLICY_T *pctx, u_char *domain, u_char *recor
 		pctx->aspf = DMARC_RECORD_A_RELAXED;
 	if (pctx->pct < 0)
 		pctx->pct = 100;
-	if (pctx->rf == DMARC_RECORD_RF_UNSPECIFIED)
-		pctx->rf = DMARC_RECORD_RF_AFRF;
-	if (pctx->ri == -1)
-		pctx->ri = 86400;
 	if (pctx->fo == DMARC_RECORD_FO_UNSPECIFIED)
 		pctx->fo = DMARC_RECORD_FO_0;
 
@@ -1992,20 +1928,6 @@ opendmarc_policy_fetch_fo(DMARC_POLICY_T *pctx, int *fo)
 	return DMARC_PARSE_OKAY;
 }
 
-OPENDMARC_STATUS_T
-opendmarc_policy_fetch_rf(DMARC_POLICY_T *pctx, int *rf)
-{
-	if (pctx == NULL)
-		return DMARC_PARSE_ERROR_NULL_CTX;
-	if (rf == NULL)
-		return DMARC_PARSE_ERROR_EMPTY;
-	if (pctx->ruf_list == NULL)
-		*rf = DMARC_RECORD_RF_UNSPECIFIED;
-	else
-		*rf = pctx->rf;
-	return DMARC_PARSE_OKAY;
-}
-
 /**************************************************************************************************
 ** OPENDMARC_POLICY_FETCH_UTILIZED_DOMAIN -- Return domain used to get the dmarc record
 **					     Either the From: domain or the organizational domain
@@ -2462,31 +2384,6 @@ opendmarc_policy_to_buf(DMARC_POLICY_T *pctx, char *buf, size_t buflen)
 	if (strlcat(buf, nbuf, buflen) >= buflen) return E2BIG;
 	if (strlcat(buf, "\n", buflen) >= buflen) return E2BIG;
 
-	if (strlcat(buf, "RF=", buflen) >= buflen) return E2BIG;
-	if (pctx->rf == 0)
-	{
-		if (strlcat(buf, "UNSPECIFIED", buflen) >= buflen) return E2BIG;
-	}
-	if ((pctx->rf&DMARC_RECORD_RF_AFRF) != 0)
-	{
-		if (strlcat(buf, "AFRF", buflen) >= buflen) return E2BIG;
-	}
-	if ((pctx->rf&DMARC_RECORD_RF_IODEF) != 0 &&
-	    (pctx->rf&DMARC_RECORD_RF_AFRF) != 0)
-	{
-		if (strlcat(buf, ",", buflen) >= buflen) return E2BIG;
-	}
-	if ((pctx->rf&DMARC_RECORD_RF_IODEF) != 0)
-	{
-		if (strlcat(buf, "IODEF", buflen) >= buflen) return E2BIG;
-	}
-	if (strlcat(buf, "\n", buflen) >= buflen) return E2BIG;
-
-	if (strlcat(buf, "RI=", buflen) >= buflen) return E2BIG;
-	(void) snprintf(nbuf, sizeof nbuf, "%d", pctx->ri);
-	if (strlcat(buf, nbuf, buflen) >= buflen) return E2BIG;
-	if (strlcat(buf, "\n", buflen) >= buflen) return E2BIG;
-
 	if (strlcat(buf, "RUA=", buflen) >= buflen) return E2BIG;
 	for (i = 0; i < pctx->rua_cnt; ++i)
 	{

libopendmarc/tests/test_dmarc_parse.c

@@ -27,14 +27,14 @@ main(int argc, char **argv)
 		/* 9  */ {"v=DMARC1; p=none; sp=bob;", DMARC_PARSE_ERROR_BAD_VALUE},
 		/* 10 */ {"v=DMARC1; p=none; adkim=bob;", DMARC_PARSE_ERROR_BAD_VALUE},
 		/* 11 */ {"v=DMARC1; p=none; aspf=bob;", DMARC_PARSE_ERROR_BAD_VALUE},
-		/* 12 */ {"v=DMARC1; p=none; rf=bob;", DMARC_PARSE_ERROR_BAD_VALUE},
-		/* 13 */ {"v=DMARC1; p=none; ri=bob;", DMARC_PARSE_ERROR_BAD_VALUE},
+		/* 12 */ {"v=DMARC1; p=none; rf=bob;", DMARC_PARSE_OKAY}, /* rf= removed by RFC 9989; now an ignored unknown tag */
+		/* 13 */ {"v=DMARC1; p=none; ri=bob;", DMARC_PARSE_OKAY}, /* ri= removed by RFC 9989; now an ignored unknown tag */
 		/* 14 */ {"v=DMARC1; p=none; pct=500;", DMARC_PARSE_ERROR_BAD_VALUE},
 		/* 15 */ {"v=DMARC1; pct=100;", DMARC_PARSE_ERROR_NO_REQUIRED_P},
 		/* 16 */ {"v=DMARC1; p=none; rua=ftp://abuse.com", DMARC_PARSE_OKAY},
 		/* 17 */ {"v=DMARC1; p=none; ruf=mailto://abuse.com", DMARC_PARSE_OKAY},
 		/* 18 */ {"v=DMARC1; p=none; ruf=mailto://abuse.com; foo=bar; buzz=happy;", DMARC_PARSE_OKAY},
-		/* 19 */ {"v=DMARC1; p=none; rf=000000000000000000000000000000000", DMARC_PARSE_ERROR_BAD_VALUE},
+		/* 19 */ {"v=DMARC1; p=none; rf=000000000000000000000000000000000", DMARC_PARSE_OKAY}, /* rf= removed by RFC 9989; now an ignored unknown tag */
 		/* 20 */ {"v=DMARC1; p=reject; t=y;", DMARC_PARSE_OKAY},
 		/* 21 */ {"v=DMARC1; p=reject; t=n;", DMARC_PARSE_OKAY},
 		/* 22 */ {"v=DMARC1; p=reject; t=bob;", DMARC_PARSE_ERROR_BAD_VALUE},