Add support for remote sudoers (#1539)

Author: bjorn3

docs/man/sudoers.5.man

@@ -599,7 +599,9 @@ The only supported use is \[cq]*\[cq] as the final argument to indicate
 .SS Including other files from within sudoers
 It is possible to include other sudoers files from within the sudoers
 file currently being parsed using the \f[I]\[at]include\f[R] and
-\f[I]\[at]includedir\f[R] directives.
+\f[I]\[at]includedir\f[R] directives; or the contents provided by an
+application over a unix domain socket using the \f[I]\[at]socket\f[R]
+directive.
 For compatibility with Todd Miller\[cq]s sudo versions prior to 1.9.1,
 \f[I]#include\f[R] and \f[I]#includedir\f[R] are also accepted.
 .PP
@@ -665,6 +667,24 @@ syntax error.
 It is still possible to run visudo with the \-f flag to edit the files
 directly, but this will not catch the redefinition of an alias that is
 also present in a different file.
+.PP
+When managing enterprise-wide sudoers rules, it is sometimes preferable
+to store them in a centralized repository. The \[at]socket directive can be
+used to include the contents provided by a server application over a unix
+domain socket. For example, providing:
+.IP
+.EX
+     \[at]socket /var/run/providers/sudoers.socket
+.EE
+.PP
+will make sudo open that socket, read the rules and close it, as if it
+was an included file. The rules must follow the same syntax used in the
+sudoers files. There is, however, one exception: when reading from a
+socket, the \[at]include, \[at]includedir and \[at]socket directives
+are not accepted.
+.PP
+Please note that the contents read from a socket are immutable from
+sudo's point of view and visudo will not be able to edit them.
 .SS Other special characters and reserved words
 The pound sign (`#') is used to indicate a comment (unless it is part of
 a #include directive or unless it occurs in the context of a user name

docs/man/sudoers.5.md

@@ -323,7 +323,7 @@ Wildcards in command line arguments are not supported---using these in original
 
 ## Including other files from within sudoers
 
-It is possible to include other sudoers files from within the sudoers file currently being parsed using the *@include* and *@includedir* directives.  For compatibility with Todd Miller's sudo versions prior to 1.9.1, *#include* and *#includedir* are also accepted.
+It is possible to include other sudoers files from within the sudoers file currently being parsed using the *@include* and *@includedir* directives; or the contents provided by an application over a unix domain socket using the *@socket* directive.  For compatibility with Todd Miller's sudo versions prior to 1.9.1, *#include* and *#includedir* are also accepted.
 
 An include file can be used, for example, to keep a site-wide sudoers file in addition to a local, per-machine file.  For the sake of this example the site-wide sudoers file will be /etc/sudoers and the per-machine one will be /etc/sudoers.local.  To include /etc/sudoers.local from within /etc/sudoers one would use the following line in /etc/sudoers:
 
@@ -343,6 +343,14 @@ sudo will suspend processing of the current file and read each file in /etc/sudo
 
 Note that unlike files included via @include, visudo will not edit the files in a @includedir directory unless one of them contains a syntax error.  It is still possible to run visudo with the -f flag to edit the files directly, but this will not catch the redefinition of an alias that is also present in a different file.
 
+When managing enterprise-wide sudoers rules, it is sometimes preferable to store them in a centralized repository. The @socket directive can be used to include the contents provided by a server application over a unix domain socket. For example, providing:
+
+         @socket /var/run/providers/sudoers.socket
+
+will make sudo open that socket, read the rules and close it, as if it was an included file. The rules must follow the same syntax used in the sudoers files. There is, however, one exception: when reading from a socket, the @include, @includedir and @socket directives are not accepted.
+
+Please note that the contents read from a socket are immutable from sudo's point of view and visudo will not be able to edit them.
+
 ## Other special characters and reserved words
 
 The pound sign (‘#’) is used to indicate a comment (unless it is part of a #include directive or unless it occurs in the context of a user name and is followed by one or more digits, in which case it is treated as a user-ID).  Both the comment character and any text after it, up to the end of the line, are ignored.

src/common/mod.rs

@@ -25,6 +25,7 @@ pub const HARDENED_ENUM_VALUE_1: u32 = 0xad5d6da; // 101011010101110101101101101
 pub const HARDENED_ENUM_VALUE_2: u32 = 0x69d61fc8; // 1101001110101100001111111001000
 pub const HARDENED_ENUM_VALUE_3: u32 = 0x1629e037; // 0010110001010011110000000110111
 pub const HARDENED_ENUM_VALUE_4: u32 = 0x1fc8d3ac; // 11111110010001101001110101100
+pub const HARDENED_ENUM_VALUE_5: u32 = 0x70372c52; // 1110000001101110010110001010011
 
 // FIXME replace with OsStr::display() once our MSRV is 1.87
 pub(crate) struct DisplayOsStr<'a>(pub(crate) &'a OsStr);

src/sudoers/ast.rs

@@ -4,7 +4,7 @@ use super::tokens::*;
 use crate::common::SudoString;
 use crate::common::{
     HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2, HARDENED_ENUM_VALUE_3,
-    HARDENED_ENUM_VALUE_4,
+    HARDENED_ENUM_VALUE_4, HARDENED_ENUM_VALUE_5,
 };
 use crate::defaults;
 
@@ -158,7 +158,8 @@ pub enum Sudo {
     Decl(Directive) = HARDENED_ENUM_VALUE_1,
     Include(String, Span) = HARDENED_ENUM_VALUE_2,
     IncludeDir(String, Span) = HARDENED_ENUM_VALUE_3,
-    LineComment = HARDENED_ENUM_VALUE_4,
+    Remote(String, Span) = HARDENED_ENUM_VALUE_4,
+    LineComment = HARDENED_ENUM_VALUE_5,
 }
 
 /// grammar:
@@ -592,6 +593,10 @@ fn parse_include(stream: &mut CharStream) -> Parsed<Sudo> {
             let (path, span) = get_path(stream, key_pos)?;
             Sudo::IncludeDir(path, span)
         }
+        Some(Username(key)) if key == "socket" => {
+            let (path, span) = get_path(stream, key_pos)?;
+            Sudo::Remote(path, span)
+        }
         _ => unrecoverable!(pos = key_pos, stream, "unknown directive"),
     };
 

src/sudoers/mod.rs

@@ -11,6 +11,7 @@ mod tokens;
 
 use std::collections::{HashMap, HashSet};
 use std::ffi::OsString;
+use std::fmt;
 use std::io;
 use std::path::{Path, PathBuf};
 
@@ -412,6 +413,11 @@ fn open_subsudoers(path: &Path) -> io::Result<Vec<basic_parser::Parsed<Sudo>>> {
     read_sudoers(source)
 }
 
+fn open_remote_sudoers(path: &Path) -> io::Result<Vec<basic_parser::Parsed<Sudo>>> {
+    let source = audit::secure_open_remote_sudoers(path)?;
+    read_sudoers(source)
+}
+
 // note: trying to DRY using GAT's is tempting but doesn't make the code any shorter
 
 #[derive(Default)]
@@ -686,6 +692,52 @@ fn analyze(
 
     let mut result: Sudoers = Default::default();
 
+    /// The IncludeState allows to tell whether including new files is allowed.
+    /// There are two situations:
+    /// - inclusion is forbidden (as when the @socket directive is used)
+    /// - inclusion is allowed but we need to ensure we didn't go beyond the limit
+    #[derive(Clone, Copy)]
+    enum IncludeState {
+        Forbidden,
+        Allowed(u8),
+    }
+
+    impl IncludeState {
+        #[inline]
+        fn inc(&mut self) -> &mut Self {
+            if let IncludeState::Allowed(x) = self {
+                *x += 1;
+            }
+            self
+        }
+
+        #[inline]
+        fn limit_reached(&self) -> bool {
+            match self {
+                IncludeState::Forbidden => true,
+                IncludeState::Allowed(x) => *x >= INCLUDE_LIMIT,
+            }
+        }
+    }
+
+    /// The directive that produced the inclusion.
+    enum IncludeDirective {
+        Include,
+        IncludeDir,
+        Remote,
+    }
+
+    impl fmt::Display for IncludeDirective {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            let s = match self {
+                IncludeDirective::Include => "@include",
+                IncludeDirective::IncludeDir => "@includedir",
+                IncludeDirective::Remote => "@socket",
+            };
+            write!(f, "{s}")
+        }
+    }
+
     fn resolve_relative(base: &Path, path: impl AsRef<Path>) -> PathBuf {
         if path.as_ref().is_relative() {
             // there should always be a parent since we start with /etc/sudoers, and make every other path
@@ -704,26 +756,40 @@ fn analyze(
         span: Span,
         path: &Path,
         diagnostics: &mut Vec<Error>,
-        count: &mut u8,
+        include_state: &mut IncludeState,
+        include_source: IncludeDirective,
     ) {
-        if *count >= INCLUDE_LIMIT {
+        // IncludeState::limit_reached() will also detect forbidden includes
+        if include_state.limit_reached() {
+            let message = match include_state {
+                IncludeState::Allowed(_) => {
+                    format!("include file limit reached opening '{}'", path.display())
+                }
+                IncludeState::Forbidden => {
+                    format!("{} forbidden at this stage", include_source)
+                }
+            };
             diagnostics.push(Error {
                 source: Some(parent.to_owned()),
                 location: Some(span),
-                message: format!("include file limit reached opening '{}'", path.display()),
-            })
-        // FIXME: this will cause an error in `visudo` if we open a non-privileged sudoers file
-        // that includes another non-privileged sudoer files.
+                message,
+            });
         } else {
-            match open_subsudoers(path) {
-                Ok(subsudoer) => {
-                    *count += 1;
-                    process(cfg, path, subsudoer, diagnostics, count)
-                }
+            let (res, next_state, kind) = match include_source {
+                IncludeDirective::Remote => (
+                    open_remote_sudoers(path),
+                    &mut IncludeState::Forbidden,
+                    "socket",
+                ),
+                _ => (open_subsudoers(path), include_state.inc(), "file"),
+            };
+
+            match res {
+                Ok(subsudoer) => process(cfg, path, subsudoer, diagnostics, next_state),
                 Err(e) => {
                     let message = if e.kind() == io::ErrorKind::NotFound {
                         // improve the error message in this case
-                        format!("cannot open sudoers file '{}'", path.display())
+                        format!("cannot open sudoers {} '{}'", kind, path.display())
                     } else {
                         e.to_string()
                     };
@@ -743,7 +809,7 @@ fn analyze(
         cur_path: &Path,
         sudoers: impl IntoIterator<Item = basic_parser::Parsed<Sudo>>,
         diagnostics: &mut Vec<Error>,
-        safety_count: &mut u8,
+        include_state: &mut IncludeState,
     ) {
         for item in sudoers {
             match item {
@@ -788,9 +854,33 @@ fn analyze(
                         span,
                         &resolve_relative(cur_path, path),
                         diagnostics,
-                        safety_count,
+                        include_state,
+                        IncludeDirective::Include,
                     ),
 
+                    Sudo::Remote(path, span) => {
+                        let socket_path = Path::new(&path);
+                        if socket_path.is_relative() {
+                            diagnostics.push(Error {
+                                source: Some(cur_path.to_owned()),
+                                location: Some(span),
+                                message: format!(
+                                    "cannot open socket {path}: path must be absolute"
+                                ),
+                            });
+                        }
+
+                        include(
+                            cfg,
+                            cur_path,
+                            span,
+                            socket_path,
+                            diagnostics,
+                            include_state,
+                            IncludeDirective::Remote,
+                        );
+                    }
+
                     Sudo::IncludeDir(path, span) => {
                         if path.contains("%h") {
                             diagnostics.push(Error {
@@ -832,7 +922,8 @@ fn analyze(
                                 span,
                                 file.as_ref(),
                                 diagnostics,
-                                safety_count,
+                                include_state,
+                                IncludeDirective::IncludeDir,
                             )
                         }
                     }
@@ -863,7 +954,13 @@ fn analyze(
     }
 
     let mut diagnostics = vec![];
-    process(&mut result, path, sudoers, &mut diagnostics, &mut 0);
+    process(
+        &mut result,
+        path,
+        sudoers,
+        &mut diagnostics,
+        &mut IncludeState::Allowed(0),
+    );
 
     let alias = &mut result.aliases;
     alias.user.0 = sanitize_alias_table(&alias.user.1, &mut diagnostics);

src/system/audit.rs

@@ -1,11 +1,13 @@
 use std::collections::HashSet;
 use std::ffi::{CStr, CString, c_int, c_uint};
-use std::fs::{DirBuilder, File, Metadata, OpenOptions};
-use std::io::{self, Error, ErrorKind};
+use std::fs::{self, DirBuilder, File, Metadata, OpenOptions};
+use std::io::{self, BufReader, Error, ErrorKind};
+use std::net::Shutdown;
 use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd};
 use std::os::unix::{
     ffi::OsStrExt,
     fs::{DirBuilderExt, MetadataExt, PermissionsExt},
+    net::UnixStream,
     prelude::OpenOptionsExt,
 };
 use std::path::{Component, Path};
@@ -137,6 +139,10 @@ pub fn secure_open_sudoers(path: impl AsRef<Path>, check_parent_dir: bool) -> io
     secure_open_impl(path.as_ref(), &mut open_options, check_parent_dir, false)
 }
 
+pub fn secure_open_remote_sudoers(path: impl AsRef<Path>) -> io::Result<BufReader<UnixStream>> {
+    secure_open_socket_impl(path.as_ref())
+}
+
 /// Open a timestamp cookie file using various security checks
 pub fn secure_open_cookie_file(path: impl AsRef<Path>) -> io::Result<File> {
     let mut open_options = OpenOptions::new();
@@ -234,6 +240,19 @@ fn secure_open_impl(
     Ok(file)
 }
 
+// Open the socket at path, provided that it is "secure".
+// "Secure" means that it passes the `checks` function above.
+fn secure_open_socket_impl(path: &Path) -> io::Result<BufReader<UnixStream>> {
+    let meta = fs::metadata(path)?;
+    checks(path, meta)?;
+
+    let stream = UnixStream::connect(path)?;
+    stream.shutdown(Shutdown::Write)?;
+    let reader = BufReader::new(stream);
+
+    Ok(reader)
+}
+
 fn open_at(parent: BorrowedFd, file_name: &CStr, create: bool) -> io::Result<OwnedFd> {
     let flags = if create {
         libc::O_NOFOLLOW | libc::O_RDWR | libc::O_CREAT

test-framework/e2e-tests/src/lib.rs

@@ -3,6 +3,7 @@
 mod pty;
 mod regression;
 mod su;
+mod sudoers;
 
 const USERNAME: &str = "ferris";