[openpgp] Minor comments on current draft

["Neal H. Walfield" <[e-mail]>]

Hi all,

First of all, thank you to the design team for coming up with the
excellent draft!  I've already posted a few bigger questions.  Here
are some minor points that I collected.  I also created an MR for some
(I think) trivial copy editing issues that I found:

  https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/221

Overall, I'm quite happy with the result.

Neal

General comment
===============

Some of the tables are quite functional whereas some are rather
pretty.  For instance, there are two tables in Section 6.3.  The first
one uses 7-bit ASCII characters to create the table's frame whereas
the second one uses the extended ASCII box drawing characters.  I
think the document ought to standardize on one convention, preferably
the latter one.

  https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-6.3

Minor Issues
============

> 4.3.  Packet Tags
>
>           |    20 | yes      | Reserved (formerly AEAD         |
>           |       |          | Encrypted Data Packet)          |
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-4.3

"formerly" implies to me that the packet was specified, but the AEAD
packet was never actually specified by an RFC.  Perhaps
s/formerly/unspecified/ would be a bit better.


>  5.2.  Signature Packet (Tag 2)
>
>  Implementations MUST NOT create version 3 signatures; they MAY
>  accept version 3 signatures.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2

Perhaps: they MAY accept version 3 signatures *made by v4 keys, but
MUST NOT accept v3 signatures made by v5 keys*.

Further, perhaps also add, as has been done in other places like
Section 14.7:

   if it is known to be dealing with historic archived legacy data,
   and the user is aware of the risks.


> 5.2.3.11.  Preferred Symmetric Ciphers for v1 SEIPD>
>
>    It is assumed that only algorithms listed are supported by the
>    recipient's software.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.3.11

The Preferred AEAD Ciphersuites subsection (the next subsection) says
that AES-128-OCB is implicitly included in that preference.  No
default is not mentioned here, but it is mentioned in Section 13.2
Symmetric Algorithm Preferences.

https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-13.2

I think it would be good to mention the default here for consistency
purposes, and also because this is a change in behavior.  For
instance, at least GnuPG assumes 3DES is the default, since, I think,
it is the mandatory-to-implement algorithm in 4880:

  https://gitlab.com/sequoia-pgp/sequoia/-/issues/522


> 5.2.5.  Malformed and Unknown Signatures
>
>    In some cases, a signature packet (or its corresponding One-Pass
>    Signature Packet, see Section 5.4) may be malformed or unknown.  For
>    example, it might encounter any of the following problems (this is
              ^^
>    not an exhaustive list):
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.5

The antecedent of "it" appears to be signature packet, but I don't
think the signature packet encounters a problem; the OpenPGP
implementation does.


> 5.4.  One-Pass Signature Packets (Tag 4)
>
>       An application that encounters a v5 One-Pass Signature packet
>       where the key version number is not 5 MUST treat the signature as
>       invalid (see Section 5.2.5).
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.4

Section 5.2.5 talks mostly about Malformed and Unknown Signatures.
(It does mention "invalid" signatures once, but it doesn't really
define that term.)  Perhaps it makes sense to tighten up the wording
and say that the OPS packet is malformed.


> 5.5.2.  Public-Key Packet Formats
>
>    V3 keys are deprecated; an implementation MUST
>    NOT generate a v3 key, but MAY accept it.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.5.2

There are several places in the standard where an implementation MAY
do something IF it is dealing with archived data, e.g.:

   An implementation that encounters malleable ciphertext MAY choose to
   release cleartext to the user if it is not encrypted using AEAD, and
   it is known to be dealing with historic archived legacy data, and the
   user is aware of the risks.

I think it would be good to add the same qualifier to the use of v3
keys here.


> 13.2.  Symmetric Algorithm Preferences
>
>    The symmetric algorithm preference is an ordered list of algorithms
>    that the keyholder accepts.  Since it is found on a self-signature,
>    it is possible that a keyholder may have multiple, different
>    preferences.  For example, Alice may have AES-128 only specified for
>    "[e-mail]" but Camellia-256, Twofish, and AES-128 specified for
>    "[e-mail]".
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-13.2

Can we please change "[e-mail]" and "[e-mail]" to
"<[e-mail]>" and "<[e-mail]>"?

Section 5.11 says that by convention User IDs are an [RFC2822] mail
name-addr, but as-is, these are not valid name-addrs.  Further, the
common regular expression used in trust signatures:

  <[^>]+[@.]example\.com>$

wouldn't match the above User IDs.


That said, these are already better than the User IDs used in the
example in Section 5.2.3.7, which was carried over from 4880:

> 5.2.3.7.  Notes on Self-Signatures
>
>   For example, suppose a key has two user names, Alice and Bob
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.3.7

Could we update those names, too?


> 13.9.  OpenPGP CFB Mode
>
>    When using a version 1 Symmetrically Encrypted Integrity Protected
>    Data packet (Section 5.13.1) or --- for historic data --- a
>    Symmetrically Encrypted Data packet (Section 5.7), OpenPGP does
>    symmetric encryption using a variant of Cipher Feedback mode (CFB
>    mode).  This section describes the procedure it uses in detail.  This
>    mode is what is used for Symmetrically Encrypted Integrity Protected
>    Data Packets (and the dangerously malleable --- and deprecated ---
>    Symmetrically Encrypted Data Packets).
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-13.9

The first (When using...) and third (This mode...) sentences seem
redundant to me and ought to be combined.


> 14.  Security Considerations
>
>    *  The DSA algorithm will work with any hash, but is sensitive to the
>       quality of the hash algorithm.  Verifiers should be aware that
>       even if the signer used a strong hash, an attacker could have
>       modified the signature to use a weak one.  Only signatures using
>       acceptably strong hash algorithms should be accepted as valid.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-14

This seems unnecessary given that Section 13.5 says:

>    An implementation MUST NOT sign or verify using DSA keys.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-13.5


> 14.  Security Considerations
>
>          +=====================+===========+====================+
>          | Asymmetric key size | Hash size | Symmetric key size |
>          +=====================+===========+====================+
>          |                1024 | 160       | 80                 |
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-14

This only applies to RSA, Elgamal, and DSA, but those are partially
(RSA) or completely (DSA, Elgamal) deprecated.  Perhaps it make sense
to talk about ECC curve sizes instead.


> 14.  Security Considerations
>
>    For example, although CAST5 is presently considered strong,
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-14

but, Section 9.3 says:

>    Implementations MUST NOT encrypt data with IDEA, TripleDES, or
>    CAST5.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-9.3

This should be made consistent.  I think section 14 should be changed.


4880 lints
==========

Minor issues that I caught that are also in 4880:

> 3.2.  Multiprecision Integers
>
>    ...
>
>    The length field of an MPI describes the length starting from its
>    most significant non-zero bit.  Thus, the MPI [00 02 01] is not
>    formed correctly.  It should be [00 01 01].
>
>    ...
>
>    Unused bits of an MPI MUST be zero.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-3.2

Reading the rules, it seems like the following MPI is valid and has
the value "1": [00 01 02].  I don't think this is intended.  Probably
the section should say that any zero padding must occur on the left.


> 5.2.3.5.  Signature Subpacket Specification
>
>    ...
>
>    An implementation SHOULD ignore any subpacket of a type that it does
>    not recognize.
>
>    Bit 7 of the subpacket type is the "critical" bit.  If set, it
>    denotes that the subpacket is one that is critical for the evaluator
>    of the signature to recognize.  If a subpacket is encountered that is
>    marked critical but is unknown to the evaluating software, the
>    evaluator SHOULD consider the signature to be in error.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.3.5

I feel like these two paragraphs should be reversed and the first
paragraph changed to indicate that this only applies to subpackets
that don't have the critical bit set.


> 5.2.3.5.  Signature Subpacket Specification
>
>    ...
>
>    if an implementation
>    chooses not to implement some of the preferences, it is required to
>    behave in a polite manner to respect the wishes of those users who do
>    implement these preferences.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.3.5

I don't understand what is meant here.  What does "polite manner"
mean?  (I'm not being picky here: I really don't understand.)


> 5.2.3.7.  Notes on Self-Signatures
>
>    For certification self-signatures, each User ID may have a self-
>    signature
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.3.7

perhaps each User ID and each User Attribute?


> 5.2.3.7.  Notes on Self-Signatures
>
>    Subpackets that appear in a certification self-signature
>    apply to the user name
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.3.7

user name seems to be a new term.  Why not User ID?  (In which case,
why not also User Attribute?)


> 5.2.3.28.  Reason for Revocation
>
>   Such a revocation SHOULD include a 0x20 code.
https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-07#section-5.2.3.28

The table uses decimal, but the text uses hexadecimal.  I think it
would be better to use one format or the other.