From 25b8a35d7ac30aecb25daba688d0c7ed8ed0682d Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Tue, 19 Apr 2022 16:39:14 +0100 Subject: [PATCH] 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 | 4 +- lib/include/srsran/upper/pdcp_entity_nr.h | 2 +- lib/src/pdcp/pdcp_entity_nr.cc | 55 +++++++++++++-------- srsgnb/src/stack/rrc/rrc_nr_ue.cc | 2 +- srsue/src/stack/rrc_nr/rrc_nr.cc | 2 +- 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/lib/include/srsran/upper/pdcp_entity_base.h b/lib/include/srsran/upper/pdcp_entity_base.h index e70be88eb..59c8c51a7 100644 --- a/lib/include/srsran/upper/pdcp_entity_base.h +++ b/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) diff --git a/lib/include/srsran/upper/pdcp_entity_nr.h b/lib/include/srsran/upper/pdcp_entity_nr.h index 2a7b89f8f..c1b06ecbe 100644 --- a/lib/include/srsran/upper/pdcp_entity_nr.h +++ b/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; diff --git a/lib/src/pdcp/pdcp_entity_nr.cc b/lib/src/pdcp/pdcp_entity_nr.cc index f3dd2597c..e50f7bfc2 100644 --- a/lib/src/pdcp/pdcp_entity_nr.cc +++ b/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]; - integrity_generate(sdu->msg, sdu->N_bytes, tx_next, mac); - + 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 - append_mac(sdu, mac); - + 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]; - extract_mac(pdu, mac); + 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,15 +191,17 @@ 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 - bool is_valid = integrity_verify(pdu->msg, pdu->N_bytes, rcvd_count, mac); - if (!is_valid) { - return; // Invalid packet, drop. + 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 @@ -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::iterator it = parent->reorder_queue.begin(); diff --git a/srsgnb/src/stack/rrc/rrc_nr_ue.cc b/srsgnb/src/stack/rrc/rrc_nr_ue.cc index 9ca216c8b..8a80f3885 100644 --- a/srsgnb/src/stack/rrc/rrc_nr_ue.cc +++ b/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); } } diff --git a/srsue/src/stack/rrc_nr/rrc_nr.cc b/srsue/src/stack/rrc_nr/rrc_nr.cc index 31bb4709d..09db08d16 100644 --- a/srsue/src/stack/rrc_nr/rrc_nr.cc +++ b/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); }