Domain-based Message Authentication, Reporting, and Conformance (DMARC) Aggregate Reporting
draft-ietf-dmarc-aggregate-reporting-32

Thanks to Phillip Hallam-Baker for the secdir review.

# Gunter Van de Velde, RTG AD, comments for draft-ietf-dmarc-aggregate-reporting-28

# The line numbers used are rendered from IETF idnits tool: https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-dmarc-aggregate-reporting-28.txt

# DMARC isn't really my area of expertise, so this review comes from a network generalist perspective. Some of my comments might be due to not fully understanding the technology; just something to keep in mind!

# following you find some non-blocking comments.

# General Review
# ==============

## I noticed that the word "Mail" is sometimes written with an uppercase M and other times in lowercase. Additionally, "email" is used in some places. Was this intentional? Would it make sense to standardize it and simply use "mail" throughout for consistency?

## idnits identifies few downrefs. The shepherd report suggest that only RFC6713 is not in the downref register, however it seems that RFC7489 may also not be in there (https://datatracker.ietf.org/doc/downref/) and is an ISE document. The last call did call out the 3 downrefs.

# Detailed Review
# ===============

167	   The report may include the following data:

GV> next a list of metadata included. Would it be useful to add usage structure for these?
For example:

Report ID
* Reporting Organization (ISP, mail receiver)
* Date range covered by the report

Email Authentication Results:
* SPF (Sender Policy Framework) alignment status
* DKIM (DomainKeys Identified Mail) alignment status
* DMARC policy result (pass/fail)

Sending Source Details:
* IP addresses of sending servers
* Number of messages sent from each IP
* Reverse DNS information of the sending IP

Policy Evaluation:
* The applied DMARC policy (none, quarantine, or reject)
* How many emails were accepted, rejected, or quarantined
* Alignment failures and reasons

Domain Alignment Check:
* Whether the SPF and DKIM domains align with the From: domain

597	   1.  A signature that passes DKIM, in strict alignment with the
598	       RFC5322.From domain
599	   2.  A signature that passes DKIM, in relaxed alignment with the
600	       RFC5322.From domain

GV> idnit or clarification. Not sure what "RFC5322.From domain" wants to specify? Is this a typo or copy/paste glitch?

789	   mail.receiver.example!example.com!1013662812!1013749130.xml.gz
790	   No specific MIME message structure is required.  It is presumed that

GV> idnit or clarification. Should this maybe read as:
There is no specific MIME message structure required with mail.receiver.example!example.com!1013662812!1013749130.xml.gz

Kind Regards,
Gunter Van de Velde
Routing Area Director

Section 7, paragraph 0
> 7.  Security Considerations

I wanted to thank Dhruv for his OPSDIR review. I agree with the issues he points out, and would like to see a resolution of his comments. 

Others have already provided feedback on issues such as the use of acronyms without any expansion, identifiers such as 'p' or 'sp' that have not been defined except in the examples, and other comments that I am not going to repeat here.

The IANA review of this document seems to not have concluded yet.

Thanks for addressing my comments in -29.

### Nits

In -29, the word remaining here is perhaps no longer needed:

```
An attempt MUST be made to deliver an aggregate report to
   every remaining URI, up to the Receiver's limits on supported URIs.
```

I think the intended behavior with the changes from -29 is to attempt to deliver to all URIs.

Section states:

        If a report generator needs to re-send a report, the system
        MUST use the same filename as the original report. This would
        allow the receiver to overwrite the data from the original,
        or discard second instance of the report.

It seems dangerous to use file names based on received strings from unknown
sources, and I don't think an RFC should recommend to use such strings as
filenames, unless proper warnings are in place to exclude harmful ones (eg
"../../../").

While a maliciously formatted unique-id should be rejected per this specification,
perhaps add a warning in the Security Considerations for this to give explicit
warning to implementers.

While I think "dot dot slash dot dot slash etc passwd" seems less likely to be writable by the mail
user, this could be used to stuff files into the mail system (eg /var/mail or any of the outgoing mail queues)

Thank you for addressing part my DISCUSS and COMMENT feedback.

Thanks for addressing my previous DISCUSS point (I told you that it was super easy to fix)

## COMMENTS (non-blocking)

### Abstract

It is unclear to me what is meant by `as supported by the receiver`.

s/replaces [RFC7489]/obsoletes and replaces [RFC7489]/

### Section 1

Suggest repeating the expansion for "DMARC".

s/to request that receivers/to request that e-mail receivers/ ? (see further)
 
### Section 2

Suggest to be consistent in the naming "Mail Receivers" (to be preferred) or "mail receivers" or "receivers".

### Section 2.1.1.8

What is a `The connecting IP`? I only know about "The connecting IP address" also valid for s/source_ip/source_ip_address/ (probably too late to change the element name).

### Section 2.1.1.10

Where is "RFC5322.From" defined ? I can guess it of course, but let's be accurate.

I would like to thank Di Ma for their DNSDir review.

Thanks for working on this specification. 

I am supporting Orie's discuss on section 2.5. I would also ask why do we call this section "transport"? First I was curious to have a section called transport :-), then I realized it describes some rules to generate report and delivering that report. Can we rename this section to something like "Gerenration and delivery" or something more meaningful in that context.

I also support Roman's discuss.