Merge pull request bpfman#777 from Billy99/billy99-unify-users

bpfd: Unify the "run as root" and "run as bpfd user" codepaths

bpfctl/src/main.rs

@@ -84,8 +84,8 @@ struct ListArgs {
 
 #[derive(Args)]
 struct LoadFileArgs {
-    /// Required: Location of local bytecode file
-    /// Example: --path /run/bpfd/examples/go-xdp-counter/bpf_bpfel.o
+    /// Required: Location of local bytecode file as fully qualified file path.
+    /// Example: --path $HOME/src/bpfd/examples/go-xdp-counter/bpf_bpfel.o
     #[clap(short, long, verbatim_doc_comment)]
     path: String,
 

bpfd-api/src/config.rs

@@ -178,7 +178,7 @@ fn default_port() -> u16 {
 }
 
 fn default_unix() -> String {
-    STPATH_BPFD_SOCKET.to_string()
+    RTPATH_BPFD_SOCKET.to_string()
 }
 
 fn default_enabled() -> bool {

bpfd-api/src/util.rs

@@ -1,14 +1,13 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Authors of bpfd
 
-pub const USRGRP_BPFD: &str = "bpfd";
-
 pub mod directories {
     // The following directories are used by bpfd. They should be created by bpfd service
     // via the bpfd.service settings. They will be manually created in the case where bpfd
     // is not being run as a service.
     //
     // ConfigurationDirectory: /etc/bpfd/
+    pub const CFGDIR_MODE: u32 = 0o6750;
     pub const CFGDIR: &str = "/etc/bpfd";
     pub const CFGDIR_BPFD_CERTS: &str = "/etc/bpfd/certs/bpfd";
     pub const CFGDIR_BPFD_CLIENT_CERTS: &str = "/etc/bpfd/certs/bpfd-client";
@@ -21,7 +20,9 @@ pub mod directories {
     pub const CFGPATH_BPFD_CERTS_KEY: &str = "/etc/bpfd/certs/bpfd/bpfd.key";
     pub const CFGPATH_BPFD_CLIENT_CERTS_PEM: &str = "/etc/bpfd/certs/bpfd-client/bpfd-client.pem";
     pub const CFGPATH_BPFD_CLIENT_CERTS_KEY: &str = "/etc/bpfd/certs/bpfd-client/bpfd-client.key";
+
     // RuntimeDirectory: /run/bpfd/
+    pub const RTDIR_MODE: u32 = 0o6770;
     pub const RTDIR: &str = "/run/bpfd";
     pub const RTDIR_XDP_DISPATCHER: &str = "/run/bpfd/dispatchers/xdp";
     pub const RTDIR_TC_INGRESS_DISPATCHER: &str = "/run/bpfd/dispatchers/tc-ingress";
@@ -32,14 +33,15 @@ pub mod directories {
     pub const RTDIR_FS_XDP: &str = "/run/bpfd/fs/xdp";
     pub const RTDIR_FS_MAPS: &str = "/run/bpfd/fs/maps";
     pub const RTDIR_PROGRAMS: &str = "/run/bpfd/programs";
-    pub const STPATH_BPFD_SOCKET: &str = "/run/bpfd/bpfd.sock";
+    pub const RTPATH_BPFD_SOCKET: &str = "/run/bpfd/bpfd.sock";
     // The CSI socket must be in it's own sub directory so we can easily create a dedicated
     // K8s volume mount for it.
     pub const RTDIR_BPFD_CSI: &str = "/run/bpfd/csi";
-    pub const STPATH_BPFD_CSI_SOCKET: &str = "/run/bpfd/csi/csi.sock";
+    pub const RTPATH_BPFD_CSI_SOCKET: &str = "/run/bpfd/csi/csi.sock";
     pub const RTDIR_BPFD_CSI_FS: &str = "/run/bpfd/csi/fs";
 
     // StateDirectory: /var/lib/bpfd/
+    pub const STDIR_MODE: u32 = 0o6770;
     pub const STDIR: &str = "/var/lib/bpfd";
-    pub const BYTECODE_IMAGE_CONTENT_STORE: &str = "/var/lib/bpfd/io.bpfd.image.content";
+    pub const STDIR_BYTECODE_IMAGE_CONTENT_STORE: &str = "/var/lib/bpfd/io.bpfd.image.content";
 }

bpfd-operator/pkg/helpers/helpers.go

@@ -166,7 +166,7 @@ func GetMaps(c *bpfdclientset.Clientset, ProgramName string, mapNames []string)
 	}
 
 	if len(bpfProgramList.Items) != 1 {
-		return nil, fmt.Errorf("error getting BpfProgram for %s, multiple bpfPrograms found", ProgramName)
+		return nil, fmt.Errorf("error getting BpfProgram for %s, multiple bpfPrograms found (%d)", ProgramName, len(bpfProgramList.Items))
 	}
 
 	prog := bpfProgramList.Items[0]

bpfd/src/certs.rs

@@ -133,7 +133,7 @@ async fn generate_ca_cert_pem(ca_cert_path: &str) -> Result<Vec<u8>, CertsError>
     tokio::fs::write(ca_key_path, ca_cert_key.private_key_to_pem_pkcs8().unwrap())
         .await
         .map_err(|_| CertsError::Error("unable to write ca key to file".to_string()))?;
-    // Set the private key such that only members of the "bpfd" group can read
+    // Set the private key such that only owner and group can read
     tokio::fs::set_permissions(ca_key_path, fs::Permissions::from_mode(0o0440))
         .await
         .map_err(|_| CertsError::Error("unable to set ca key file permissions".to_string()))?;

bpfd/src/main.rs

@@ -31,7 +31,7 @@ use nix::{
     unistd::{getuid, User},
 };
 use systemd_journal_logger::{connected_to_journal, JournalLog};
-use utils::create_bpffs;
+use utils::{create_bpffs, set_dir_permissions};
 
 use crate::{serve::serve, utils::read_to_string};
 const BPFD_ENV_LOG_LEVEL: &str = "RUST_LOG";
@@ -92,6 +92,8 @@ fn main() -> anyhow::Result<()> {
             create_dir_all(RTDIR_FS_TC_EGRESS)
                 .context("unable to create tc egress dispatcher dir")?;
             create_dir_all(RTDIR_FS_MAPS).context("unable to create maps directory")?;
+            create_dir_all(RTDIR_BPFD_CSI).context("unable to create CSI directory")?;
+            create_dir_all(RTDIR_BPFD_CSI_FS).context("unable to create socket directory")?;
 
             create_dir_all(CFGDIR_BPFD_CERTS).context("unable to create bpfd certs directory")?;
             create_dir_all(CFGDIR_BPFD_CLIENT_CERTS)
@@ -100,13 +102,13 @@ fn main() -> anyhow::Result<()> {
             create_dir_all(CFGDIR_STATIC_PROGRAMS)
                 .context("unable to create static programs directory")?;
 
-            create_dir_all(RTDIR_BPFD_CSI).context("unable to create socket directory")?;
-
-            create_dir_all(RTDIR_BPFD_CSI_FS).context("unable to create socket directory")?;
-
-            create_dir_all(BYTECODE_IMAGE_CONTENT_STORE)
+            create_dir_all(STDIR_BYTECODE_IMAGE_CONTENT_STORE)
                 .context("unable to create bytecode image store directory")?;
 
+            set_dir_permissions(CFGDIR, CFGDIR_MODE).await;
+            set_dir_permissions(RTDIR, RTDIR_MODE).await;
+            set_dir_permissions(STDIR, STDIR_MODE).await;
+
             let config = if let Ok(c) = read_to_string(CFGPATH_BPFD_CONFIG).await {
                 c.parse().unwrap_or_else(|_| {
                     warn!("Unable to parse config file, using defaults");

bpfd/src/serve.rs

@@ -1,15 +1,15 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Authors of bpfd
 
-use std::{fs::remove_file, net::SocketAddr, os::unix::fs::PermissionsExt, path::Path};
+use std::{fs::remove_file, net::SocketAddr, path::Path};
 
 use anyhow::Context;
 use bpfd_api::{
     config::{self, Config},
-    util::directories::BYTECODE_IMAGE_CONTENT_STORE,
+    util::directories::STDIR_BYTECODE_IMAGE_CONTENT_STORE,
     v1::bpfd_server::BpfdServer,
 };
-use log::{debug, info, warn};
+use log::{debug, info};
 use tokio::{
     join,
     net::UnixListener,
@@ -23,8 +23,13 @@ use tonic::transport::{Server, ServerTlsConfig};
 
 pub use crate::certs::get_tls_config;
 use crate::{
-    bpf::BpfManager, errors::BpfdError, oci_utils::ImageManager, rpc::BpfdLoader,
-    static_program::get_static_programs, storage::StorageManager, utils::SOCK_MODE,
+    bpf::BpfManager,
+    errors::BpfdError,
+    oci_utils::ImageManager,
+    rpc::BpfdLoader,
+    static_program::get_static_programs,
+    storage::StorageManager,
+    utils::{set_file_permissions, SOCK_MODE},
 };
 
 pub async fn serve(
@@ -82,7 +87,7 @@ pub async fn serve(
     let (itx, irx) = mpsc::channel(32);
 
     let mut image_manager =
-        ImageManager::new(BYTECODE_IMAGE_CONTENT_STORE, allow_unsigned, irx).await?;
+        ImageManager::new(STDIR_BYTECODE_IMAGE_CONTENT_STORE, allow_unsigned, irx).await?;
     let image_manager_handle = tokio::spawn(async move {
         image_manager.run().await;
     });
@@ -163,14 +168,7 @@ async fn serve_unix(
     let uds = UnixListener::bind(&path)?;
     let uds_stream = UnixListenerStream::new(uds);
     // Always set the file permissions of our listening socket.
-    if (tokio::fs::set_permissions(path.clone(), std::fs::Permissions::from_mode(SOCK_MODE)).await)
-        .is_err()
-    {
-        warn!(
-            "Unable to set permissions on bpfd socket {}. Continuing",
-            path
-        );
-    }
+    set_file_permissions(&path.clone(), SOCK_MODE).await;
 
     let serve = Server::builder()
         .add_service(service)

bpfd/src/storage.rs

@@ -10,7 +10,7 @@ use std::{
 use anyhow::{Context, Result};
 use async_trait::async_trait;
 use aya::maps::MapData;
-use bpfd_api::util::directories::{RTDIR_BPFD_CSI_FS, STPATH_BPFD_CSI_SOCKET};
+use bpfd_api::util::directories::{RTDIR_BPFD_CSI_FS, RTPATH_BPFD_CSI_SOCKET};
 use bpfd_csi::v1::{
     identity_server::{Identity, IdentityServer},
     node_server::{Node, NodeServer},
@@ -362,17 +362,17 @@ impl StorageManager {
     }
 
     pub async fn run(self) {
-        let path: &Path = Path::new(STPATH_BPFD_CSI_SOCKET);
+        let path: &Path = Path::new(RTPATH_BPFD_CSI_SOCKET);
         // Listen on Unix socket
         if path.exists() {
             // Attempt to remove the socket, since bind fails if it exists
             remove_file(path).expect("Panicked cleaning up stale csi socket");
         }
 
         let uds = UnixListener::bind(path)
-            .unwrap_or_else(|_| panic!("failed to bind {STPATH_BPFD_CSI_SOCKET}"));
+            .unwrap_or_else(|_| panic!("failed to bind {RTPATH_BPFD_CSI_SOCKET}"));
         let uds_stream = UnixListenerStream::new(uds);
-        set_file_permissions(STPATH_BPFD_CSI_SOCKET, SOCK_MODE).await;
+        set_file_permissions(RTPATH_BPFD_CSI_SOCKET, SOCK_MODE).await;
 
         let node_service = NodeServer::new(self.csi_node);
         let identity_service = IdentityServer::new(self.csi_identity);

bpfd/src/utils.rs

@@ -4,14 +4,12 @@
 use std::{os::unix::fs::PermissionsExt, path::Path, str};
 
 use anyhow::{Context, Result};
-use bpfd_api::util::USRGRP_BPFD;
 use log::{debug, info, warn};
 use nix::{
     mount::{mount, MsFlags},
     net::if_::if_nametoindex,
 };
 use tokio::{fs, io::AsyncReadExt};
-use users::get_group_by_name;
 
 use crate::errors::BpfdError;
 
@@ -63,25 +61,18 @@ pub(crate) fn get_ifindex(iface: &str) -> Result<u32, BpfdError> {
 }
 
 pub(crate) async fn set_file_permissions(path: &str, mode: u32) {
-    // Determine if User Group exists, if not, do nothing
-    if get_group_by_name(USRGRP_BPFD).is_some() {
-        // Set the permissions on the file based on input
-        if (tokio::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)).await).is_err()
-        {
-            warn!("Unable to set permissions on file {}. Continuing", path);
-        }
+    // Set the permissions on the file based on input
+    if (tokio::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)).await).is_err() {
+        warn!("Unable to set permissions on file {}. Continuing", path);
     }
 }
 
 pub(crate) async fn set_dir_permissions(directory: &str, mode: u32) {
-    // Determine if User Group exists, if not, do nothing
-    if get_group_by_name(USRGRP_BPFD).is_some() {
-        // Iterate through the files in the provided directory
-        let mut entries = fs::read_dir(directory).await.unwrap();
-        while let Some(file) = entries.next_entry().await.unwrap() {
-            // Set the permissions on the file based on input
-            set_file_permissions(&file.path().into_os_string().into_string().unwrap(), mode).await;
-        }
+    // Iterate through the files in the provided directory
+    let mut entries = fs::read_dir(directory).await.unwrap();
+    while let Some(file) = entries.next_entry().await.unwrap() {
+        // Set the permissions on the file based on input
+        set_file_permissions(&file.path().into_os_string().into_string().unwrap(), mode).await;
     }
 }
 

docs/developer-guide/configuration.md

@@ -48,9 +48,6 @@ Valid fields:
 - **client_cert**: Client certificate file location, intended to be used by bpfd clients (`bpfctl`, `bpfd-agent`, etc).
 - **client_key**: Client certificate key file location, intended to be used by bpfd clients (`bpfctl`, `bpfd-agent`, etc).
 
-If bpfd is running as a systemd service, then the certificates must be accessible by bpfd
-(owned by the bpfd User and User Group).
-
 ### Config Section: [interfaces]
 
 This section of the configuration file allows the XDP Mode for a given interface to be set.
@@ -76,8 +73,7 @@ Valid fields:
 In this section different endpoints can be configured for bpfd to listen on. We currently support TCP sockets
 with IPv4 and Ipv6 addresses and Unix domain sockets.
 When using TCP sockets, the tls configuration will be used to secure communication.
-Unix domain sockets provide a simpler communication with no encryption. These sockets are owned by the bpfd
-user and user group when running as a systemd or non-root process.
+Unix domain sockets provide a simpler communication with no encryption.
 
 Valid fields:
 

docs/developer-guide/linux-capabilities.md

@@ -11,6 +11,25 @@ required set of capabilities via the `bpfd.service` file using the `AmbientCapab
 All spawned threads are stripped of all capabilities, removing all sudo privileges
 (see `drop_linux_capabilities()` usage), leaving only the main thread with only the needed set of capabilities.
 
+## Current bpfd Linux Capabilities
+
+Below are the current set of Linux capabilities required by bpfd to operate:
+
+* **CAP_BPF:**
+    * Required to load BPF programs and create BPF maps.
+* **CAP_DAC_READ_SEARCH:**
+    * Required by Tracepoint programs, needed by aya to check the tracefs mount point.
+      For example, trying to read "/sys/kernel/tracing" and "/sys/kernel/debug/tracing".
+* **CAP_NET_ADMIN:**
+    * Required for TC programs to attach/detach to/from a qdisc.
+* **CAP_SETPCAP:**
+    * Required to allow bpfd to drop Linux Capabilities on spawned threads.
+* **CAP_SYS_ADMIN:** 
+    * Kprobe (Kprobe and Uprobe) and Tracepoint programs are considered perfmon programs and require CAP_PERFMON and CAP_SYS_ADMIN to load.
+    * TC and XDP programs are considered admin programs and require CAP_NET_ADMIN and CAP_SYS_ADMIN to load.
+* **CAP_SYS_RESOURCE:**
+    * Required by bpfd to call `setrlimit()` on `RLIMIT_MEMLOCK`.
+
 ## Debugging Linux Capabilities
 
 As new features are added, the set of Linux capabilities required by bpfd may change over time.

docs/developer-guide/logging.md

@@ -55,11 +55,6 @@ After=network.target
 [Service]
 Environment="RUST_LOG=Info"    <==== Set Log Level Here
 ExecStart=/usr/sbin/bpfd
-MemoryAccounting=true
-MemoryLow=infinity
-MemoryMax=infinity
-User=bpfd
-Group=bpfd
 AmbientCapabilities=CAP_BPF CAP_DAC_READ_SEARCH CAP_NET_ADMIN CAP_PERFMON CAP_SYS_ADMIN CAP_SYS_RESOURCE
 CapabilityBoundingSet=CAP_BPF CAP_DAC_READ_SEARCH CAP_NET_ADMIN CAP_PERFMON CAP_SYS_ADMIN CAP_SYS_RESOURCE
 ```

docs/developer-guide/testing.md

@@ -70,9 +70,8 @@ directory.  These programs are compiled by executing `cargo xtask build-ebpf
 We also load some tests from local files to test the `load-from-file` option.
 
 The `bpf` directory also contains a script called `build_push_images.sh` that
-can be used to build and push new images to quay if the code is changed.  We
-plan to push the images automatically when code gets merged ([issue
-\#533](<https://github.com/bpfd-dev/bpfd/issues/533>)). However, it's still
+can be used to build and push new images to quay if the code is changed.
+Images get pushed automatically when code gets merged, however, it's still
 useful to be able to push them manually sometimes. For example, when a new test
 case requires that both the eBPF and integration code be changed together.  It
 is also a useful template for new eBPF test code that needs to be pushed.