Merge pull request OpenDDS#4344 from jrw972/action-validity

Publish/subscribe actions do not have a validity section
iguessthislldo · Nov 20, 2023 · d3a09ea · d3a09ea
2 parents 39514f9 + 2d5bc25
commit d3a09ea
Show file tree

Hide file tree

Showing 15 changed files with 558 additions and 32 deletions.
diff --git a/dds/DCPS/DisjointSequence.h b/dds/DCPS/DisjointSequence.h
@@ -165,13 +165,25 @@ class OpenDDS_Dcps_Export DisjointSequence {
         return;
       }
 
-      const T above = upper + 1;
-      for (typename Container::iterator i = lower_bound_i(lower - 1);
-          i != ranges_.end() && (i->first <= above || i->second <= above);
-          /* iterate in loop because of removal */) {
-        lower = (std::min)(lower, i->first);
-        upper = (std::max)(upper, i->second);
-        ranges_.erase(i++);
+      typename Container::iterator pos = lower_bound_i(lower);
+      if (pos != ranges_.begin()) {
+        std::advance(pos, -1);
+      }
+
+      typename Container::iterator limit = lower_bound_i(upper);
+      if (limit != ranges_.end()) {
+        std::advance(limit, 1);
+      }
+
+      while (pos != limit) {
+        if ((upper < pos->first || lower > pos->second) &&
+            !(static_cast<T>(pos->first - 1) == upper || static_cast<T>(pos->second + 1) == lower)) {
+          ++pos;
+        } else {
+          lower = (std::min)(lower, pos->first);
+          upper = (std::max)(upper, pos->second);
+          ranges_.erase(pos++);
+        }
       }
 
       ranges_.insert(TPair(lower, upper));

diff --git a/dds/DCPS/security/AccessControl/Permissions.cpp b/dds/DCPS/security/AccessControl/Permissions.cpp
@@ -166,6 +166,29 @@ int Permissions::load(const SSL::SignedDocument& doc)
                     action.partitions.push_back(to_string(partitionNode));
                   }
                 }
+              } else if (ACE_TEXT("validity") == XStr(topicListNode->getNodeName())) {
+                const xercesc::DOMNodeList* validityNodes = topicListNode->getChildNodes();
+                for (XMLSize_t vn = 0, vn_len = validityNodes->getLength(); vn < vn_len; ++vn) {
+                  const xercesc::DOMNode* validityNode = validityNodes->item(vn);
+                  const XStr v_tag = validityNode->getNodeName();
+                  if (v_tag == ACE_TEXT("not_before")) {
+                    if (!parse_time(validityNode->getTextContent(), action.validity.not_before)) {
+                      if (security_debug.access_error) {
+                        ACE_ERROR((LM_ERROR, "(%P|%t) ERROR: {access_error} Permissions::load: "
+                                   "invalid datetime in not_before\n"));
+                      }
+                      return -1;
+                    }
+                  } else if (v_tag == ACE_TEXT("not_after")) {
+                    if (!parse_time(validityNode->getTextContent(), action.validity.not_after)) {
+                      if (security_debug.access_error) {
+                        ACE_ERROR((LM_ERROR, "(%P|%t) ERROR: {access_error} Permissions::load: "
+                                   "invalid datetime in not_after\n"));
+                      }
+                      return -1;
+                    }
+                  }
+                }
               }
             }
 
@@ -276,6 +299,19 @@ bool Permissions::Action::partitions_match(const DDS::StringSeq& entity_partitio
   return allow_or_deny == ALLOW;
 }
 
+bool Permissions::Action::valid(time_t now_utc) const
+{
+  if (validity.not_before != 0 && now_utc < validity.not_before) {
+    return false;
+  }
+
+  if (validity.not_after != 0 && now_utc > validity.not_after) {
+    return false;
+  }
+
+  return true;
+}
+
 }
 }
 

diff --git a/dds/DCPS/security/AccessControl/Permissions.h b/dds/DCPS/security/AccessControl/Permissions.h
@@ -26,7 +26,7 @@ OPENDDS_BEGIN_VERSIONED_NAMESPACE_DECL
 namespace OpenDDS {
 namespace Security {
 
-struct Permissions : DCPS::RcObject {
+struct OpenDDS_Security_Export Permissions : DCPS::RcObject {
   typedef DCPS::RcHandle<Permissions> shared_ptr;
 
   enum AllowDeny_t {
@@ -42,15 +42,37 @@ struct Permissions : DCPS::RcObject {
   struct Validity_t {
     time_t not_before;
     time_t not_after;
+
+    Validity_t()
+      : not_before(0)
+      , not_after(0)
+    {}
+
+    Validity_t(time_t nb, time_t na)
+      : not_before(nb)
+      , not_after(na)
+    {}
+
+    bool operator==(const Validity_t& other) const
+    {
+      return not_before == other.not_before && not_after == other.not_after;
+    }
+
+    bool operator!=(const Validity_t& other) const
+    {
+      return not_before != other.not_before || not_after != other.not_after;
+    }
   };
 
-  struct Action {
+  struct OpenDDS_Security_Export Action {
     PublishSubscribe_t ps_type;
     std::vector<std::string> topics;
     std::vector<std::string> partitions;
+    Validity_t validity;
 
     bool topic_matches(const char* topic) const;
     bool partitions_match(const DDS::StringSeq& entity_partitions, AllowDeny_t allow_or_deny) const;
+    bool valid(time_t now_utc) const;
   };
 
   typedef std::vector<Action> Actions;

diff --git a/dds/DCPS/security/AccessControlBuiltInImpl.cpp b/dds/DCPS/security/AccessControlBuiltInImpl.cpp
@@ -396,15 +396,17 @@ ::CORBA::Boolean AccessControlBuiltInImpl::check_create_datawriter(
     return CommonUtilities::set_security_error(ex, -1, 0, "AccessControlBuiltInImpl::check_create_datawriter: Permissions grant not found");
   }
 
-  if (!validate_date_time(grant->validity, ex)) {
+  const time_t now_utc = utc_now();
+  if (!validate_date_time(grant->validity, now_utc, ex)) {
     return false;
   }
 
-  if (!search_permissions(topic_name, domain_id, partition, Permissions::PUBLISH, *grant, ex)) {
+  time_t expiration_time = grant->validity.not_after;
+  if (!search_permissions(topic_name, domain_id, partition, Permissions::PUBLISH, *grant, now_utc, expiration_time, ex)) {
     return false;
   }
 
-  make_task(local_rp_task_)->insert(permissions_handle, grant->validity.not_after);
+  make_task(local_rp_task_)->insert(permissions_handle, expiration_time);
 
   return true;
 }
@@ -459,15 +461,17 @@ ::CORBA::Boolean AccessControlBuiltInImpl::check_create_datareader(
     return CommonUtilities::set_security_error(ex, -1, 0, "AccessControlBuiltInImpl::check_create_datareader: Permissions grant not found");
   }
 
-  if (!validate_date_time(grant->validity, ex)) {
+  const time_t now_utc = utc_now();
+  if (!validate_date_time(grant->validity, now_utc, ex)) {
     return false;
   }
 
-  if (!search_permissions(topic_name, domain_id, partition, Permissions::SUBSCRIBE, *grant, ex)) {
+  time_t expiration_time = grant->validity.not_after;
+  if (!search_permissions(topic_name, domain_id, partition, Permissions::SUBSCRIBE, *grant, now_utc, expiration_time, ex)) {
     return false;
   }
 
-  make_task(local_rp_task_)->insert(permissions_handle, grant->validity.not_after);
+  make_task(local_rp_task_)->insert(permissions_handle, expiration_time);
 
   return true;
 }
@@ -525,7 +529,8 @@ ::CORBA::Boolean AccessControlBuiltInImpl::check_create_topic(
     return CommonUtilities::set_security_error(ex, -1, 0, "AccessControlBuiltInImpl::check_create_topic: grant not found");
   }
 
-  if (!validate_date_time(grant->validity, ex)) {
+  const time_t now_utc = utc_now();
+  if (!validate_date_time(grant->validity, now_utc, ex)) {
     return false;
   }
 
@@ -731,17 +736,19 @@ ::CORBA::Boolean AccessControlBuiltInImpl::check_remote_datawriter(
     return CommonUtilities::set_security_error(ex, -1, 0, "AccessControlBuiltInImpl::check_remote_datawriter: Permissions grant not found");
   }
 
-  if (!validate_date_time(grant->validity, ex)) {
+  const time_t now_utc = utc_now();
+  if (!validate_date_time(grant->validity, now_utc, ex)) {
     return false;
   }
 
+  time_t expiration_time = grant->validity.not_after;
   if (!search_permissions(publication_data.base.base.topic_name, domain_id,
                           publication_data.base.base.partition, Permissions::PUBLISH,
-                          *grant, ex)) {
+                          *grant, now_utc, expiration_time, ex)) {
     return false;
   }
 
-  make_task(remote_rp_task_)->insert(permissions_handle, grant->validity.not_after);
+  make_task(remote_rp_task_)->insert(permissions_handle, expiration_time);
 
   return true;
 }
@@ -791,17 +798,19 @@ ::CORBA::Boolean AccessControlBuiltInImpl::check_remote_datareader(
     return CommonUtilities::set_security_error(ex, -1, 0, "AccessControlBuiltInImpl::check_remote_datareader: Permissions grant not found");
   }
 
-  if (!validate_date_time(grant->validity, ex)) {
+  const time_t now_utc = utc_now();
+  if (!validate_date_time(grant->validity, now_utc, ex)) {
     return false;
   }
 
+  time_t expiration_time = grant->validity.not_after;
   if (!search_permissions(subscription_data.base.base.topic_name, domain_id,
                           subscription_data.base.base.partition, Permissions::SUBSCRIBE,
-                          *grant, ex)) {
+                          *grant, now_utc, expiration_time, ex)) {
     return false;
   }
 
-  make_task(remote_rp_task_)->insert(permissions_handle, grant->validity.not_after);
+  make_task(remote_rp_task_)->insert(permissions_handle, expiration_time);
 
   return true;
 }
@@ -892,7 +901,8 @@ ::CORBA::Boolean AccessControlBuiltInImpl::check_remote_topic(
     return CommonUtilities::set_security_error(ex, -1, 0, "AccessControlBuiltInImpl::check_remote_topic: grant not found");
   }
 
-  if (!validate_date_time(grant->validity, ex)) {
+  const time_t now_utc = utc_now();
+  if (!validate_date_time(grant->validity, now_utc, ex)) {
     return false;
   }
 
@@ -1298,8 +1308,17 @@ AccessControlBuiltInImpl::make_task(RevokePermissionsTask_rch& task)
   return task;
 }
 
+time_t AccessControlBuiltInImpl::utc_now()
+{
+  // Get the current time as UTC
+  const time_t now = std::time(0);
+  std::tm* const now_utc_tm = std::gmtime(&now);
+  return std::mktime(now_utc_tm);
+}
+
 bool AccessControlBuiltInImpl::validate_date_time(
   const Permissions::Validity_t& validity,
+  time_t now_utc,
   DDS::Security::SecurityException& ex)
 {
   if (validity.not_before == 0) {
@@ -1314,11 +1333,6 @@ bool AccessControlBuiltInImpl::validate_date_time(
     return false;
   }
 
-  // Get the current time as UTC
-  const time_t now = std::time(0);
-  std::tm* const now_utc_tm = std::gmtime(&now);
-  const time_t now_utc = std::mktime(now_utc_tm);
-
   if (now_utc < validity.not_before) {
     CommonUtilities::set_security_error(ex, -1, 0,
       "AccessControlBuiltInImpl::validate_date_time: Permissions grant hasn't started yet.");
@@ -1480,15 +1494,21 @@ bool AccessControlBuiltInImpl::search_permissions(
   const DDS::PartitionQosPolicy& partition,
   const Permissions::PublishSubscribe_t pub_or_sub,
   const Permissions::Grant& grant,
+  time_t now_utc,
+  time_t& expiration_time,
   DDS::Security::SecurityException& ex)
 {
   for (Permissions::Rules::const_iterator rit = grant.rules.begin(); rit != grant.rules.end(); ++rit) {
     if (rit->domains.has(domain_id)) {
       for (Permissions::Actions::const_iterator ait = rit->actions.begin(); ait != rit->actions.end(); ++ait) {
         if (ait->ps_type == pub_or_sub &&
             ait->topic_matches(topic_name) &&
-            ait->partitions_match(partition.name, rit->ad_type)) {
+            ait->partitions_match(partition.name, rit->ad_type) &&
+            ait->valid(now_utc)) {
           if (rit->ad_type == Permissions::ALLOW) {
+            if (ait->validity.not_after != 0) {
+              expiration_time = std::min(expiration_time, ait->validity.not_after);
+            }
             return true;
           } else {
             return CommonUtilities::set_security_error(ex, -1, 0, "AccessControlBuiltInImpl: DENY rule matched");

diff --git a/dds/DCPS/security/AccessControlBuiltInImpl.h b/dds/DCPS/security/AccessControlBuiltInImpl.h
@@ -295,7 +295,10 @@ class OpenDDS_Security_Export AccessControlBuiltInImpl
 
   RevokePermissionsTask_rch& make_task(RevokePermissionsTask_rch& task);
 
+  static time_t utc_now();
+
   bool validate_date_time(const Permissions::Validity_t& validity,
+                          time_t now_utc,
                           DDS::Security::SecurityException& ex);
 
   bool get_sec_attributes(DDS::Security::PermissionsHandle permissions_handle,
@@ -310,6 +313,8 @@ class OpenDDS_Security_Export AccessControlBuiltInImpl
                           const DDS::PartitionQosPolicy& partition,
                           Permissions::PublishSubscribe_t pub_or_sub,
                           const Permissions::Grant& grant,
+                          time_t now_utc,
+                          time_t& expiration_time,
                           DDS::Security::SecurityException& ex);
 
   void parse_class_id(const std::string& class_id,

diff --git a/dds/DCPS/security/SSL/SignedDocument.h b/dds/DCPS/security/SSL/SignedDocument.h
@@ -32,10 +32,11 @@ class OpenDDS_Security_Export SignedDocument {
   bool load(const std::string& uri, DDS::Security::SecurityException& ex);
   bool verify(const Certificate& ca);
 
-  const DDS::OctetSeq& original() const {return original_;}
-  const std::string& content() const {return content_;}
+  const DDS::OctetSeq& original() const { return original_; }
+  void content(const std::string& value) { content_ = value; }
+  const std::string& content() const { return content_; }
   bool verified() const { return verified_; }
-  const std::string& filename() const {return filename_;}
+  const std::string& filename() const { return filename_; }
 
   bool operator==(const SignedDocument& other) const;
 

diff --git a/docs/devguide/dds_security.rst b/docs/devguide/dds_security.rst
@@ -860,6 +860,19 @@ When no data tag list is given for an "allow" PSR rule, the empty set of data ta
 For "deny" PSR rules, the rule will apply if the associated DDS entity is using any of the data tags listed.
 When no data tag list is given for a "deny" PSR rule, the set of "all possible tags" is used as the default value.
 
+.. _dds_security--psr-validity:
+
+validity
+""""""""
+
+.. attention::
+
+   This is an OpenDDS extension.
+
+This structure defines the validity of a particular publish or subscribe action.
+Thus, it is possible to declare that an action is valid for some subset of the grant's validity.
+The format for `validity` is the same as :ref:`dds_security--validity`.
+
 .. _dds_security--default_rule:
 
 default_rule
@@ -975,3 +988,6 @@ The following DDS Security features are not implemented in OpenDDS.
 
 #. Signing (without encrypting) at the payload level, see :omgissue:`DDSSEC12-59`
 
+The following features are OpenDDS extensions:
+
+#. Validity of publish/subscribe actions :ref:`dds_security--psr-validity`.
diff --git a/docs/news.d/action-validity.rst b/docs/news.d/action-validity.rst
@@ -0,0 +1,5 @@
+.. news-prs: 4344
+
+.. news-start-section: Additions
+- It is now possible to specify the :ref:`validity for individual publish/subscribe actions <dds_security--psr-validity>` in DDS Security Permission documents.  This is an OpenDDS extension.
+.. news-end-section