sync up various TODOs with issues (#2417)

Author: davepacheco

common/src/api/external/mod.rs

@@ -480,9 +480,6 @@ impl JsonSchema for RoleName {
 /// can fail (if the value is larger than i64::MAX).  We provide all of these for
 /// consumers' convenience.
 // TODO-cleanup This could benefit from a more complete implementation.
-// TODO-correctness RFD 4 requires that this be a multiple of 256 MiB.  We'll
-// need to write a validator for that.
-// /
 //
 // The maximum byte count of i64::MAX comes from the fact that this is stored in
 // the database as an i64.  Constraining it here ensures that we can't fail to

common/src/sql/dbinit.sql

@@ -1788,7 +1788,6 @@ CREATE TABLE omicron.public.device_auth_request (
 );
 
 -- Access tokens granted in response to successful device authorization flows.
--- TODO-security: expire tokens.
 CREATE TABLE omicron.public.device_access_token (
     token STRING(40) PRIMARY KEY,
     client_id UUID NOT NULL,

nexus/db-model/src/device_auth.rs

@@ -75,7 +75,6 @@ fn generate_token() -> String {
 /// suggested in RFC 8628 §6.1 (User Code Recommendations); q.v. also for
 /// a discussion of entropy requirements. On input, use codes should be
 /// uppercased, and characters not in this alphabet should be stripped.
-// TODO-security: user code tries should be rate-limited
 const USER_CODE_ALPHABET: [char; 20] = [
     'B', 'C', 'D', 'F', 'G', 'H', 'J', 'K', 'L', 'M', 'N', 'P', 'Q', 'R', 'S',
     'T', 'V', 'W', 'X', 'Z',

nexus/src/app/device_auth.rs

@@ -118,7 +118,6 @@ impl super::Nexus {
                 message: "device authorization request expired".to_string(),
             })
         } else {
-            // TODO-security: set an expiration time for the valid token.
             self.db_datastore
                 .device_access_token_create(
                     opctx,

nexus/src/app/mod.rs

@@ -63,7 +63,7 @@ pub(crate) const MAX_DISKS_PER_INSTANCE: u32 = 8;
 
 pub(crate) const MAX_NICS_PER_INSTANCE: usize = 8;
 
-// TODO-completness: Support multiple external IPs
+// TODO-completeness: Support multiple external IPs
 pub(crate) const MAX_EXTERNAL_IPS_PER_INSTANCE: usize = 1;
 
 pub(crate) struct ExternalServers {

nexus/src/app/sagas/instance_migrate.rs

@@ -194,7 +194,7 @@ async fn sim_instance_migrate(
     }
     let source_nat = SourceNatConfig::from(snat_ip.into_iter().next().unwrap());
 
-    // The TODOs below are tracked in
+    // The TODO items below are tracked in
     //   https://github.com/oxidecomputer/omicron/issues/1783
     let instance_hardware = InstanceHardware {
         runtime: runtime.into(),

nexus/src/app/silo.rs

@@ -517,7 +517,7 @@ impl super::Nexus {
         // silo_user_password_verify().
         // TODO-security There may still be some vulnerability to timing attack
         // here, in that we'll do one fewer database lookup if a user does not
-        // exist.
+        // exist.  Rate limiting might help.  See omicron#2184.
         let fetch_user = self
             .datastore()
             .silo_user_fetch_by_external_id(

nexus/src/bin/nexus.rs

@@ -5,13 +5,10 @@
 //! Executable program to run Nexus, the heart of the control plane
 
 // TODO
-// - TCP and HTTP KeepAlive parameters
 // - Server hostname
 // - Disable signals?
-// - Analogs for actix client_timeout (request timeout), client_shutdown (client
-//   shutdown timeout), server backlog, number of workers, max connections per
-//   worker, max connect-in-progress sockets, shutdown_timeout (server shutdown
-//   timeout)
+// - General networking and runtime tuning for availability and security: see
+//   omicron#2184, omicron#2414.
 
 use clap::Parser;
 use omicron_common::cmd::fatal;

nexus/src/db/datastore/role.rs

@@ -236,8 +236,6 @@ impl DataStore {
     where
         T: authz::ApiResourceWithRolesType + AuthorizedResource + Clone,
     {
-        // TODO-security We should carefully review what permissions are
-        // required for modifying the policy of a resource.
         opctx.authorize(authz::Action::ModifyPolicy, authz_resource).await?;
 
         let authz_resource = authz_resource.clone();

nexus/src/db/fixed_data/silo_user.rs

@@ -13,7 +13,7 @@ lazy_static! {
     /// local development
     // TODO-security Once we have a way to bootstrap the initial Silo with the
     // initial privileged user, this user should be created in the test suite,
-    // not automatically at Nexus startup.
+    // not automatically at Nexus startup.  See omicron#2305.
     pub static ref USER_TEST_PRIVILEGED: db::model::SiloUser =
         db::model::SiloUser::new(
             *db::fixed_data::silo::SILO_ID,
@@ -47,7 +47,7 @@ lazy_static! {
     /// Test user that's granted no privileges, used for automated testing
     // TODO-security Once we have a way to bootstrap the initial Silo with the
     // initial privileged user, this user should be created in the test suite,
-    // not automatically at Nexus startup.
+    // not automatically at Nexus startup.  See omicron#2305.
     pub static ref USER_TEST_UNPRIVILEGED: db::model::SiloUser =
         db::model::SiloUser::new(
             *db::fixed_data::silo::SILO_ID,