Merge "Verify key characteristics of generated/imported keys." into main
diff --git a/OWNERS b/OWNERS
index 03e5769..6fdb550 100644
--- a/OWNERS
+++ b/OWNERS
@@ -1,5 +1,4 @@
 alanstokes@google.com
-cbrubaker@google.com
 drysdale@google.com
 eranm@google.com
 hasinitg@google.com
@@ -8,4 +7,5 @@
 kroot@google.com
 sethmo@google.com
 swillden@google.com
+trong@google.com
 zeuthen@google.com
diff --git a/fsverity_init/Android.bp b/fsverity_init/Android.bp
index 83c5945..07eaf6a 100644
--- a/fsverity_init/Android.bp
+++ b/fsverity_init/Android.bp
@@ -10,26 +10,10 @@
 cc_binary {
     name: "fsverity_init",
     srcs: [
-        "main.cpp",
+        "fsverity_init.cpp",
     ],
     static_libs: [
         "libc++fs",
-        "libfsverity_init",
-        "libmini_keyctl_static",
-    ],
-    shared_libs: [
-        "libbase",
-        "libkeyutils",
-        "liblog",
-    ],
-    cflags: ["-Werror", "-Wall", "-Wextra"],
-}
-
-cc_library {
-    name: "libfsverity_init",
-    srcs: ["fsverity_init.cpp"],
-    static_libs: [
-        "libc++fs",
         "libmini_keyctl_static",
     ],
     shared_libs: [
@@ -38,6 +22,4 @@
         "liblog",
     ],
     cflags: ["-Werror", "-Wall", "-Wextra"],
-    export_include_dirs: ["include"],
-    recovery_available: true,
 }
diff --git a/fsverity_init/fsverity_init.cpp b/fsverity_init/fsverity_init.cpp
index 61f84dd..797118d 100644
--- a/fsverity_init/fsverity_init.cpp
+++ b/fsverity_init/fsverity_init.cpp
@@ -14,6 +14,25 @@
  * limitations under the License.
  */
 
+//
+// fsverity_init is a tool for loading X.509 certificates into the kernel keyring used by the
+// fsverity builtin signature verification kernel feature
+// (https://www.kernel.org/doc/html/latest/filesystems/fsverity.html#built-in-signature-verification).
+// Starting in Android 14, Android has actually stopped using this feature, as it was too inflexible
+// and caused problems.  It has been replaced by userspace signature verification.  Also, some uses
+// of fsverity in Android are now for integrity-only use cases.
+//
+// Regardless, there may exist fsverity files on-disk that were created by Android 13 or earlier.
+// These files still have builtin signatures.  If the kernel is an older kernel that still has
+// CONFIG_FS_VERITY_BUILTIN_SIGNATURES enabled, these files cannot be opened unless the
+// corresponding key is in the ".fs-verity" keyring.  Therefore, this tool still has to exist and be
+// used to load keys into the kernel, even though this has no security purpose anymore.
+//
+// This tool can be removed as soon as all supported kernels are guaranteed to have
+// CONFIG_FS_VERITY_BUILTIN_SIGNATURES disabled, or alternatively as soon as support for upgrades
+// from Android 13 or earlier is no longer required.
+//
+
 #define LOG_TAG "fsverity_init"
 
 #include <sys/types.h>
@@ -23,33 +42,10 @@
 
 #include <android-base/file.h>
 #include <android-base/logging.h>
-#include <android-base/properties.h>
 #include <android-base/strings.h>
 #include <log/log.h>
 #include <mini_keyctl_utils.h>
 
-bool LoadKeyToKeyring(key_serial_t keyring_id, const char* desc, const char* data, size_t size) {
-    key_serial_t key = add_key("asymmetric", desc, data, size, keyring_id);
-    if (key < 0) {
-        PLOG(ERROR) << "Failed to add key";
-        return false;
-    }
-    return true;
-}
-
-bool LoadKeyFromStdin(key_serial_t keyring_id, const char* keyname) {
-    std::string content;
-    if (!android::base::ReadFdToString(STDIN_FILENO, &content)) {
-        LOG(ERROR) << "Failed to read key from stdin";
-        return false;
-    }
-    if (!LoadKeyToKeyring(keyring_id, keyname, content.c_str(), content.size())) {
-        LOG(ERROR) << "Failed to load key from stdin";
-        return false;
-    }
-    return true;
-}
-
 void LoadKeyFromFile(key_serial_t keyring_id, const char* keyname, const std::string& path) {
     LOG(INFO) << "LoadKeyFromFile path=" << path << " keyname=" << keyname;
     std::string content;
@@ -57,8 +53,8 @@
         LOG(ERROR) << "Failed to read key from " << path;
         return;
     }
-    if (!LoadKeyToKeyring(keyring_id, keyname, content.c_str(), content.size())) {
-        LOG(ERROR) << "Failed to load key from " << path;
+    if (add_key("asymmetric", keyname, content.c_str(), content.size(), keyring_id) < 0) {
+        PLOG(ERROR) << "Failed to add key from " << path;
     }
 }
 
@@ -81,3 +77,28 @@
     LoadKeyFromDirectory(keyring_id, "fsv_system_", "/system/etc/security/fsverity");
     LoadKeyFromDirectory(keyring_id, "fsv_product_", "/product/etc/security/fsverity");
 }
+
+int main(int argc, const char** argv) {
+    if (argc < 2) {
+        LOG(ERROR) << "Not enough arguments";
+        return -1;
+    }
+
+    key_serial_t keyring_id = android::GetKeyringId(".fs-verity");
+    if (keyring_id < 0) {
+        // This is expected on newer kernels.  See comment at the beginning of this file.
+        LOG(DEBUG) << "no initialization required";
+        return 0;
+    }
+
+    const std::string_view command = argv[1];
+
+    if (command == "--load-verified-keys") {
+        LoadKeyFromVerifiedPartitions(keyring_id);
+    } else {
+        LOG(ERROR) << "Unknown argument(s).";
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/fsverity_init/include/fsverity_init.h b/fsverity_init/include/fsverity_init.h
deleted file mode 100644
index c3bc93b..0000000
--- a/fsverity_init/include/fsverity_init.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <mini_keyctl_utils.h>
-
-bool LoadKeyFromStdin(key_serial_t keyring_id, const char* keyname);
-void LoadKeyFromFile(key_serial_t keyring_id, const char* keyname, const std::string& path);
-void LoadKeyFromVerifiedPartitions(key_serial_t keyring_id);
diff --git a/fsverity_init/main.cpp b/fsverity_init/main.cpp
deleted file mode 100644
index b502b91..0000000
--- a/fsverity_init/main.cpp
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <string>
-
-#include <android-base/file.h>
-#include <android-base/logging.h>
-#include <android-base/properties.h>
-#include <fsverity_init.h>
-#include <log/log.h>
-#include <mini_keyctl_utils.h>
-
-int main(int argc, const char** argv) {
-    if (argc < 2) {
-        LOG(ERROR) << "Not enough arguments";
-        return -1;
-    }
-
-    key_serial_t keyring_id = android::GetKeyringId(".fs-verity");
-    if (keyring_id < 0) {
-        LOG(ERROR) << "Failed to find .fs-verity keyring id";
-        return -1;
-    }
-
-    const std::string_view command = argv[1];
-
-    if (command == "--load-verified-keys") {
-        LoadKeyFromVerifiedPartitions(keyring_id);
-    } else if (command == "--load-extra-key") {
-        if (argc != 3) {
-            LOG(ERROR) << "--load-extra-key requires <key_name> argument.";
-            return -1;
-        }
-        if (!LoadKeyFromStdin(keyring_id, argv[2])) {
-            return -1;
-        }
-    } else if (command == "--lock") {
-        if (!android::base::GetBoolProperty("ro.debuggable", false)) {
-            if (keyctl_restrict_keyring(keyring_id, nullptr, nullptr) < 0) {
-                PLOG(ERROR) << "Cannot restrict .fs-verity keyring";
-            }
-        }
-    } else {
-        LOG(ERROR) << "Unknown argument(s).";
-        return -1;
-    }
-
-    return 0;
-}
diff --git a/keystore/keystore_attestation_id.cpp b/keystore/keystore_attestation_id.cpp
index ccd3808..75c62dd 100644
--- a/keystore/keystore_attestation_id.cpp
+++ b/keystore/keystore_attestation_id.cpp
@@ -74,8 +74,8 @@
 }
 
 KeyAttestationApplicationIdProvider::KeyAttestationApplicationIdProvider()
-    : BpKeyAttestationApplicationIdProvider(
-          android::defaultServiceManager()->getService(String16("sec_key_att_app_id_provider"))) {}
+    : BpKeyAttestationApplicationIdProvider(android::defaultServiceManager()->waitForService(
+          String16("sec_key_att_app_id_provider"))) {}
 
 DECLARE_STACK_OF(ASN1_OCTET_STRING);
 
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 8d5e985..3bf582f 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -51,9 +51,9 @@
     /// An outstanding per operation authorization request.
     OpAuth,
     /// An outstanding request for per operation authorization and secure timestamp.
-    TimeStampedOpAuth(Receiver<Result<TimeStampToken, Error>>),
+    TimeStampedOpAuth(Mutex<Receiver<Result<TimeStampToken, Error>>>),
     /// An outstanding request for a timestamp token.
-    TimeStamp(Receiver<Result<TimeStampToken, Error>>),
+    TimeStamp(Mutex<Receiver<Result<TimeStampToken, Error>>>),
 }
 
 #[derive(Debug)]
@@ -64,8 +64,6 @@
     hat: Mutex<Option<HardwareAuthToken>>,
 }
 
-unsafe impl Sync for AuthRequest {}
-
 impl AuthRequest {
     fn op_auth() -> Arc<Self> {
         Arc::new(Self { state: AuthRequestState::OpAuth, hat: Mutex::new(None) })
@@ -73,7 +71,7 @@
 
     fn timestamped_op_auth(receiver: Receiver<Result<TimeStampToken, Error>>) -> Arc<Self> {
         Arc::new(Self {
-            state: AuthRequestState::TimeStampedOpAuth(receiver),
+            state: AuthRequestState::TimeStampedOpAuth(Mutex::new(receiver)),
             hat: Mutex::new(None),
         })
     }
@@ -82,7 +80,10 @@
         hat: HardwareAuthToken,
         receiver: Receiver<Result<TimeStampToken, Error>>,
     ) -> Arc<Self> {
-        Arc::new(Self { state: AuthRequestState::TimeStamp(receiver), hat: Mutex::new(Some(hat)) })
+        Arc::new(Self {
+            state: AuthRequestState::TimeStamp(Mutex::new(receiver)),
+            hat: Mutex::new(Some(hat)),
+        })
     }
 
     fn add_auth_token(&self, hat: HardwareAuthToken) {
@@ -100,7 +101,11 @@
 
         let tst = match &self.state {
             AuthRequestState::TimeStampedOpAuth(recv) | AuthRequestState::TimeStamp(recv) => {
-                let result = recv.recv().context("In get_auth_tokens: Sender disconnected.")?;
+                let result = recv
+                    .lock()
+                    .unwrap()
+                    .recv()
+                    .context("In get_auth_tokens: Sender disconnected.")?;
                 Some(result.context(ks_err!(
                     "Worker responded with error \
                     from generating timestamp token.",
diff --git a/keystore2/src/id_rotation.rs b/keystore2/src/id_rotation.rs
index 460caa7..5904047 100644
--- a/keystore2/src/id_rotation.rs
+++ b/keystore2/src/id_rotation.rs
@@ -26,7 +26,7 @@
 use std::fs;
 use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
-use std::time::Duration;
+use std::time::{Duration, SystemTime};
 
 const ID_ROTATION_PERIOD: Duration = Duration::from_secs(30 * 24 * 60 * 60); // Thirty days.
 static TIMESTAMP_FILE_NAME: &str = "timestamp";
@@ -36,6 +36,8 @@
 /// and passed down to the users of the feature which can then query the timestamp on demand.
 #[derive(Debug, Clone)]
 pub struct IdRotationState {
+    /// We consider the time of last factory reset to be the point in time when this timestamp file
+    /// is created.
     timestamp_path: PathBuf,
 }
 
@@ -47,25 +49,41 @@
         Self { timestamp_path }
     }
 
-    /// Reads the metadata of or creates the timestamp file. It returns true if the timestamp
-    /// file is younger than `ID_ROTATION_PERIOD`, i.e., 30 days.
-    pub fn had_factory_reset_since_id_rotation(&self) -> Result<bool> {
+    /// Returns true iff a factory reset has occurred since the last ID rotation.
+    pub fn had_factory_reset_since_id_rotation(
+        &self,
+        creation_datetime: &SystemTime,
+    ) -> Result<bool> {
         match fs::metadata(&self.timestamp_path) {
             Ok(metadata) => {
-                let duration_since_factory_reset = metadata
-                    .modified()
-                    .context("File creation time not supported.")?
-                    .elapsed()
-                    .context("Failed to compute time elapsed since factory reset.")?;
-                Ok(duration_since_factory_reset < ID_ROTATION_PERIOD)
+                // For Tag::UNIQUE_ID, temporal counter value is defined as Tag::CREATION_DATETIME
+                // divided by 2592000000, dropping any remainder. Temporal counter value is
+                // effectively the index of the ID rotation period that we are currently in, with
+                // each ID rotation period being 30 days.
+                let temporal_counter_value = creation_datetime
+                    .duration_since(SystemTime::UNIX_EPOCH)
+                    .context(ks_err!("Failed to get epoch time"))?
+                    .as_millis()
+                    / ID_ROTATION_PERIOD.as_millis();
+
+                // Calculate the beginning of the current ID rotation period, which is also the
+                // last time ID was rotated.
+                let id_rotation_time: SystemTime = SystemTime::UNIX_EPOCH
+                    .checked_add(ID_ROTATION_PERIOD * temporal_counter_value.try_into()?)
+                    .context(ks_err!("Failed to get ID rotation time."))?;
+
+                let factory_reset_time =
+                    metadata.modified().context(ks_err!("File creation time not supported."))?;
+
+                Ok(id_rotation_time <= factory_reset_time)
             }
             Err(e) => match e.kind() {
                 ErrorKind::NotFound => {
                     fs::File::create(&self.timestamp_path)
-                        .context("Failed to create timestamp file.")?;
+                        .context(ks_err!("Failed to create timestamp file."))?;
                     Ok(true)
                 }
-                _ => Err(e).context("Failed to open timestamp file."),
+                _ => Err(e).context(ks_err!("Failed to open timestamp file.")),
             },
         }
         .context(ks_err!())
@@ -78,47 +96,75 @@
     use keystore2_test_utils::TempDir;
     use nix::sys::stat::utimes;
     use nix::sys::time::{TimeVal, TimeValLike};
-    use std::convert::TryInto;
-    use std::time::UNIX_EPOCH;
+    use std::thread::sleep;
 
-    #[test]
-    fn test_had_factory_reset_since_id_rotation() -> Result<()> {
-        let temp_dir = TempDir::new("test_had_factory_reset_since_id_rotation_")
-            .expect("Failed to create temp dir.");
+    static TEMP_DIR_NAME: &str = "test_had_factory_reset_since_id_rotation_";
+
+    fn set_up() -> (TempDir, PathBuf, IdRotationState) {
+        let temp_dir = TempDir::new(TEMP_DIR_NAME).expect("Failed to create temp dir.");
+        let mut timestamp_file_path = temp_dir.path().to_owned();
+        timestamp_file_path.push(TIMESTAMP_FILE_NAME);
         let id_rotation_state = IdRotationState::new(temp_dir.path());
 
-        let mut temp_file_path = temp_dir.path().to_owned();
-        temp_file_path.push(TIMESTAMP_FILE_NAME);
+        (temp_dir, timestamp_file_path, id_rotation_state)
+    }
+
+    #[test]
+    fn test_timestamp_creation() {
+        let (_temp_dir, timestamp_file_path, id_rotation_state) = set_up();
+        let creation_datetime = SystemTime::now();
 
         // The timestamp file should not exist.
-        assert!(!temp_file_path.exists());
+        assert!(!timestamp_file_path.exists());
 
-        // This should return true.
-        assert!(id_rotation_state.had_factory_reset_since_id_rotation()?);
+        // Trigger timestamp file creation one second later.
+        sleep(Duration::new(1, 0));
+        assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
 
         // Now the timestamp file should exist.
-        assert!(temp_file_path.exists());
+        assert!(timestamp_file_path.exists());
 
-        // We should still return true because the timestamp file is young.
-        assert!(id_rotation_state.had_factory_reset_since_id_rotation()?);
+        let metadata = fs::metadata(&timestamp_file_path).unwrap();
+        assert!(metadata.modified().unwrap() > creation_datetime);
+    }
 
-        // Now let's age the timestamp file by backdating the modification time.
-        let metadata = fs::metadata(&temp_file_path)?;
-        let mtime = metadata.modified()?;
-        let mtime = mtime.duration_since(UNIX_EPOCH)?;
-        let mtime =
-            mtime.checked_sub(ID_ROTATION_PERIOD).expect("Failed to subtract id rotation period");
-        let mtime = TimeVal::seconds(mtime.as_secs().try_into().unwrap());
+    #[test]
+    fn test_existing_timestamp() {
+        let (_temp_dir, timestamp_file_path, id_rotation_state) = set_up();
 
-        let atime = metadata.accessed()?;
-        let atime = atime.duration_since(UNIX_EPOCH)?;
-        let atime = TimeVal::seconds(atime.as_secs().try_into().unwrap());
+        // Let's start with at a known point in time, so that it's easier to control which ID
+        // rotation period we're in.
+        let mut creation_datetime = SystemTime::UNIX_EPOCH;
 
-        utimes(&temp_file_path, &atime, &mtime)?;
+        // Create timestamp file and backdate it back to Unix epoch.
+        fs::File::create(&timestamp_file_path).unwrap();
+        let mtime = TimeVal::seconds(0);
+        let atime = TimeVal::seconds(0);
+        utimes(&timestamp_file_path, &atime, &mtime).unwrap();
 
-        // Now that the file has aged we should see false.
-        assert!(!id_rotation_state.had_factory_reset_since_id_rotation()?);
+        // Timestamp file was backdated to the very beginning of the current ID rotation period.
+        // So, this should return true.
+        assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
 
-        Ok(())
+        // Move time forward, but stay in the same ID rotation period.
+        creation_datetime += Duration::from_millis(1);
+
+        // We should still return true because we're in the same ID rotation period.
+        assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
+
+        // Move time to the next ID rotation period.
+        creation_datetime += ID_ROTATION_PERIOD;
+
+        // Now we should see false.
+        assert!(!id_rotation_state
+            .had_factory_reset_since_id_rotation(&creation_datetime)
+            .unwrap());
+
+        // Move timestamp to the future. This shouldn't ever happen, but even in this edge case ID
+        // must be rotated.
+        let mtime = TimeVal::seconds((ID_ROTATION_PERIOD.as_secs() * 10).try_into().unwrap());
+        let atime = TimeVal::seconds((ID_ROTATION_PERIOD.as_secs() * 10).try_into().unwrap());
+        utimes(&timestamp_file_path, &atime, &mtime).unwrap();
+        assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
     }
 }
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 5eed37c..db44d4b 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -405,8 +405,7 @@
     ) -> Result<Vec<KeyParameter>> {
         let mut result = params.to_vec();
 
-        // Unconditionally add the CREATION_DATETIME tag and prevent callers from
-        // specifying it.
+        // Prevent callers from specifying the CREATION_DATETIME tag.
         if params.iter().any(|kp| kp.tag == Tag::CREATION_DATETIME) {
             return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT)).context(ks_err!(
                 "KeystoreSecurityLevel::add_required_parameters: \
@@ -414,12 +413,16 @@
             ));
         }
 
+        // Use this variable to refer to notion of "now". This eliminates discrepancies from
+        // quering the clock multiple times.
+        let creation_datetime = SystemTime::now();
+
         // Add CREATION_DATETIME only if the backend version Keymint V1 (100) or newer.
         if self.hw_info.versionNumber >= 100 {
             result.push(KeyParameter {
                 tag: Tag::CREATION_DATETIME,
                 value: KeyParameterValue::DateTime(
-                    SystemTime::now()
+                    creation_datetime
                         .duration_since(SystemTime::UNIX_EPOCH)
                         .context(ks_err!(
                             "KeystoreSecurityLevel::add_required_parameters: \
@@ -462,7 +465,7 @@
             }
             if self
                 .id_rotation_state
-                .had_factory_reset_since_id_rotation()
+                .had_factory_reset_since_id_rotation(&creation_datetime)
                 .context(ks_err!("Call to had_factory_reset_since_id_rotation failed."))?
             {
                 result.push(KeyParameter {
diff --git a/prng_seeder/src/cutils_socket.rs b/prng_seeder/src/cutils_socket.rs
index ab2c869..b408be6 100644
--- a/prng_seeder/src/cutils_socket.rs
+++ b/prng_seeder/src/cutils_socket.rs
@@ -19,7 +19,11 @@
 
 pub fn android_get_control_socket(name: &str) -> Result<UnixListener> {
     let name = CString::new(name)?;
+    // SAFETY: name is a valid C string, and android_get_control_socket doesn't retain it after it
+    // returns.
     let fd = unsafe { cutils_socket_bindgen::android_get_control_socket(name.as_ptr()) };
     ensure!(fd >= 0, "android_get_control_socket failed");
+    // SAFETY: android_get_control_socket either returns a valid and open FD or -1, and we checked
+    // that it's not -1.
     Ok(unsafe { UnixListener::from_raw_fd(fd) })
 }
diff --git a/prng_seeder/src/drbg.rs b/prng_seeder/src/drbg.rs
index 89c5a88..808ea18 100644
--- a/prng_seeder/src/drbg.rs
+++ b/prng_seeder/src/drbg.rs
@@ -23,6 +23,9 @@
 
 impl Drbg {
     pub fn new(entropy: &Entropy) -> Result<Drbg> {
+        // SAFETY: entropy must be a valid pointer because it comes from a reference, and a null
+        // pointer is allowed for personalization. CTR_DRBG_new doesn't retain the entropy pointer
+        // for use after it returns.
         let p = unsafe { bssl_sys::CTR_DRBG_new(entropy.as_ptr(), std::ptr::null(), 0) };
         ensure!(!p.is_null(), "CTR_DRBG_new failed");
         Ok(Drbg(p))
@@ -30,6 +33,9 @@
 
     pub fn reseed(&mut self, entropy: &Entropy) -> Result<()> {
         ensure!(
+            // SAFETY: We know that self.0 is valid because it was initialised from CTR_DRBG_new in
+            // Drbg::new above. The entropy pointer must be valid because it comes from a reference,
+            // and CTR_DRBG_reseed doesn't retain it after it returns.
             unsafe { bssl_sys::CTR_DRBG_reseed(self.0, entropy.as_ptr(), std::ptr::null(), 0) }
                 == 1,
             "CTR_DRBG_reseed failed"
@@ -39,6 +45,10 @@
 
     pub fn generate(&mut self, buf: &mut [u8]) -> Result<()> {
         ensure!(
+            // SAFETY: We know that self.0 is valid because it was initialised from CTR_DRBG_new in
+            // Drbg::new above. The out pointer and length must be valid and unaliased because they
+            // come from a mutable slice reference, and CTR_DRBG_generate doesn't retain them after
+            // it returns.
             unsafe {
                 bssl_sys::CTR_DRBG_generate(
                     self.0,
@@ -56,10 +66,13 @@
 
 impl Drop for Drbg {
     fn drop(&mut self) {
+        // SAFETY: We know that self.0 is valid because it was initialised from CTR_DRBG_new in
+        // Drbg::new above, and this is the only place that frees it.
         unsafe {
             bssl_sys::CTR_DRBG_free(self.0);
         }
     }
 }
 
+// SAFETY: CTR_DRBG functions can be called from any thread.
 unsafe impl Send for Drbg {}
diff --git a/prng_seeder/src/main.rs b/prng_seeder/src/main.rs
index 924481a..f8b0c63 100644
--- a/prng_seeder/src/main.rs
+++ b/prng_seeder/src/main.rs
@@ -70,6 +70,7 @@
 fn setup() -> Result<(ConditionerBuilder, UnixListener)> {
     configure_logging()?;
     let cli = Cli::try_parse()?;
+    // SAFETY: Nothing else sets the signal handler, so either it was set here or it is the default.
     unsafe { signal::signal(signal::Signal::SIGPIPE, signal::SigHandler::SigIgn) }
         .context("In setup, setting SIGPIPE to SIG_IGN")?;