lib,pdcp_nr: make sure we don extract MAC-I if integrity is not enabled. Make sure gNB does not enable integrity on DRBs

lib/include/srsran/upper/pdcp_entity_base.h

@@ -76,7 +76,7 @@ public:
     } else {
       integrity_direction = direction;
     }
-    logger.debug("LCID=%d, integrity=%s", lcid, srsran_direction_text[integrity_direction]);
+    logger.debug("Enabled integrity. LCID=%d, integrity=%s", lcid, srsran_direction_text[integrity_direction]);
   }
 
   void enable_encryption(srsran_direction_t direction = DIRECTION_TXRX)
@@ -89,7 +89,7 @@ public:
     } else {
       encryption_direction = direction;
     }
-    logger.debug("LCID=%d, encryption=%s", lcid, srsran_direction_text[integrity_direction]);
+    logger.debug("Enabled encryption. LCID=%d, encryption=%s", lcid, srsran_direction_text[integrity_direction]);
   }
 
   void enable_security_timed(srsran_direction_t direction, uint32_t sn)

lib/include/srsran/upper/pdcp_entity_nr.h

@@ -39,7 +39,7 @@ public:
                  srsran::task_sched_handle  task_sched_,
                  srslog::basic_logger&      logger,
                  uint32_t                   lcid);
-  ~pdcp_entity_nr() final;
+  ~pdcp_entity_nr() final = default;
   bool configure(const pdcp_config_t& cnfg_) final;
   void reset() final;
   void reestablish() final;

lib/src/pdcp/pdcp_entity_nr.cc

@@ -32,15 +32,6 @@ pdcp_entity_nr::pdcp_entity_nr(srsue::rlc_interface_pdcp* rlc_,
   encryption_direction = DIRECTION_NONE;
 }
 
-pdcp_entity_nr::~pdcp_entity_nr() {}
-
-// Reestablishment procedure: 38.323 5.2
-void pdcp_entity_nr::reestablish()
-{
-  logger.info("Re-establish %s with bearer ID: %d", rb_name.c_str(), cfg.bearer_id);
-  // TODO
-}
-
 bool pdcp_entity_nr::configure(const pdcp_config_t& cnfg_)
 {
   if (active) {
@@ -81,6 +72,13 @@ bool pdcp_entity_nr::configure(const pdcp_config_t& cnfg_)
   return true;
 }
 
+// Reestablishment procedure: 38.323 5.2
+void pdcp_entity_nr::reestablish()
+{
+  logger.info("Re-establish %s with bearer ID: %d", rb_name.c_str(), cfg.bearer_id);
+  // TODO
+}
+
 // Used to stop/pause the entity (called on RRC conn release)
 void pdcp_entity_nr::reset()
 {
@@ -121,9 +119,10 @@ void pdcp_entity_nr::write_sdu(unique_byte_buffer_t sdu, int sn)
   // Perform header compression TODO
 
   // Integrity protection
-  uint8_t mac[4];
+  uint8_t mac[4] = {};
+  if (is_drb() && (integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX)) {
     integrity_generate(sdu->msg, sdu->N_bytes, tx_next, mac);
-
+  }
   // Ciphering
   cipher_encrypt(sdu->msg, sdu->N_bytes, tx_next, sdu->msg);
 
@@ -131,11 +130,21 @@ void pdcp_entity_nr::write_sdu(unique_byte_buffer_t sdu, int sn)
   write_data_header(sdu, tx_next);
 
   // Append MAC-I
+  if (is_drb() && (integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX)) {
     append_mac(sdu, mac);
-
+  }
   // Set meta-data for RLC AM
   sdu->md.pdcp_sn = tx_next;
 
+  logger.info(sdu->msg,
+              sdu->N_bytes,
+              "TX %s PDU, HFN=%d, SN=%d, integrity=%s, encryption=%s",
+              rb_name.c_str(),
+              HFN(tx_next),
+              SN(tx_next),
+              srsran_direction_text[integrity_direction],
+              srsran_direction_text[encryption_direction]);
+
   // Check if PDCP is associated with more than on RLC entity TODO
   // Write to lower layers
   rlc->write_sdu(lcid, std::move(sdu));
@@ -166,8 +175,10 @@ void pdcp_entity_nr::write_pdu(unique_byte_buffer_t pdu)
   discard_data_header(pdu); // TODO: Check wheather the header is part of integrity check.
 
   // Extract MAC
-  uint8_t mac[4];
+  uint8_t mac[4] = {};
+  if (is_drb() && (integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX)) {
     extract_mac(pdu, mac);
+  }
 
   // Calculate RCVD_COUNT
   uint32_t rcvd_hfn, rcvd_count;
@@ -180,16 +191,18 @@ void pdcp_entity_nr::write_pdu(unique_byte_buffer_t pdu)
   }
   rcvd_count = COUNT(rcvd_hfn, rcvd_sn);
 
-  logger.debug("RCVD_HFN %u RCVD_SN %u, RCVD_COUNT %u", rcvd_hfn, rcvd_sn, rcvd_count);
+  logger.debug("RCVD_HFN=%u, RCVD_SN=%u, RCVD_COUNT=%u", rcvd_hfn, rcvd_sn, rcvd_count);
 
   // Decripting
   cipher_decrypt(pdu->msg, pdu->N_bytes, rcvd_count, pdu->msg);
 
   // Integrity check
+  if (is_drb() && (integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX)) {
     bool is_valid = integrity_verify(pdu->msg, pdu->N_bytes, rcvd_count, mac);
     if (!is_valid) {
       return; // Invalid packet, drop.
     }
+  }
 
   // Check valid rcvd_count
   if (rcvd_count < rx_deliv) {
@@ -276,7 +289,7 @@ void pdcp_entity_nr::deliver_all_consecutive_counts()
 // Reordering Timer Callback (t-reordering)
 void pdcp_entity_nr::reordering_callback::operator()(uint32_t timer_id)
 {
-  parent->logger.debug("Reordering timer expired");
+  parent->logger.info("Reordering timer expired. Re-order queue size=%d", parent->reorder_queue.size());
 
   // Deliver all PDCP SDU(s) with associeted COUNT value(s) < RX_REORD
   for (std::map<uint32_t, unique_byte_buffer_t>::iterator it = parent->reorder_queue.begin();

srsgnb/src/stack/rrc/rrc_nr_ue.cc

@@ -1392,7 +1392,7 @@ int rrc_nr::ue::update_pdcp_bearers(const asn1::rrc_nr::radio_bearer_cfg_s& radi
     parent->pdcp->add_bearer(rnti, rlc_bearer->lc_ch_id, pdcp_cnfg);
 
     if (sec_ctx.is_as_sec_cfg_valid()) {
-      update_as_security(rlc_bearer->lc_ch_id);
+      update_as_security(rlc_bearer->lc_ch_id, false, false);
     }
   }
 

srsue/src/stack/rrc_nr/rrc_nr.cc

@@ -2079,7 +2079,6 @@ bool rrc_nr::apply_drb_add_mod(const drb_to_add_mod_s& drb_cfg)
 
 bool rrc_nr::apply_security_cfg(const security_cfg_s& security_cfg)
 {
-  // TODO derive correct keys
   if (security_cfg.key_to_use_present) {
     if (security_cfg.key_to_use.value != security_cfg_s::key_to_use_opts::options::secondary) {
       logger.warning("Only secondary key supported yet");
@@ -2120,6 +2119,7 @@ bool rrc_nr::apply_security_cfg(const security_cfg_s& security_cfg)
 
   // Apply security config for all known NR lcids
   for (auto& lcid : lcid_drb) {
+    logger.debug("Applying PDCP security config. LCID=%d", lcid.first);
     pdcp->config_security(lcid.first, sec_cfg);
     pdcp->enable_encryption(lcid.first);
   }