[Webpush] Ben Campbell's Discuss on draft-ietf-webpush-vapid-03: (with DISCUSS and COMMENT)

[Ben Campbell <[e-mail]>]

Ben Campbell has entered the following ballot position for
draft-ietf-webpush-vapid-03: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-webpush-vapid/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

In section 2: "A push service MAY reject a request with a 403 (Forbidden) status
code [RFC7235] if the JWT signature or its claims are invalid."

This seems to leave the possibility of simply ignoring an invalid VAPID token
or signature. Assuming we are talking about push servers that support
VAPID in the first place, that seems dangerous. Wouldn't it be safer in
the general case to treat a request with an invalid VAPID token as at
least a bit fishy?

I don't mean to say that ignoring the token is never the right thing to
do. But the MAY seems week without some guidance on what other actions
might be reasonable under what circumstances.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Substantive:

I agree with Ekr's DISCUSS.

-3:
"This authentication scheme is intended for use by an application server
when using the Web Push protocol [RFC8030], but it could be used in
other contexts if applicable."

This guidance seems risky without some description of the properties a
given context should have for vapid to be appropriate to use. Please
consider elaborating, or removing the clause after the comma.

"Some implementations permit the same P-256 key to be used for signing
and key exchange.  An application server MUST select a different private
key for the key exchange [I-D.ietf-webpush-encryption] and signing the
authentication token."

I don't understand the intent here. It seems to say some implementations
do this, but MUST not. Does "implementations" refer to VAPID or
something else?

-4.1: "A push service MUST ignore the body of a request that uses a
different media type."

This seems to give VAPID a monopoly on adding
bodies. It precludes adding other media types in future webpush
extensions. Is that the intent?

- 4.2: "A push service MUST reject a message that omits mandatory
credentials with a 401 (Unauthorized) status code. "

This was already stated in section 2. Please avoid repeating normative
text, as it creates ambiguity about which text is authoritative, and can
complicate future updates.  This section seems the more detailed, so perhaps
section 2 could avoid the 2119 text. (But see my DISCUSS point on section 2.)

- 8.2:  [I-D.ietf-webpush-encryption] should be normative, since it is
referenced in a MUST level normative requirement in section 3.2. (If
that reference is not intended to be normative, then please rephrase the
referring sentence.)

Editorial:

- Abstract: Please mention the name of the mechanism in the abstract. It
might also be helpful to mention that there are cryptographic signatures
involved. (Same for section 1).

In general, the numerous mentions of "this design" tend to be ambiguous
about whether we talk of the design in this document rather than that of
webpush in general. Once could substitue "VAPID" for many instances.

- 1.0: "Additionally, this design also ..." "Additionally" and "also"
are redundant

- 1.2: Please use the boilerplate from RFC 8174.

- 4.1: "The user agent includes the public key of the application server when
requesting the creation of a push subscription."

I assume this means "A user agent that wishes to create a restricted
subscription...". I know the heading says "Creating a restricted
subscription", but in general the body text make sense even without the
heading.

"The public key is then added to the request to create a push
subscription.  The push subscription request is extended to include a
body.  The body of the request is a JSON object as described in
[RFC7159].  A "vapid" member is added to this JSON object, containing
the public key on the P-256 curve, encoded in the uncompressed form
[X9.62] and base64url encoded [RFC7515].  The media type of the body is
set to "application/webpush-options+json" (see Section 6.3 for
registration of this media type)."

The actor(s) are obscured by the heavy use of passive voice.

- 6.3: Don't we usually list the IESG as the change controller?