Give better catchable error messages for ignored tags (#1401)

Author: squell

src/sudoers/ast.rs

@@ -118,6 +118,7 @@ pub struct Tag {
     pub(super) env: EnvironmentControl,
     pub(super) apparmor_profile: Option<String>,
     pub(super) noexec: ExecControl,
+    pub(super) ignored: Vec<Span>,
 }
 
 impl Tag {
@@ -388,25 +389,6 @@ impl Parse for MetaOrTag {
         };
 
         let result: Modifier = match keyword.as_str() {
-            // we do not support this, and that should make sudo-rs "fail safe"
-            "INTERCEPT" => unrecoverable!(
-                pos = start_pos,
-                stream,
-                "INTERCEPT is not supported by sudo-rs"
-            ),
-            // this is less fatal
-            "LOG_INPUT" | "NOLOG_INPUT" | "LOG_OUTPUT" | "NOLOG_OUTPUT" | "MAIL" | "NOMAIL"
-            | "FOLLOW" => {
-                eprintln_ignore_io_error!(
-                    "sudo-rs: {} tags in the sudoers policy are ignored",
-                    keyword.as_str()
-                );
-                switch(|_| {})?
-            }
-
-            // 'NOFOLLOW' and 'NOINTERCEPT' are the default behaviour.
-            "NOFOLLOW" | "NOINTERCEPT" => switch(|_| {})?,
-
             "EXEC" => switch(|tag| tag.noexec = ExecControl::Exec)?,
             "NOEXEC" => switch(|tag| tag.noexec = ExecControl::Noexec)?,
 
@@ -420,18 +402,35 @@ impl Parse for MetaOrTag {
                 let path: ChDir = expect_nonterminal(stream)?;
                 Box::new(move |tag| tag.cwd = Some(path.clone()))
             }
+
             // we do not support these, and that should make sudo-rs "fail safe"
-            spec @ ("CHROOT" | "TIMEOUT" | "NOTBEFORE" | "NOTAFTER") => unrecoverable!(
-                pos = start_pos,
-                stream,
-                "{spec} is not supported by sudo-rs"
-            ),
+            spec @ ("INTERCEPT" | "CHROOT" | "TIMEOUT" | "NOTBEFORE" | "NOTAFTER") => {
+                unrecoverable!(
+                    pos = start_pos,
+                    stream,
+                    "{spec} is not supported by sudo-rs"
+                )
+            }
             "ROLE" | "TYPE" => unrecoverable!(
                 pos = start_pos,
                 stream,
                 "SELinux role based access control is not yet supported by sudo-rs"
             ),
 
+            // this is less fatal
+            "LOG_INPUT" | "NOLOG_INPUT" | "LOG_OUTPUT" | "NOLOG_OUTPUT" | "MAIL" | "NOMAIL"
+            | "FOLLOW" => {
+                let ignored_location = Span {
+                    start: start_pos,
+                    end: stream.get_pos(),
+                };
+                expect_syntax(':', stream)?;
+                Box::new(move |tag| tag.ignored.push(ignored_location))
+            }
+
+            // 'NOFOLLOW' and 'NOINTERCEPT' are the default behaviour.
+            "NOFOLLOW" | "NOINTERCEPT" => switch(|_| {})?,
+
             "APPARMOR_PROFILE" => {
                 expect_syntax('=', stream)?;
                 let StringParameter(profile) = expect_nonterminal(stream)?;

src/sudoers/basic_parser.rs

@@ -28,8 +28,8 @@
 /// Type holding a parsed object (or error information if parsing failed)
 pub type Parsed<T> = Result<T, Status>;
 
-#[derive(Copy, Clone)]
-#[cfg_attr(test, derive(Debug, PartialEq))]
+#[derive(Copy, Clone, PartialEq)]
+#[cfg_attr(test, derive(Debug, Eq))]
 pub struct Span {
     pub start: (usize, usize),
     pub end: (usize, usize),

src/sudoers/mod.rs

@@ -741,7 +741,14 @@ fn analyze(
                 Ok(line) => match line {
                     Sudo::LineComment => {}
 
-                    Sudo::Spec(permission) => cfg.rules.push(permission),
+                    Sudo::Spec(permission) => {
+                        diagnostics.extend(get_ignored_tags(&permission).map(|span| Error {
+                            source: Some(cur_path.to_owned()),
+                            location: Some(span),
+                            message: "this tag is ignored by sudo-rs".to_string(),
+                        }));
+                        cfg.rules.push(permission);
+                    }
 
                     Sudo::Decl(HostAlias(mut def)) => cfg.aliases.host.1.append(&mut def),
                     Sudo::Decl(UserAlias(mut def)) => cfg.aliases.user.1.append(&mut def),
@@ -828,6 +835,20 @@ fn analyze(
         }
     }
 
+    fn get_ignored_tags(
+        PermissionSpec { permissions, .. }: &PermissionSpec,
+    ) -> impl Iterator<Item = Span> + '_ {
+        permissions
+            .iter()
+            .flat_map(|(_host, runas_cmds)| runas_cmds)
+            .flat_map(|(_runas, CommandSpec(tags, _cmd))| tags)
+            .flat_map(|modifier| {
+                let mut tag = Tag::default();
+                modifier(&mut tag);
+                tag.ignored
+            })
+    }
+
     let mut diagnostics = vec![];
     process(&mut result, path, sudoers, &mut diagnostics, &mut 0);