Simplify grammar checker

src/packet/grammar.ts

@@ -15,20 +15,20 @@ export class GrammarError extends Error {
 }
 
 
-const isValidLiteralMessage = (tagList: enums.packet[], _acceptPartial: boolean) => tagList.length === 1 && tagList[0] === enums.packet.literalData;
+const isValidLiteralMessage = (tagList: enums.packet[]) => tagList.length === 1 && tagList[0] === enums.packet.literalData;
-const isValidCompressedMessage = (tagList: enums.packet[], _acceptPartial: boolean) => tagList.length === 1 && tagList[0] === enums.packet.compressedData;
+const isValidCompressedMessage = (tagList: enums.packet[]) => tagList.length === 1 && tagList[0] === enums.packet.compressedData;
-const isValidEncryptedMessage = (tagList: enums.packet[], acceptPartial: boolean) => {
+const isValidEncryptedMessage = (tagList: enums.packet[]) => {
   // Encrypted Message: Encrypted Data | ESK Sequence, Encrypted Data.
-  const isValidESKSequence = (tagList: enums.packet[], _acceptPartial: boolean) => (
+  const isValidESKSequence = (tagList: enums.packet[]) => (
     tagList.every(packetTag => new Set([enums.packet.publicKeyEncryptedSessionKey, enums.packet.symEncryptedSessionKey]).has(packetTag))
   );
   const encryptedDataPacketIndex = tagList.findIndex(tag => new Set([enums.packet.aeadEncryptedData, enums.packet.symmetricallyEncryptedData, enums.packet.symEncryptedIntegrityProtectedData]).has(tag));
   if (encryptedDataPacketIndex < 0) {
-    return isValidESKSequence(tagList, acceptPartial);
+    return isValidESKSequence(tagList);
   }
 
   return (encryptedDataPacketIndex === tagList.length - 1) &&
-    isValidESKSequence(tagList.slice(0, encryptedDataPacketIndex), acceptPartial);
+    isValidESKSequence(tagList.slice(0, encryptedDataPacketIndex));
 };
 
 const isValidSignedMessage = (tagList: enums.packet[], acceptPartial: boolean) => {
@@ -50,39 +50,20 @@ const isValidSignedMessage = (tagList: enums.packet[], acceptPartial: boolean) =
   return false;
 };
 
-const isUnknownPacketTag = (tag: number): tag is enums.packet => {
-  try {
-    enums.read(enums.packet, tag);
-    return false;
-  } catch (e) {
-    return true;
-  }
-};
-
 /**
  * Implements grammar checks based on https://www.rfc-editor.org/rfc/rfc9580.html#section-10.3 .
- * @param notNormalizedList - list of packet tags to validate
+ * @param packetList - list of packet tags to validate
  * @param acceptPartial - whether the list of tags corresponds to a partially-parsed message
  * @returns whether the list of tags is valid
  */
 const isValidOpenPGPMessage = (
-  notNormalizedList: number[] /** might have unknown tags */,
+  packetList: number[],
   acceptPartial: boolean
 ): boolean => {
-  // Take care of packet tags that can appear anywhere in the sequence:
+  return isValidLiteralMessage(packetList) ||
-  // 1. A Marker packet (Section 5.8) can appear anywhere in the sequence.
+    isValidCompressedMessage(packetList) ||
-  // 2. An implementation MUST be able to process Padding packets anywhere else in an OpenPGP stream so that future revisions of this document may specify further locations for padding.
+    isValidEncryptedMessage(packetList) ||
-  // 3. An unknown non-critical packet MUST be ignored (criticality is enforced on parsing).
+    isValidSignedMessage(packetList, acceptPartial);
-  const normalizedList: enums.packet[] = notNormalizedList.filter(tag => (
-    tag !== enums.packet.marker &&
-    tag !== enums.packet.padding &&
-      !isUnknownPacketTag(tag)
-  ));
-
-  return isValidLiteralMessage(normalizedList, acceptPartial) ||
-    isValidCompressedMessage(normalizedList, acceptPartial) ||
-    isValidEncryptedMessage(normalizedList, acceptPartial) ||
-    isValidSignedMessage(normalizedList, acceptPartial);
 };
 
 /**
@@ -95,7 +76,6 @@ const isValidOpenPGPMessage = (
 export const getMessageGrammarValidator = ({ delayReporting }: { delayReporting: boolean }) => {
   let logged = false;
 
-
   /**
   * @returns `true` on successful grammar validation; if `delayReporting` is set, `null` is returned
   *   if validation is still pending (partial parsing, waiting for authentication to be confirmed).

src/packet/packetlist.js

@@ -69,9 +69,9 @@ class PacketList extends Array {
     if (config.additionalAllowedPackets.length) {
       allowedPackets = { ...allowedPackets, ...util.constructAllowedPackets(config.additionalAllowedPackets) };
     }
+    const tagsRead = [];
     this.stream = streamTransformPair(bytes, async (readable, writable) => {
       const writer = streamGetWriter(writable);
-      const writtenTags = [];
       try {
         while (true) {
           await writer.ready;
@@ -85,6 +85,9 @@ class PacketList extends Array {
                 return;
               }
               const packet = newPacketFromTag(parsed.tag, allowedPackets);
+              // Unknown packets throw in the call above, we ignore them
+              // in the grammar checker.
+              tagsRead.push(parsed.tag);
               packet.packets = new PacketList();
               packet.fromStream = util.isStream(parsed.packet);
               try {
@@ -96,12 +99,6 @@ class PacketList extends Array {
                 throw e;
               }
               await writer.write(packet);
-              writtenTags.push(parsed.tag);
-              // The `writtenTags` are only sensitive if we are parsing an _unauthenticated_ decrypted stream,
-              // since they can enable an decryption oracle.
-              // It's responsibility of the caller to pass a `grammarValidator` that takes care of
-              // postponing error reporting until the data has been authenticated.
-              grammarValidator?.(writtenTags, true, config);
             } catch (e) {
               // If an implementation encounters a critical packet where the packet type is unknown in a packet sequence,
               // it MUST reject the whole packet sequence. On the other hand, an unknown non-critical packet MUST be ignored.
@@ -141,8 +138,6 @@ class PacketList extends Array {
               } else {
                 const unparsedPacket = new UnparseablePacket(parsed.tag, parsed.packet);
                 await writer.write(unparsedPacket);
-                writtenTags.push(parsed.tag);
-                grammarValidator?.(writtenTags, true, config);
               }
               util.printDebugError(e);
             }
@@ -150,7 +145,7 @@ class PacketList extends Array {
           if (done) {
             // Here we are past the MDC check for SEIPDv1 data, hence
             // the data is always authenticated at this point.
-            grammarValidator?.(writtenTags, false, config);
+            grammarValidator?.(tagsRead, false, config);
             await writer.ready;
             await writer.close();
             return;
@@ -169,8 +164,14 @@ class PacketList extends Array {
         this.push(value);
       } else {
         this.stream = null;
+        break;
       }
-      if (done || supportsStreaming(value.constructor.tag)) {
+      if (supportsStreaming(value.constructor.tag)) {
+        // The `tagsRead` are only sensitive if we are parsing an _unauthenticated_ decrypted stream,
+        // since they can enable an decryption oracle.
+        // It's responsibility of the caller to pass a `grammarValidator` that takes care of
+        // postponing error reporting until the data has been authenticated.
+        grammarValidator?.(tagsRead, true, config);
         break;
       }
     }