Add documentation for g10/parse-packet.c.

* g10/packet.h: Add documentation for functions defined in
parse-packet.c.
* g10/parse-packet.c: Improve comments for many functions.

--
Signed-off-by: Neal H. Walfield <neal@g10code.com>.

g10/packet.h

@@ -1,6 +1,7 @@
 /* packet.h - OpenPGP packet definitions
  * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006,
  *               2007 Free Software Foundation, Inc.
+ * Copyright (C) 2015 g10 Code GmbH
  *
  * This file is part of GnuPG.
  *
@@ -373,7 +374,7 @@ struct packet_struct {
 	PKT_pubkey_enc	*pubkey_enc;	/* PKT_PUBKEY_ENC */
 	PKT_onepass_sig *onepass_sig;	/* PKT_ONEPASS_SIG */
 	PKT_signature	*signature;	/* PKT_SIGNATURE */
-	PKT_public_key	*public_key;	/* PKT_PUBLIC_[SUB)KEY */
+	PKT_public_key	*public_key;	/* PKT_PUBLIC_[SUB]KEY */
 	PKT_public_key	*secret_key;	/* PKT_SECRET_[SUB]KEY */
 	PKT_comment	*comment;	/* PKT_COMMENT */
 	PKT_user_id	*user_id;	/* PKT_USER_ID */
@@ -417,9 +418,19 @@ int proc_encryption_packets (ctrl_t ctrl, void *ctx, iobuf_t a);
 int list_packets( iobuf_t a );
 
 /*-- parse-packet.c --*/
+
+/* Sets the packet list mode to MODE (i.e., whether we are dumping a
+   packet or not).  Returns the current mode.  This allows for
+   temporarily suspending dumping by doing the following:
+
+     int saved_mode = set_packet_list_mode (0);
+     ...
+     set_packet_list_mode (saved_mode);
+*/
 int set_packet_list_mode( int mode );
 
 #if DEBUG_PARSE_PACKET
+/* There are debug functions and should not be used directly.  */
 int dbg_search_packet( iobuf_t inp, PACKET *pkt, off_t *retpos, int with_uid,
                        const char* file, int lineno  );
 int dbg_parse_packet( iobuf_t inp, PACKET *ret_pkt,
@@ -441,27 +452,147 @@ int dbg_skip_some_packets( iobuf_t inp, unsigned n,
 #define skip_some_packets( a,b ) \
              dbg_skip_some_packets((a),(b), __FILE__, __LINE__ )
 #else
+/* Return the next valid OpenPGP packet in *PKT.  (This function will
+   skip any packets whose type is 0.)
+
+   Returns 0 on success, -1 if EOF is reached, and an error code
+   otherwise.  In the case of an error, the packet in *PKT may be
+   partially constructed.  As such, even if there is an error, it is
+   necessary to free *PKT to avoid a resource leak.  To detect what
+   has been allocated, clear *PKT before calling this function.  */
+int parse_packet( iobuf_t inp, PACKET *pkt);
+
+/* Return the first OpenPGP packet in *PKT that contains a key (either
+   a public subkey, a public key, a secret subkey or a secret key) or,
+   if WITH_UID is set, a user id.
+
+   Saves the position in the pipeline of the start of the returned
+   packet (according to iobuf_tell) in RETPOS, if it is not NULL.
+
+   The return semantics are the same as parse_packet.  */
 int search_packet( iobuf_t inp, PACKET *pkt, off_t *retpos, int with_uid );
-int parse_packet( iobuf_t inp, PACKET *ret_pkt);
+
+/* Copy all packets (except invalid packets, i.e., those with a type
+   of 0) from INP to OUT until either an error occurs or EOF is
+   reached.
+
+   Returns -1 when end of file is reached or an error code, if an
+   error occured.  (Note: this function never returns 0, because it
+   effectively keeps going until it gets an EOF.)  */
 int copy_all_packets( iobuf_t inp, iobuf_t out );
+
+/* Like copy_all_packets, but stops at the first packet that starts at
+   or after STOPOFF (as indicated by iobuf_tell).
+
+   Example: if STOPOFF is 100, the first packet in INP goes from 0 to
+   110 and the next packet starts at offset 111, then the packet
+   starting at offset 0 will be completely processed (even though it
+   extends beyond STOPOFF) and the packet starting at offset 111 will
+   not be processed at all.  */
 int copy_some_packets( iobuf_t inp, iobuf_t out, off_t stopoff );
+
+/* Skips the next N packets from INP.
+
+   If parsing a packet returns an error code, then the function stops
+   immediately and returns the error code.  Note: in the case of an
+   error, this function does not indicate how many packets were
+   successfully processed.  */
 int skip_some_packets( iobuf_t inp, unsigned n );
 #endif
 
+/* Parse a signature packet and store it in *SIG.
+
+   The signature packet is read from INP.  The OpenPGP header (the tag
+   and the packet's length) have already been read; the next byte read
+   from INP should be the first byte of the packet's contents.  The
+   packet's type (as extract from the tag) must be passed as PKTTYPE
+   and the packet's length must be passed as PKTLEN.  This is used as
+   the upper bound on the amount of data read from INP.  If the packet
+   is shorter than PKTLEN, the data at the end will be silently
+   skipped.  If an error occurs, an error code will be returned.  -1
+   means the EOF was encountered.  0 means parsing was successful.  */
 int parse_signature( iobuf_t inp, int pkttype, unsigned long pktlen,
 		     PKT_signature *sig );
+
+/* Given a subpacket area (typically either PKT_signature.hashed or
+   PKT_signature.unhashed), either:
+
+     - test whether there are any subpackets with the critical bit set
+       that we don't understand,
+
+     - list the subpackets, or,
+
+     - find a subpacket with a specific type.
+
+   REQTYPE indicates the type of operation.
+
+   If REQTYPE is SIGSUBPKT_TEST_CRITICAL, then this function checks
+   whether there are any subpackets that have the critical bit and
+   which GnuPG cannot handle.  If GnuPG understands all subpackets
+   whose critical bit is set, then this function returns simply
+   returns SUBPKTS.  If there is a subpacket whose critical bit is set
+   and which GnuPG does not understand, then this function returns
+   NULL and, if START is not NULL, sets *START to the 1-based index of
+   the subpacket that violates the constraint.
+
+   If REQTYPE is SIGSUBPKT_LIST_HASHED or SIGSUBPKT_LIST_UNHASHED, the
+   packets are dumped.  Note: if REQTYPE is SIGSUBPKT_LIST_HASHED,
+   this function does not check whether the hash is correct; this is
+   merely an indication of the section that the subpackets came from.
+
+   If REQTYPE is anything else, then this function interprets the
+   values as a subpacket type and looks for the first subpacket with
+   that type.  If such a packet is found, *CRITICAL (if not NULL) is
+   set if the critical bit was set, *RET_N is set to the offset of the
+   subpacket's content within the SUBPKTS buffer, *START is set to the
+   1-based index of the subpacket within the buffer, and returns
+   &SUBPKTS[*RET_N].
+
+   *START is the number of initial subpackets to not consider.  Thus,
+   if *START is 2, then the first 2 subpackets are ignored.  */
 const byte *enum_sig_subpkt ( const subpktarea_t *subpkts,
                               sigsubpkttype_t reqtype,
                               size_t *ret_n, int *start, int *critical );
+
+/* Shorthand for:
+
+     enum_sig_subpkt (buffer, reqtype, ret_n, NULL, NULL); */
 const byte *parse_sig_subpkt ( const subpktarea_t *buffer,
                                sigsubpkttype_t reqtype,
                                size_t *ret_n );
+
+/* This calls parse_sig_subpkt first on the hashed signature area in
+   SIG and then, if that returns NULL, calls parse_sig_subpkt on the
+   unhashed subpacket area in SIG.  */
 const byte *parse_sig_subpkt2 ( PKT_signature *sig,
                                 sigsubpkttype_t reqtype);
+
+/* Returns whether the N byte large buffer BUFFER is sufficient to
+   hold a subpacket of type TYPE.  Note: the buffer refers to the
+   contents of the subpacket (not the header) and it must already be
+   initialized: for some subpackets, it checks some internal
+   constraints.
+
+   Returns 0 if the size is acceptable.  Returns -2 if the buffer is
+   definately too short.  To check for an error, check whether the
+   return value is less than 0.  */
 int parse_one_sig_subpkt( const byte *buffer, size_t n, int type );
+
+/* Looks for revocation key subpackets (see RFC 4880 5.2.3.15) in the
+   hashed area of the signature packet.  Any that are found are added
+   to SIG->REVKEY and SIG->NUMREVKEYS is updated appropriately.  */
 void parse_revkeys(PKT_signature *sig);
+
+/* Extract the attributes from the buffer at UID->ATTRIB_DATA and
+   update UID->ATTRIBS and UID->NUMATTRIBS accordingly.  */
 int parse_attribute_subpkts(PKT_user_id *uid);
+
+/* Set the UID->NAME field according to the attributes.  MAX_NAMELEN
+   must be at least 71.  */
 void make_attribute_uidname(PKT_user_id *uid, size_t max_namelen);
+
+/* Allocate and initialize a new GPG control packet.  DATA is the data
+   to save in the packet.  */
 PACKET *create_gpg_control ( ctrlpkttype_t type,
                              const byte *data,
                              size_t datalen );

g10/parse-packet.c

@@ -88,6 +88,7 @@ static int parse_mdc (IOBUF inp, int pkttype, unsigned long pktlen,
 static int parse_gpg_control (IOBUF inp, int pkttype, unsigned long pktlen,
 			      PACKET * packet, int partial);
 
+/* Read a 16-bit value in MSB order (big endian) from an iobuf.  */
 static unsigned short
 read_16 (IOBUF inp)
 {
@@ -98,6 +99,7 @@ read_16 (IOBUF inp)
 }
 
 
+/* Read a 32-bit value in MSB order (big endian) from an iobuf.  */
 static unsigned long
 read_32 (IOBUF inp)
 {
@@ -226,6 +228,8 @@ set_packet_list_mode (int mode)
 }
 
 
+/* If OPT.VERBOSE is set, print a warning that the algorithm ALGO is
+   not suitable for signing and encryption.  */
 static void
 unknown_pubkey_warning (int algo)
 {
@@ -258,12 +262,6 @@ unknown_pubkey_warning (int algo)
 }
 
 
-/* Parse a packet and return it in packet structure.
- * Returns: 0 := valid packet in pkt
- *	   -1 := no more packets
- *	   >0 := error
- * Note: The function may return an error and a partly valid packet;
- * caller must free this packet.   */
 #ifdef DEBUG_PARSE_PACKET
 int
 dbg_parse_packet (IOBUF inp, PACKET *pkt, const char *dbg_f, int dbg_l)
@@ -437,11 +435,32 @@ skip_some_packets (IOBUF inp, unsigned n)
 #endif /*!DEBUG_PARSE_PACKET*/
 
 
-/*
+/* Parse a packet and save it in *PKT.
- * Parse packet.  Stores 1 at SKIP 1 if the packet should be skipped;
+
- * this is the case if either ONLYKEYPKTS is set and the parsed packet
+   If OUT is not NULL and the packet is valid (its type is not 0),
- * isn't a key packet or the packet-type is 0, indicating deleted
+   then the header, the initial length field and the packet's contents
- * stuff.  If OUT is not NULL, a special copymode is used.
+   are written to OUT.  In this case, the packet is not saved in *PKT.
+
+   ONLYKEYPKTS is a simple packet filter.  If ONLYKEYPKTS is set to 1,
+   then only public subkey packets, public key packets, private subkey
+   packets and private key packets are parsed.  The rest are skipped
+   (i.e., the header and the contents are read from the pipeline and
+   discarded).  If ONLYKEYPKTS is set to 2, then in addition to the
+   above 4 types of packets, user id packets are also accepted.
+
+   DO_SKIP is a more coarse grained filter.  Unless ONLYKEYPKTS is set
+   to 2 and the packet is a user id packet, all packets are skipped.
+
+   Finally, if a packet is invalid (it's type is 0), it is skipped.
+
+   If a packet is skipped and SKIP is not NULL, then *SKIP is set to
+   1.
+
+   Note: ONLYKEYPKTS and DO_SKIP are only respected if OUT is NULL,
+   i.e., the packets are not simply being copied.
+
+   If RETPOS is not NULL, then the position of INP (as returned by
+   iobuf_tell) is saved there before any data is read from INP.
   */
 static int
 parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
@@ -470,6 +489,8 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
   else
     pos = 0; /* (silence compiler warning) */
 
+  /* The first byte of a packet is the so-called tag.  The highest bit
+     must be set.  */
   if ((ctb = iobuf_get (inp)) == -1)
     {
       rc = -1;
@@ -477,17 +498,31 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
     }
   hdrlen = 0;
   hdr[hdrlen++] = ctb;
+
   if (!(ctb & 0x80))
     {
       log_error ("%s: invalid packet (ctb=%02x)\n", iobuf_where (inp), ctb);
       rc = gpg_error (GPG_ERR_INV_PACKET);
       goto leave;
     }
+
+  /* Immediately following the header is the length.  There are two
+     formats: the old format and the new format.  If bit 6 (where the
+     least significant bit is bit 0) is set in the tag, then we are
+     dealing with a new format packet.  Otherwise, it is an old format
+     packet.  */
   pktlen = 0;
   new_ctb = !!(ctb & 0x40);
   if (new_ctb)
     {
+      /* Get the packet's type.  This is encoded in the 6 least
+	 significant bits of the tag.  */
       pkttype = ctb & 0x3f;
+
+      /* Extract the packet's length.  New format packets have 4 ways
+	 to encode the packet length.  The value of the first byte
+	 determines the encoding and partially determines the length.
+	 See section 4.2.2 of RFC 4880 for details.  */
       if ((c = iobuf_get (inp)) == -1)
 	{
 	  log_error ("%s: 1st length byte missing\n", iobuf_where (inp));
@@ -539,7 +574,7 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
               break;
 
             default:
-              log_error ("%s: partial length for invalid"
+              log_error ("%s: partial length invalid for"
                          " packet type %d\n", iobuf_where (inp), pkttype);
               rc = gpg_error (GPG_ERR_INV_PACKET);
               goto leave;
@@ -548,8 +583,13 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
 
     }
   else
+    /* This is an old format packet.  */
     {
+      /* Extract the packet's type.  This is encoded in bits 2-5.  */
       pkttype = (ctb >> 2) & 0xf;
+
+      /* The type of length encoding is encoded in bits 0-1 of the
+	 tag.  */
       lenbytes = ((ctb & 3) == 3) ? 0 : (1 << (ctb & 3));
       if (!lenbytes)
 	{
@@ -594,9 +634,13 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
     }
 
   if (with_uid && pkttype == PKT_USER_ID)
+    /* If ONLYKEYPKTS is set to 2, then we never skip user id packets,
+       even if DO_SKIP is set.  */
     ;
   else if (do_skip
+	   /* type==0 is not allowed.  This is an invalid packet.  */
 	   || !pkttype
+	   /* When ONLYKEYPKTS is set, we don't skip keys.  */
 	   || (onlykeypkts && pkttype != PKT_PUBLIC_SUBKEY
 	       && pkttype != PKT_PUBLIC_KEY
 	       && pkttype != PKT_SECRET_SUBKEY && pkttype != PKT_SECRET_KEY))
@@ -686,12 +730,13 @@ parse (IOBUF inp, PACKET * pkt, int onlykeypkts, off_t * retpos,
       rc = parse_marker (inp, pkttype, pktlen);
       break;
     default:
+      /* Unknown packet.  Skip it.  */
       skip_packet (inp, pkttype, pktlen, partial);
       break;
     }
 
  leave:
-  /* FIXME: Do we leak in case of an error?  */
+  /* FIXME: We leak in case of an error (see the xmalloc's above).  */
   if (!rc && iobuf_error (inp))
     rc = GPG_ERR_INV_KEYRING;
 
@@ -720,6 +765,20 @@ dump_hex_line (int c, int *i)
 }
 
 
+/* Copy the contents of a packet from the pipeline IN to the pipeline
+   OUT.
+
+   The header and length have already been read from INP and the
+   decoded values are given as PKGTYPE and PKTLEN.
+
+   If the packet is a partial body length packet (RFC 4880, Section
+   4.2.2.4), then iobuf_set_partial_block_mode should already have
+   been called on INP and PARTIAL should be set.
+
+   If PARTIAL is set or PKTLEN is 0 and PKTTYPE is PKT_COMPRESSED,
+   copy until the first EOF is encountered on INP.
+
+   Returns 0 on success and an error code if an error occurs.  */
 static int
 copy_packet (IOBUF inp, IOBUF out, int pkttype,
 	     unsigned long pktlen, int partial)
@@ -758,6 +817,9 @@ copy_packet (IOBUF inp, IOBUF out, int pkttype,
 }
 
 
+/* Skip an unknown packet.  PKTTYPE is the packet's type, PKTLEN is
+   the length of the packet's content and PARTIAL is whether partial
+   body length encoding in used (in this case PKTLEN is ignored).  */
 static void
 skip_packet (IOBUF inp, int pkttype, unsigned long pktlen, int partial)
 {
@@ -792,8 +854,9 @@ skip_packet (IOBUF inp, int pkttype, unsigned long pktlen, int partial)
 
 
 /* Read PKTLEN bytes form INP and return them in a newly allocated
-   buffer.  In case of an error NULL is returned and a error messages
+   buffer.  In case of an error (including reading fewer than PKTLEN
-   printed.  */
+   bytes from INP before EOF is returned), NULL is returned and an
+   error message is logged.  */
 static void *
 read_rest (IOBUF inp, size_t pktlen)
 {
@@ -1124,6 +1187,12 @@ parse_pubkeyenc (IOBUF inp, int pkttype, unsigned long pktlen,
 }
 
 
+/* Dump a subpacket to LISTFP.  BUFFER contains the subpacket in
+   question and points to the type field in the subpacket header (not
+   the start of the header).  TYPE is the subpacket's type with the
+   critical bit cleared.  CRITICAL is the value of the CRITICAL bit.
+   BUFLEN is the length of the buffer and LENGTH is the length of the
+   subpacket according to the subpacket's header.  */
 static void
 dump_sig_subpkt (int hashed, int type, int critical,
 		 const byte * buffer, size_t buflen, size_t length)
@@ -1560,7 +1629,11 @@ enum_sig_subpkt (const subpktarea_t * pktbuf, sigsubpkttype_t reqtype,
       buflen -= n;
     }
   if (reqtype == SIGSUBPKT_TEST_CRITICAL)
-    return buffer;  /* Used as True to indicate that there is no. */
+    /* Returning NULL means we found a subpacket with the critical bit
+       set that we dn't grok.  We've iterated over all the subpackets
+       and haven't found such a packet so we need to return a non-NULL
+       value.  */
+    return buffer;
 
   /* Critical bit we don't understand. */
   if (start)