[Dcrup] Review of draft-ietf-dcrup-dkim-crypto-03

[Martin Thomson <[e-mail]>]

In its current form, I had a hard time extracting the goals of this
document from the text, even though the abstract is clear, the
introduction isn't clear enough on the specifics, see below.

I think that Scott's draft does a much better job of the SHA-1 removal
and attempting to do this all at once in this document isn't helping
its clarity at all.  (Also, this practice of patching RFCs by
providing replacement for statements in them makes reading the update
as a standalone document unnecessarily difficult in my opinion.)

On hashing here: hash everything.  Ed25519 might be the last algorithm
that has a key that is this small.  Uniformity is more valuable than
space here.

On PSS: I think that the idea is to retain the existing primitive with
the fingerprint-based mode, so we should define "rsafp" as using
PKCS#1v1.5 and accept the shortcomings of that.  I would support the
addition of a PSS-based scheme.

Introduction

The framing of this document needs to be improved.  The point of this
draft is to define new ways to sign e-mail messages that are stronger
than what the existing mechanisms allow.

The first paragraph here is good, it establishes that the existing
mechanism (the only one!) is limited by real practical concerns.

The second paragraph should instead say that it is introducing
signature algorithms:

~~~
This document adds two signing mechanisms for DKIM.  A mechanism using
the Edwards-Curve Digital Signature Algorithm (EdDSA) [RFC8032] is
defined in Section 3.  A variant of RSA signatures that uses the
existing underlying RSA primitives is defined in Section 4.  This RSA
variant uses a hashed representation of the public key that allows for
use of larger keys.
~~~

Section 3

This could be more direct:
~~~
3. Signature with Ed25519

The "ed25519" signature algorithm uses Ed25519 [RFC8032] and SHA-256 [SHA-2].

To use this signature method, the message is canonicalized according
to [RFC6376] then hashed using SHA-256.  The hash is signed using the
Pure variant of Ed25519.

This signature algorithm is identified with the value "ed25519" in the
"k=" parameter of a textual key representation and the value
"ed25519-sha256" in the "a=" parameter of the DKIM-Signature header
field.
~~~

IMPORTANT: This should be "ed25519" instead of "eddsa"; we've started
identifying a single primitive in other protocols, and you don't want
to contort things if you ever have to add Ed448.

Section 4

In the same way, this could be direct.  This is defining a new
signature algorithm.  The description can explain how this diverges
from the existing norm, but still be a complete specification.

~~~
The "rsafp" signature algorithm uses the same RSA-PKCS#1v1.5 [RSA-REF]
primitive to sign as that used in "rsa".  However, this algorithm
places a hash of the public key in textual representation in place of
the DER-encoded RSAPublicKey defined in [RFC6376].  This allows for
use of larger keys without the need for larger textual
representations, which might otherwise exceed the space in DNS TXT
records.

The method for signing a message is the same as that described for
"rsa-sha256" in [RFC6376].  The signer MUST also include a "p="
parameter in the DKIM-Signature header field (see Section XXX).  This
parameter includes the base64- and DER-encoded RSAPublicKey that was
used to generate the signature.

Textual representations of "rsafp" keys are identical to that of
"rsa-sha256", except that the "p=" parameter contains a SHA-256 hash
of the DER-encoded RSAPublicKey.  This limits the size of the textual
representation.

In addition to verifying the signature using this public key, a
verifier MUST also ensure that the decoded "p=" parameter that
accompanies the signature is equal to the SHA-256 hash of the decoded
"p=" parameter from the textual representation of the key.

This signature algorithm is identified with the value "rsafp" in the
"k=" parameter of a textual key representation and the value
"rsafp-sha256" in the "a=" parameter of the DKIM-Signature header
field.
~~~

IMPORTANT: The document then needs a new section defining "p=" for use
in DKIM-Signature.

This paragraph doesn't seem to be adding any value:

   Section 3.7 of [RFC6376], on computing the message hashes, is not
   modified.  Since the key in the k= tag is known in advance, it [is]
   included in the signature in the same manner as all of the other
   signature fields other than b=.

Sections 3 and 4

Both Section 3 and Section 4 need to explicitly say that the "h="
parameter is not valid for the algorithms defined in this document and
that SHA-256 is the only allowed hash algorithm that can be used with
these.  The current structure does not permit the use of a different
hash algorithm for the fingerprint; we should be prepared to define a
new identifier if we ever need to provide a SHA-256 alternative.

Nits

Capitalize "section" throughout.

There is a reference to FIPS-180 in a table, but it doesn't appear in
the references section.