openpgp: Accept unknown packets in production rules.

  - We validate certificate structures based on packet tags.  In the
    past, this lead to problems where a secret key packet was parsed
    to an unknown packet because the secret bits were malformed.  This
    lead to a crash in the generated parser because it was assuming to
    see a secret key packet, but got an unknown packet.

  - This was changed in cd5eb82edfb326d7cbde29ee105f9f88e045c240 so
    that we validate the certificate structure based on packet kinds,
    i.e. we would only consider a packet a secret key packet if we
    managed to parse it into one.  However, this caused the parser to
    be overly strict, causing problems with forward compatibility, and
    the parser to return an Error::MalformedCert instead of an
    Error::UnsupportedCert (see #170).

  - Return to validating on packet tags, but make the parser code
    aware that we may have parsed some packets (like secret key
    packets) to unknown packets.

  - This effectively reverts commit
    cd5eb82edfb326d7cbde29ee105f9f88e045c240.

  - Fixes #170.

4 files changed, 79 insertions, 36 deletions

diff --git a/openpgp/src/cert/parser/low_level/grammar.lalrpop b/openpgp/src/cert/parser/low_level/grammar.lalrpop
index d81ff6e3..59462c8c 100644
--- a/openpgp/src/cert/parser/low_level/grammar.lalrpop
+++ b/openpgp/src/cert/parser/low_level/grammar.lalrpop
@@ -1,16 +1,21 @@
 // -*- mode: Rust; -*-
 
+use std::convert::TryInto;
+
 use crate::Error;
+use crate::packet::Any;
 use crate::packet::Signature;
 use crate::packet::UserID;
 use crate::packet::UserAttribute;
 use crate::packet::{key, Key};
 use crate::packet::Unknown;
 use crate::Packet;
+use crate::policy::HashAlgoSecurity::SecondPreImageResistance;
 use crate::cert::prelude::*;
 
 use crate::cert::parser::low_level::lexer;
 use crate::cert::parser::low_level::lexer::{Token, Component};
+use crate::cert::parser::low_level::grammar_util::*;
 use crate::cert::ComponentBundles;
 use crate::cert::bundle::{
     PrimaryKeyBundle,
@@ -157,7 +162,7 @@ OptionalComponents: Option<Vec<Component>> = {
 Component: Option<Component> = {
     <key:Subkey> <sigs:OptionalSignatures> => {
         match key {
-            Some(key) => {
+            Some(Ok(key)) => {
                 let sigs = sigs.unwrap();
                 let sec = key.hash_algo_security();
 
@@ -171,13 +176,22 @@ Component: Option<Component> = {
                     other_revocations: vec![],
                 }))
             },
+            Some(Err(u)) => Some(Component::UnknownBundle(UnknownBundle {
+                component: u,
+                hash_algo_security: SecondPreImageResistance,
+                self_signatures: vec![],
+                certifications: sigs.unwrap_or_default(),
+                attestations: vec![],
+                self_revocations: vec![],
+                other_revocations: vec![],
+            })),
             // Just validating a cert...
             None => None,
         }
     },
     <u:UserID> <sigs:OptionalSignatures> => {
         match u {
-            Some(u) => {
+            Some(Ok(u)) => {
                 let sigs = sigs.unwrap();
                 let sec = u.hash_algo_security();
 
@@ -191,13 +205,22 @@ Component: Option<Component> = {
                     other_revocations: vec![],
                 }))
             },
+            Some(Err(u)) => Some(Component::UnknownBundle(UnknownBundle {
+                component: u,
+                hash_algo_security: SecondPreImageResistance,
+                self_signatures: vec![],
+                certifications: sigs.unwrap_or_default(),
+                attestations: vec![],
+                self_revocations: vec![],
+                other_revocations: vec![],
+            })),
             // Just validating a cert...
             None => None,
         }
     },
     <u:UserAttribute> <sigs:OptionalSignatures> => {
         match u {
-            Some(u) => {
+            Some(Ok(u)) => {
                 let sigs = sigs.unwrap();
                 let sec = u.hash_algo_security();
 
@@ -211,6 +234,15 @@ Component: Option<Component> = {
                     other_revocations: vec![],
                 }))
             },
+            Some(Err(u)) => Some(Component::UnknownBundle(UnknownBundle {
+                component: u,
+                hash_algo_security: SecondPreImageResistance,
+                self_signatures: vec![],
+                certifications: sigs.unwrap_or_default(),
+                attestations: vec![],
+                self_revocations: vec![],
+                other_revocations: vec![],
+            })),
             // Just validating a cert...
             None => None,
         }
@@ -237,59 +269,55 @@ Component: Option<Component> = {
     },
 }
 
-Subkey: Option<Key<key::PublicParts, key::SubordinateRole>> = {
+Subkey: Option<PacketOrUnknown<Key<key::PublicParts, key::SubordinateRole>>> = {
     <t:PUBLIC_SUBKEY> => {
-        match t.into() {
-            Some(Packet::PublicSubkey(key)) => Some(key),
+        match Option::<Packet>::from(t) {
+            Some(p) => Some(p.downcast().map_err(
+                |p| p.try_into().expect("infallible for unknown and this packet"))),
             // Just validating a cert...
             None => None,
-            Some(pkt) =>
-              unreachable!("Expected public subkey packet, got {:?}", pkt),
         }
     },
     <t:SECRET_SUBKEY> => {
-        match t.into() {
-            Some(Packet::SecretSubkey(key)) => Some(key.parts_into_public()),
+        match Option::<Packet>::from(t) {
+            Some(p) => Some(Any::<key::SecretSubkey>::downcast(p)
+                .map_err(
+                    |p| p.try_into().expect("infallible for unknown and this packet"))
+                .map(|sk| sk.parts_into_public())),
             // Just validating a cert...
             None => None,
-            Some(pkt) =>
-              unreachable!("Expected secret subkey packet, got {:?}", pkt),
         }
     },
 }
 
-UserID: Option<UserID> = {
+UserID: Option<PacketOrUnknown<UserID>> = {
     <t:USERID> => {
-        match t.into() {
-            Some(Packet::UserID(u)) => Some(u),
+        match Option::<Packet>::from(t) {
+            Some(p) => Some(p.downcast().map_err(
+                |p| p.try_into().expect("infallible for unknown and this packet"))),
             // Just validating a cert...
             None => None,
-            Some(pkt) =>
-              unreachable!("Expected user id packet, got {:?}", pkt),
         }
     },
 }
 
-UserAttribute: Option<UserAttribute> = {
+UserAttribute: Option<PacketOrUnknown<UserAttribute>> = {
     <t:USER_ATTRIBUTE> => {
-        match t.into() {
-            Some(Packet::UserAttribute(u)) => Some(u),
+        match Option::<Packet>::from(t) {
+            Some(p) => Some(p.downcast().map_err(
+                |p| p.try_into().expect("infallible for unknown and this packet"))),
             // Just validating a cert...
             None => None,
-            Some(pkt) =>
-              unreachable!("Expected user attribute packet, got {:?}", pkt),
         }
     },
 }
 
 Unknown: Option<Unknown> = {
     <t:UNKNOWN> => {
-        match t.into() {
-            Some(Packet::Unknown(u)) => Some(u),
+        match Option::<Packet>::from(t) {
+            Some(p) => p.try_into().ok(),
             // Just validating a cert...
             None => None,
-            Some(pkt) =>
-              unreachable!("Expected unknown packet, got {:?}", pkt),
         }
     },
 }
diff --git a/openpgp/src/cert/parser/low_level/grammar_util.rs b/openpgp/src/cert/parser/low_level/grammar_util.rs
new file mode 100644
index 00000000..8d4aa529
--- /dev/null
+++ b/openpgp/src/cert/parser/low_level/grammar_util.rs
@@ -0,0 +1,12 @@
+//! Contains utility functions and definitions for the grammar.
+
+use crate::{
+    packet::Unknown,
+};
+
+/// A local alias that is either a concrete packet body or a (likely
+/// Unknown) packet.
+///
+/// This is used in the parser grammar, but cannot be defined there.
+/// lalrpop's parser doesn't allow polymorphic type aliases.
+pub type PacketOrUnknown<T> = std::result::Result<T, Unknown>;
diff --git a/openpgp/src/cert/parser/low_level/lexer.rs b/openpgp/src/cert/parser/low_level/lexer.rs
index 29639271..0838d2b1 100644
--- a/openpgp/src/cert/parser/low_level/lexer.rs
+++ b/openpgp/src/cert/parser/low_level/lexer.rs
@@ -114,16 +114,17 @@ impl From<Token> for Option<Packet> {
 
 impl From<Packet> for Option<Token> {
     fn from(p: Packet) -> Self {
-        match p {
-            p @ Packet::PublicKey(_) => Some(Token::PublicKey(Some(p))),
-            p @ Packet::SecretKey(_) => Some(Token::SecretKey(Some(p))),
-            p @ Packet::PublicSubkey(_) => Some(Token::PublicSubkey(Some(p))),
-            p @ Packet::SecretSubkey(_) => Some(Token::SecretSubkey(Some(p))),
-            p @ Packet::UserID(_) => Some(Token::UserID(Some(p))),
-            p @ Packet::UserAttribute(_) => Some(Token::UserAttribute(Some(p))),
-            p @ Packet::Signature(_) => Some(Token::Signature(Some(p))),
-            p @ Packet::Trust(_) => Some(Token::Trust(Some(p))),
-            p @ Packet::Unknown(_) => Some(Token::Unknown(p.tag(), Some(p))),
+        match p.tag() {
+            Tag::PublicKey => Some(Token::PublicKey(Some(p))),
+            Tag::SecretKey => Some(Token::SecretKey(Some(p))),
+            Tag::PublicSubkey => Some(Token::PublicSubkey(Some(p))),
+            Tag::SecretSubkey => Some(Token::SecretSubkey(Some(p))),
+            Tag::UserID => Some(Token::UserID(Some(p))),
+            Tag::UserAttribute => Some(Token::UserAttribute(Some(p))),
+            Tag::Signature => Some(Token::Signature(Some(p))),
+            Tag::Trust => Some(Token::Trust(Some(p))),
+            t @ Tag::Unknown(_) => Some(Token::Unknown(t, Some(p))),
+            t @ Tag::Private(_) => Some(Token::Unknown(t, Some(p))),
             _ => None,
         }
     }
diff --git a/openpgp/src/cert/parser/low_level/mod.rs b/openpgp/src/cert/parser/low_level/mod.rs
index eef79987..79f620d3 100644
--- a/openpgp/src/cert/parser/low_level/mod.rs
+++ b/openpgp/src/cert/parser/low_level/mod.rs
@@ -13,6 +13,8 @@ lalrpop_util::lalrpop_mod!(
     "/cert/parser/low_level/grammar.rs"
 );
 
+pub(crate) mod grammar_util;
+
 pub(crate) use self::lexer::Token;
 pub(crate) use self::lexer::Lexer;