Re: [openpgp] AD review of draft-ietf-openpgp-crypto-refresh-10

On Fri, 25 Aug 2023, Roman Danyliw wrote:

> ** There are a number of errata

See other email. I went through the entire list and filed a few PRs and
request for texts.

> ** Editorial.  The document makes statements about “legacy implementation” or “modern implementation”.  “Historic” is also used.

> Could these be crisply defined in the early parts of the documents.  Perhaps Section 1.1.  Does “legacy” mean conformant to RFC4880?

We had that but we removed it. It gets a bit confusing as at times
legacy meant "pre-IETF", or RFC 2440, or RFC 4880. And then things
changed because 4880 is legacy too. So now, basically everything
not v6 is legacy. I'll go through it and see but I would like to
avoid needing to re-introuce terms like PGP2, PGP65.

> ** Section 2.2.  Editorial.  Is there a reason this section uses the term “sending software”/”receiving software” and the previous section (2.1) used “sending OpenPGP implementation”/”receiving OpenPGP implementation”?

Fixed. It now uses "implementation" instead of "software".

> ** Section 3.7.1
>   These are described in the subsections below.  If the "Generate?"
>   column is not "Yes", the S2K entry is used only for reading in
>   backwards compatibility mode and should not be used to generate new
>   output.
>
> Should normative “SHOULD NOT” be used here instead?

Fixed.

> ** Section 3.7.1.3
>   The above formula is in C, where "Int32" is a type for a 32-bit integer, and the variable "c" is the coded
>   count, Octet 10.
>
> Since formal syntax is being invoked, please cite the relevant version of the C language.  Perhaps C99 (ISO/IEC 9899:1999), C11 (ISO/IEC 9899:2011), or C18 (ISO/IEC 9899:2018)?  C99 would be:
>
> [C99]
> International Organization for Standardization, "Programming languages - C: C99, correction 3:2007", ISO/IEC 9899:1999/Cor 3:2007, November 2007.

Fixed.

> ** Section 3.7.1.4.  What is “m”?
> I was initial confused because of the number of names that Octet 19 has – Octet 19, “one-octet exponent indicating the memory size m”, “encoded_m”, “encoded memory size”.
>
> Wouldn’t it be clearer to say:
> OLD
>   Octet  19:       one-octet exponent indicating the memory size m
>
> NEW
>  Octet  19:       one-octet encoded memory size, encoded_m
>
> This simplifies:
> OLD
> The memory size m is 2**encoded_m kibibytes of RAM, where "encoded_m"
> is the encoded memory size in Octet 19.  The encoded memory size MUST
>
> NEW
> The memory size m is 2**encoded_m kibibytes of RAM.  Encoded_m MUST

Fixed.

> ** Section 4.3.  Are “packet tag” and “packet type” synonyms?  If so, can that be explicitly stated.

Yes, but it would change things all over the place. We have a PR for
that wasn't merged. Would this PR be your preference over the current
situation: https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/324

> ** Section 5.1.2.  Editorial
>   * A one-octet size of the following two fields. The size may also be zero, and the key version and
>    fingerprint omitted for an "anonymous recipient" (see Section 5.1.8).
>
> Is it the next two fields in the list (version number and fingerprint)?  I ask because all of the fields in this section are presented in a single level, enumerated list.  I would have expected some kind of indentation or naming of the fields.

It is the next two fields, so indentation would be invalid. I've
clarified the text to hopefully clarify this better.

> ** Section 5.1.6 and 5.1.7.  Please provide references for SHA256 and SHA512.

Done using RFC6234.

> ** Section 5.2.3.
>   *  A scalar octet count for the following hashed subpacket data.  For
>      a v4 signature, this is a two-octet field.  For a v6 signature,
>      this is a four-octet field.  Note that this is the length in
>      octets of all of the hashed subpackets; a pointer incremented by
>      this number will skip over the hashed subpackets.
>
>   *  Hashed subpacket data set (zero or more subpackets).
>
>   *  A scalar octet count for the following unhashed subpacket data.
>      For a v4 signature, this is a two-octet field.  For a v6
>      signature, this is a four-octet field.  Note that this is the
>      length in octets of all of the unhashed subpackets; a pointer
>      incremented by this number will skip over the unhashed subpackets.
>
>   *  Unhashed subpacket data set (zero or more subpackets).
>
> -- I’m stumbling on “scalar octet count” and the assumptions needed to determine what is the “following hashed subpacket data” given that everything is in a bulleted list.  Is this the equivalent of:
>
> OLD
>   *  A scalar octet count for the following hashed subpacket data.  For
>      a v4 signature, this is a two-octet field.  For a v6 signature,
>      this is a four-octet field.  Note that this is the length in
>      octets of all of the hashed subpackets;
>
> NEW
> A two-octet (for a v4 signature) or four-octet (for a v6 signature) count of the total number octets in all the unhashed subpackets

Instead of your suggestion I did the following:

-- A scalar octet count for the following hashed subpacket data.
+- A scalar octet count for the hashed subpacket data that follows this octet.

> -- Per “a pointer incremented by this number will skip over the unhashed subpackets”, what pointer is this talking about?

It is talking about an implementation, eg how you could (in C) increment
your pointer to skip over the following payload bits. I changed it to
"an implementation's pointer".

> ** Section 5.2.3.9
>    It is certainly possible for a signature to contain conflicting
>  information in subpackets. For example, a   signature may contain
>  multiple copies of a preference or multiple expiration times. In most
>  cases, an implementation SHOULD use the last subpacket in the hashed
>  section of the signature, but MAY use any conflict resolution scheme
>  that makes more sense.
>
> It appears that the WG was very deliberate in providing flexibility.  I’m concerned that this might lead to interoperability issues.  For example, let’s say there are multiple copies of preferences and one implementation decides to use the first subpacket and the another decides to use the last subpacket (as seems like would be permitted by “… but MAY use any conflict resolution strategy”).  Couldn’t that lead to divergent behavior?

This is something the WG should take up to answer.

> ** Section 5.2.3.10
>   A cryptographically-valid self-signature should be accepted from any primary key
>
> Is the intent here really “_must_ be accepted from any primary key”?

I think this leaves wiggle space in case of future cryptographic disasters. Eg let's
say a signature algorithm has been broken?

> ** Section 5.2.3.23
>  An application that wants the functionality of delegating revocation
>  SHOULD instead use an escrowed Revocation Signature.
>
> If this subpacket is deprecated, why is the guidance to use escrowed Revocation Signatures only framed as a “SHOULD”?  What is the alternative?

I think normative language is not needed here, changed to "has to use an escrowed Revocation Signature."

> ** Section 5.2.3.26.  What is the security implication of retrieving keys from the key server over ftp or http?

The authorization Key ID is protected. The obtained key has a
self-signature to prove itself. A MITM can mutilate the content
but it will just be discarded. It is the same as interfering
with a secure transport to a known key server's IP, eg by
blocking https to the (wellknown) keyserver. As it is already
stated that this method MUST NOT be used, I don't think we
need to elaborate on the security or privacy aspect of this method.

> ** Section 5.2.5.
>   * A known-weak hash algorithm (e.g. MD5)
>    …
>    When an implementation encounters such a malformed or unknown
>    signature, it MUST ignore the signature for validation purposes.
>
> The above text requires that the signature be ignored when it encounters a “known-weak hash algorithm”.  What is the interoperable way in which this would be determined (as opposed to a local policy decision)?

As there is no "negotiation" happening, the only way to ensure
interoperability is to preventively migrate and use MTI algorithms. If
an algorithm becomes weak (hopefully before being blatantly broken),
individuals can migrate to more modern and save algorythms. This also
assumes the IETF will still exist and be able to release timely RFC
updates :)

I don't think new or modified text is needed for this, but let us know
if you disagree.

> ** Section 5.3
> The versions differ in how they encrypt the session key with the passphrase, and in what they encode. The version of the SKESK packet must align with the version of the SEIPD packet (see Section 10.3.2.1). Any new version of the SKESK packet should be registered in the registry established in Section 10.3.2.1.
>
> Should normative MUST and SHOULDs be used in this text instead?

The first must I changed to MUST. The second should is really a note
to future implementers, so I don't think using normative language
makes much sense there? I changed it to say "New versions of the SKESK
packet will be registered in the registry established in [...].

> ** Section 5.3.2.  Editorial.
>  * A one-octet scalar octet count of the following 5 fields.
>  * A one-octet symmetric cipher algorithm identifier.
>  * A one-octet AEAD algorithm identifier.
>  * A one-octet scalar octet count of the following field.
>  * A string-to-key (S2K) specifier. The length of the string-to-key specifier depends on its type (see Section 3.7.1).
>
> Is “A one-octet scalar octet count of the following 5 fields” saying that it is the length count of those next 5 fields in the bulleted list?  If so, it would be helpful to be cleared about which “5 fields” they are.

Same fix as earlier applied.

> ** Section 5.5.2
>   There are four versions of key-material packets, two of which are
>   strongly deprecated.
>   OpenPGP implementations SHOULD create keys with version 6 format. V4
>   keys are deprecated; an implementation SHOULD NOT generate a v4 key,
>   but SHOULD accept it. V3 keys are deprecated; an implementation MUST
>   NOT generate a v3 key, but MAY accept it. V2 keys are deprecated; an
>   implementation MUST NOT generate a v2 key, but MAY accept it.
>
> In parsing all of these normative language relative to version numbers:
>
> -- What does “_strongly_ deprecated” mean as opposed to just “deprecated”?

Changed to:

There are four versions of key-material packets.  The V2 and V3 versions have
been deprecated since 1998. The V4 version has been deprecated by this
document in 2023.

> -- Is there an MTI key version?  I’ll note that the text provides guidance for generating and accepting for v2, 3, and 4.  But for v6, only guidance on creating a key is provided?  Should the text say that “OpenPGP implementations are RECOMMENDED to generate keys with version 6 format and MUST accept them”?

Defining MTI's is a bit tricky, as people want to be able to decrypt
their files from 30 years ago. That is why the document spends so much
text describing the older versions for implementers.

I don't think there is much value in saying "MUST accept v6" because
v6 is defined in this document, so if you are not accepting v6, you
are implementing RFC4880 and not RFC4880bis (this document).

> -- In trying to understanding “SHOULD create v6 keys” and “SHOULD NOT create v4 keys”.  What are the circumstances where v4 keys should continue to be used?  Is this guidance trying to say “given support for v4 and v6, implementations must prefer creating v6 keys assuming this wouldn’t create interoperability issues” (or “implementations that can create v6 keys MUST do so unless interoperability issues (determined through some out of band mechanism) with the recipient would arise”)

It is trying to say "for new things, use V6 and support older stuff when
needed based on the deployment". Your question of when v4 keys "should
continue to be used" is different from "creating keys". The advise is
for new keys to be the latest shiny most secure ones, with the caveat
of SHOULD being that if you need to talk to stuff that doesn't know v6,
you better create v4 keys. But the choice of creating v4 or v6 depends
on the environment of deployment, and not just the implementation you
yourself are writing or using.

I don't think a textual change is needed here, but let me know if you
disagree.

> ** Section 5.5.2
>   Any new Key version should be registered in the registry established in Section 10.3.2.2.

Changed to again avoid normative language or their lowercase
counterparts using:

> I appreciate that this is a lower case “should”, but is there really optionality in registering in this IANA table.  Maybe s/should be registered/must be registered/.

Any new Key version will be registered in the registry established in [...]

Although I'd also be happy to just remove these lines.

> ** Section 5.5.2.*.  Section 5.5.2 makes normative statements about “V2” keys.  However, the subsequent Sections 5.5.2.1 – 5.5.2.3 don’t describe what a “V2” is.  The table in Section 10.3.2.2 also omits it.

It does state: V2 keys are identical to the deprecated v3 keys except for the version number.
I pulled it up to the beginning of the section with a minor change:

V2 keys are identical to v3 keys except for the version number.

And added the v2 keys in the table in Section 10.3.2.2 pointing to the
V3 section.

> ** Section 5.5.3
> ==[ snip ]==
> * Conditionally included string-to-key parameter fields:
>     - If string-to-key usage octet was 255, 254, or 253, a one-octet symmetric encryption algorithm.
>     - If string-to-key usage octet was 253 (AEAD), a one-octet AEAD algorithm.
>    - Only for a version 6 packet, and if string-to-key usage octet was 255, 254, or 253, a one-octet count of the following field.
>    - If string-to-key usage octet was 255, 254, or 253, a string-to-key (S2K) specifier. The length of the string-to-key specifier depends on its type (see Section 3.7.1).
>    - If string-to-key usage octet was 253 (AEAD), an initialization vector (IV) of size specified by the AEAD algorithm (see Section 5.13.2), which is used as the nonce for the AEAD algorithm.
>    - If string-to-key usage octet was 255, 254, or a cipher algorithm identifier (that is, the secret data uses some form of CFB encryption), an initialization vector (IV) of the same length as the cipher's block size.
> ==[ snip ]==
>
> - Per “Only for a version 6 packet, and if string-to-key usage octet was 255, 254, or 253, a one-octet count of the following field”, are the “following field” the next 3x “if bullet points”?

Same fix as earlier.

> - Per “Only for a version 6 packet, and if string-to-key usage octet was 255, 254, or 253, a one-octet count of the following field”, what is a “one-octet count”? Is that a count of how many optional fields are included or the length of all of the conditional fields?

Fixed with "a one-octet count of the size of the one field following this octet."

> - Per “Only for a version 6 packet, and if string-to-key usage octet was 255, 254, or 253, a one-octet count of the following field”, the second bullet of this list says that V6 packets cannot use value 255 (i.e., “version 6 packet MUST NOT use the value 255 (MalleableCFB)”).  There also appears to be subsequent guidance about using V6 and 255.

Good one. I removed "255" from the list of "255, 254, or 253".

> ** Section 5.5.3
>   However, this checksum is deprecated; an implementation SHOULD NOT use it, but should rather use
>   the SHA-1 hash denoted with a usage octet of 254.
>
> Since this guidance on the checksum is “SHOULD NOT”, under what circumstances would it still be used despite the security concerns?

If your implementation needs to be able to use this older style private
key formats?

[ to be continued ]

Paul

> ** Section 5.5.3
>   When decrypting the secret key material using any of these schemes
>   (that is, where the usage octet is non-zero), the resulting cleartext
>   octet stream MUST be well-formed.  In particular, an implementation
>   MUST NOT interpret octets beyond the unwrapped cleartext octet stream
>   as part of any of the unwrapped MPI objects.  Furthermore, an
>   implementation MUST reject as unusable any secret key material whose
>   cleartext length does not align with the lengths of the unwrapped MPI
>   objects.
>
> Per “… MUST be well-formed”, as opposed to what?  What makes it well formed?  Is the normative guidance really the next two sentences?  If so, recommend dropping the “MUST” (using “must”) in the first sentence.
>
> ** Section 5.7
>   This packet is obsolete.  An implementation MUST NOT create this
>   packet.  An implementation MAY process such a packet but it MUST
>   return a clear diagnostic that a non-integrity protected packet has
>   been processed.  The implementation SHOULD also return an error in
>   this case and stop processing.
>
> -- What is the difference between the mandatory “diagnostic” and the optional “error”?
>
> -- The text seems to be saying that the “implementation MAY process the packet” but next says the “implementation SHOULD return an error … and stop processing”.  How should this guidance be combined?  Is the implementation permitted to process the packet or not?
>
> ** Section 5.9
>      Older versions of OpenPGP used t (0x74) to indicate textual data,
>      but did not specify the character encoding.  Implementations
>      SHOULD NOT emit this value.  An implementation that receives a
>      literal data packet with this value in the format field SHOULD
>      interpret the packet data as UTF-8 encoded text, unless reliable
>      (not attacker-controlled) context indicates a specific alternate
>      text encoding.  This mode is deprecated due to its ambiguity.
>               …
>      Text data MUST be encoded with UTF-8 (see [RFC3629]), and stored
>      with <CR><LF> text endings (that is, network-normal line endings).
>      These should be converted to native line endings by the receiving
>      software.
>
> The first paragraph seems to allow for sending data as non-UTF-8 by using ‘t’ (i.e., by using the language SHOULD NOT).  The other paragraph seems to be very strict in saying only UTF-8 can be used.  This seems to be a conflict.
>
> ** Section 5.9
>   Due to their inherent malleability, an implementation that generates
>   a literal data packet SHOULD avoid storing any significant data in
>   these fields.
>
> -- Is “significant data” here suggesting “something important” vs. a “large volume” of data?
>
> -- If the guidance is about not storing anything important, under what circumstances would that be avoided since the guidance is “SHOULD avoid” vs. “MUST avoid”?
>
> ** Section 5.9
>   If the implementation is certain that the data is
>   textual and is encoded with UTF-8 (for example, if it will follow
>   this literal data packet with a signature packet of type 0x01 (see
>   Section 5.2.1), it MAY set the format octet to u.  Otherwise, it
>   SHOULD set the format octet to b.
>
> -- If the implementation is “certain” the data is UTF-8, why not consistently use ‘u’?
>
> -- There seems to be a series of conditional clause with this text – MAY use ‘u’ and otherwise then  SHOULD use u.  Shouldn’t the SHOULD be a MUST?  Otherwise, if ‘u’ or b’ are not used, what should be?
>
> ** Section 5.13
>   This offers a more cryptographically rigorous defense against
>   ciphertext malleability, but may not be as widely supported yet.
>
> This strikes me as a bit like an implementation report.  Do you feel that it is really necessary and that it will age well?
>
> ** Section 5.13.2.
>   *  A one-octet cipher algorithm identifier.
>
>   *  A one-octet AEAD algorithm identifier.
>
> From what registries would these identifiers come?
>
> ** Section 5.13.2
>   *  Thirty-two octets of salt.  The salt is used to derive the message
>      key and must be unique.
>
> Unique relative to what?  Should this say something to the effect of “randomly generated with a strong PRNG”?  Isn’t this a normative requirement too – s/must be/MUST be/?
>
> ** Section 15.13.2.  I’m missing something foundational.  Where is the length of the ciphertext stored?
>
> ** Section 15.14.
>
>   Its contents SHOULD be random octets to make the length obfuscation
>   it provides more robust even when compressed.
>
> What would be the case for non-random octets?
>
> ** Section 6
>   When decoding base64, an OpenPGP implementation must ignore all white
>   space.
>
> s/implementations must ignore/implementations MUST ignore/
>
> ** Section 6
>   The OpenPGP standard specifies one such printable encoding scheme to
>   ensure interoperability.
>
> For clarity, it should be stated that this “printable encoding scheme” is called “ASCII Armor”.
>
> ** Section 6.1.1.  Please provide a normative reference to the C programming language
>
> ** Section 11.5
>
>   Refer to Section 11.5.1 for the details
>   regarding the choice of the KEK algorithm, which SHOULD be one of the
>   three AES algorithms.
>
> It appears that only AES algorithms are specified in Table 30 of Section 11.5.1.  What is the alternative to using AES?
>
> ** Section 13.  Editorial.
>
>   *  As with any technology involving cryptography, you should check
>      the current literature to determine if any algorithms used here
>      have been found to be vulnerable to an attack.
>
> Who is the “you”?  s/you should/users of an OpenPGP implementation should/
>
>
> ** Section 13.  Editorial.  Forward references:
>
>   *  The MD5 and SHA-1 hash algorithms have been found to have
>      weaknesses, with collisions found in a number of cases.  MD5 and
>      SHA-1 are deprecated for use in OpenPGP.
>
> Add a reference to Section 9.5
>
>   *  Many security protocol designers think that it is a bad idea to
>      use a single key for both privacy (encryption) and integrity
>      (signatures).  In fact, this was one of the motivating forces
>      behind the v4 key format with separate signature and encryption
>      keys.  Using a single key for encrypting and signing is
>      discouraged.
>
> Provide a citation for which backs the consensus of “many security protocol designers”.
>
> ** Section 13.6
>
>   When a human is actively involved, the result of such a verification
>   is dubious.  There is little evidence that most humans are good at
>   precise comparison of high-entropy data, particularly when that data
>   is represented in compact textual form like a hexadecimal
>   fingerprint.
>
> Is there a usability claim (citation) which backs this claim?
>
> ** Section 13.7
>
>   Implementers should implement AEAD (v2 SEIPD and S2K usage octet 253)
>   promptly and encourage its spread.
>
>   Users should migrate to AEAD with all due speed.
>
> These seem like unusual recommendations.  How are they recommendations in speed different than normative guidance that various functionality SHOULD be used?
>
> ** Section 13.8
>
>   Avoiding the pitfalls described above requires context-specific
>   expertise.  An implementation should only make use of the session-
>   key-reuse feature in any particular application layer when it can
>   follow reasonable documentation about how to deploy the feature
>   safely in the specific application.
>
> Could this text please be clarified?  Specifically, what is the origin of the “reasonable documentations”?  Where does the “context-specific expertise” come from?
>
> ** Section 13.10
>
>   In most cases, the operating system provides an
>   appropriate facility such as a getrandom() syscall
>
> Is this text talking about the Linux getrandom() system call?  Please be clear.  It’s been a while, but I think Win32 would be BCryptGenRandom()
>
> ** Section 14
>
>      OpenPGP permits an implementation to declare what
>      features it does and does not support, but ASCII armor is not one
>      of these.
>
> Could this guidance be clarified?  The text prior make clear that ASCII armor is not-MTI.  Why couldn’t an implementation declare that it doesn’t support it?
>
> ** Normative References
>
> --  (idnits) Obsolete normative reference: RFC 2822 (Obsoleted by RFC 5322)
>
>
> -- [HAC] is used for RIPEMD, MD5, and RSA in Section 9.*.  Instead use:
>
> RIPEMD-160 =
>   [RIPEMD-160]  3.ISO/IEC 10118-3:1998, "Information technology -
>                 Security techniques - Hash-functions - Part 3:
>                 Dedicated hash-functions," International Organization
>                 for Standardization, Geneva, Switzerland, 1998.
>
> MD5 = RFC1321 (“The MD5 Message-Digest Algorithm”)
>
>
> RSA = NIST FIPS 186-4
>
> -- for PKCS #5:
>
> OLD
>
>   [PKCS5]    RSA Laboratories, "PKCS #5 v2.0: Password-Based
>              Cryptography Standard", 25 March 1999.
>
> NEW
>   RFC2898
>
> -- for TripleDES:
>
> OLD
>   [SCHNEIER] Schneier, B., "Applied Cryptography Second Edition:
>              protocols, algorithms, and source code in C", 1996.
>
> NEW
>
> NIST Special Publication 800-67 Revision 2 (NIST SP800-67R2)
>
>
> -- Per Bzip2, citing a website as normative is going to be tricky.  Is there another reference?
>
>   [BZ2]      Seward, J., "The Bzip2 and libbzip2 home page", 2010,
>              <http://www.bzip.org/>.
>
> -- For these, better references are needed.  One can’t tell if these are conference proceedings, papers, etc.  It’s just an author list, title and year.  This is what I found using Google Scholar.
>
>   [EAX]      Bellare, M., Rogaway, P., and D. Wagner, "A Conventional
>              Authenticated-Encryption Mode", April 2003.
>
> ==[ bibtex]==
> @article{bellare2003conventional,
>  title={A conventional authenticated-encryption mode},
>  author={Bellare, Mihir and Rogaway, Phillip and Wagner, David},
>  journal={manuscript, April},
>  year={2003}
> }
> ==[ bibtex ]==
>
> https://seclab.cs.ucdavis.edu/papers/eax.pdf
>
>   [TWOFISH]  Schneier, B., Kelsey, J., Whiting, D., Wagner, D., Hall,
>              C., and N. Ferguson, "The Twofish Encryption Algorithm",
>              1999.
>
> Schneier B, Kelsey J, Whiting D, Wagner D, Hall C, Ferguson N (1998) Twofish: a 128-bit block cipher. Proceedings of the first AES candidate conference. National Institute of Standards and Technology.
>
> ==[ bibtex ]==
> @article{schneier1998twofish,
>  title={Twofish: A 128-bit block cipher},
>  author={Schneier, Bruce and Kelsey, John and Whiting, Doug and Wagner, David and Hall, Chris and Ferguson, Niels},
>  journal={NIST AES Proposal},
>  volume={15},
>  number={1},
>  pages={23--91},
>  year={1998}
> }
> ==[ bibtex ]==
>
> https://www.schneier.com/wp-content/uploads/2016/02/paper-twofish-paper.pdf
>
> Thanks,
> Roman
> _______________________________________________
> openpgp mailing list
> openpgp@ietf.org
> https://www.ietf.org/mailman/listinfo/openpgp