"From" header missing from SHOULD

Bug #1525048 reported by Skyler Slade
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
dkimpy
Fix Released
High
Scott Kitterman

Bug Description

According to RFC 4871 section 5.4, Recommended Signature Content, "From" "SHOULD be included in the signature." However, dkim.DKIM.SHOULD does not include this.

For example,
In [10]: dkim.DKIM.SHOULD
Out[10]:
('sender',
 'reply-to',
 'subject',
 'date',
 'message-id',
 'to',
 'cc',
 'mime-version',
 'content-type',
 'content-transfer-encoding',
 'content-id',
 'content- description',
 'resent-date',
 'resent-from',
 'resent-sender',
 'resent-to',
 'resent-cc',
 'resent-message-id',
 'in-reply-to',
 'references',
 'list-id',
 'list-help',
 'list-unsubscribe',
 'list-subscribe',
 'list-post',
 'list-owner',
 'list-archive')

From is listed in dkim.DKIM.RFC5322_SINGLETON but this list serves a different purpose.

Tested in version 0.5.4.

Revision history for this message
Stuart Gathman (stuart-gathman) wrote :

That's interesting, but From is actually a mandatory header, so putting it in the SHOULD list would be redundant. (Although it shouldn't break anything.) We could have a MANDATORY list, but From seems to be the only really mandatory header.

Revision history for this message
Scott Kitterman (kitterman) wrote :

RFC 6376, which obsoleted RFC 4871, has this:

5.4. Determine the Header Fields to Sign

   The From header field MUST be signed (that is, included in the "h="
   tag of the resulting DKIM-Signature header field). ...

I think that since From is included in the FROZEN list just above the SHOULD list (line 290 of dkim/__init__.py currently) it will always be signed. If that's not the case, then we need to add mandatory.

Changed in dkimpy:
importance: Undecided → High
Revision history for this message
Skyler Slade (jsslade) wrote :

Because From is in FROZEN, it'll be signed twice (related to: https://bugs.launchpad.net/dkimpy/+bug/799175). If implementations don't want to sign From twice, they have to remove it from frozen_sign and add it to should_sign which seems like an oversight in the API.

For example
        dkim_obj.frozen_sign = set([])
        dkim_obj.should_sign.add('from')

Otherwise signing will fail, because From is not included.

Revision history for this message
Skyler Slade (jsslade) wrote :

I misspoke. What I meant to say was, if you don't want to freeze Subject and Date, for example, you must remove them from frozen_sign. Since frozen_sign is a set, you must set the entire set to a new set (you can't del frozen_set['Subject'] for example). A user needs to know to add From to frozen_set or to should_sign, and if nothing else, this isn't clear from the available documentation.

Revision history for this message
Scott Kitterman (kitterman) wrote :

I think the cleanest way to address this (I agree it's currently convoluted) is to add a new MUST list which contains only From.

Changed in dkimpy:
status: New → Triaged
Revision history for this message
Stuart Gathman (stuart-gathman) wrote :

The frozen_sign is a regular set that you can modify, not a frozen_set. The term "frozen" is from the RFC, and means you can't have more than one of those headers. We could rename to something that doesn't conflict conceptually with python frozen_set, like "singleton_sign". But that has all sorts of compatibility issues.

Revision history for this message
Stuart Gathman (stuart-gathman) wrote :

BTW, From MUST be signed twice, because it is a mandatory frozen/singleton header. You can't have more than one From header. It should not be easy to change this. It is still possible with the current API, and that's ok.

Revision history for this message
Stuart Gathman (stuart-gathman) wrote :

I see we have an add_frozen method. Maybe all you really want is a remove_frozen method. (Although the del frozen_set['subject'] *does* work - and is exactly what a remove_frozen would do.)

In the remove_frozen method, I could raise an exception if you try to remove 'from', with an i_know_what_i_am_doing=True parameter to not raise the exception.

However, having more than one Subject or Date is a violation of the mail RFCs, not just DKIM. Do we really want to make it easy? The legitimate application of remove_frozen would be for additional headers that were added by the application. It should probably require the i_know_what_i_am_doing flag for removing any of from,subject,date.

Revision history for this message
Stuart Gathman (stuart-gathman) wrote :

I agree that using a MUST list with just 'from' in the places where it checks that 'from' is signed could make the code more consistent. Although the indirection (having to go look at the list) has its own drawbacks.

Revision history for this message
Stuart Gathman (stuart-gathman) wrote :

Bug 799175 reminds me that From is special in another way. You can have more than one of frozen_sign headers - N headers are signed N+1 times to freeze the number (detect additions when checking the signature). There is a list of headers that mail RFCs say MUST occur at most once - however Scott says we shouldn't try to enforce mail RFCs. However, the whole concept of DKIM depends on there being ONE and ONLY ONE From header. The rest are elaborations. Putting 'from' in a MUST list expresses the ONE specialness, but not the ONLY ONE specialness.

Revision history for this message
Skyler Slade (jsslade) wrote :

Regarding "From MUST be signed twice, because it is a mandatory frozen/singleton header. You can't have more than one From header" this is not a requirement of RFC 6376. In section 8.15, Attacks Involving Extra Header Fields, double-signing From is presented as an option:

   "These can represent serious attacks, but they have nothing to do with
   DKIM; ... DKIM can aid in detecting addition of specific fields in transit.
   This is done by having the Signer list the field name(s) in the "h=" tag an
   extra time (e.g., "h=from:from:..." for a message with one From field), so
   that addition of an instance of that field downstream will render the
   signature unable to be verified."

It might be a good idea to double sign but it's not a strict requirement.

Revision history for this message
Scott Kitterman (kitterman) wrote :

It's true that double signing is not an RFC requirement, but it's a safe and easy way to resolve a security concern that's identified in the RFC. We did test what different implementations due with a signature in a message with multiple From and generally they well grab either the first or the second, but without oversigning verification didn't consistently failed. MUAs then do different things with two Froms, so it seems best to oversign.

Changed in dkimpy:
milestone: none → future
Revision history for this message
Scott Kitterman (kitterman) wrote :

It turns out it's fine to have a field both in should and frozen, so I added it.

Changed in dkimpy:
assignee: nobody → Scott Kitterman (kitterman)
milestone: future → none
status: Triaged → Fix Committed
Revision history for this message
Scott Kitterman (kitterman) wrote :

Fixed in 0.9.0.

Changed in dkimpy:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.