From 9e113f8b277cdb6036ffad1af0e67bd4ed1671cc Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Sun, 1 Mar 2020 14:51:51 +0100 Subject: [PATCH] pdcp: allow to configure security for rx/tx seperately previously PDCP security (integrity and ciphering) could only be enabled for both Rx and Tx at the same time. this, however, caused an issue during the conformance testing in which, in TC_8_2_1_1() for example, the eNB sends a SecModeCommand and a RRC Reconfiguration in the same MAC TB. In this case, the eNB needs to be able to enable DL security right after sending the SecModeCmd in order to send the RRCReconfig encrypted. However, enabling UL security needs to be postponed until after the SecModeComplete is received. This patch allows to enable PDCP security for rx/tx independently if that is needed. The default way is like before, enabling it for tx/rx at the same time. --- lib/include/srslte/interfaces/ue_interfaces.h | 22 ++++++------ lib/include/srslte/upper/pdcp.h | 4 +-- lib/include/srslte/upper/pdcp_entity_base.h | 36 ++++++++++++++++--- lib/include/srslte/upper/pdcp_entity_nr.h | 1 + lib/src/upper/pdcp.cc | 8 ++--- lib/src/upper/pdcp_entity_base.cc | 1 + lib/src/upper/pdcp_entity_lte.cc | 33 +++++++++-------- lib/src/upper/pdcp_entity_nr.cc | 16 ++++----- lib/test/upper/pdcp_lte_test.h | 4 +-- lib/test/upper/pdcp_nr_test.h | 4 +-- srsenb/src/stack/upper/pdcp.cc | 4 +-- srsue/src/stack/rrc/rrc.cc | 8 ++--- 12 files changed, 85 insertions(+), 56 deletions(-) diff --git a/lib/include/srslte/interfaces/ue_interfaces.h b/lib/include/srslte/interfaces/ue_interfaces.h index 935e20855..10dbf0de0 100644 --- a/lib/include/srslte/interfaces/ue_interfaces.h +++ b/lib/include/srslte/interfaces/ue_interfaces.h @@ -40,6 +40,7 @@ #include "srslte/interfaces/rrc_interface_types.h" #include "srslte/phy/channel/channel.h" #include "srslte/phy/rf/rf.h" +#include "srslte/upper/pdcp_entity_base.h" namespace srsue { @@ -227,25 +228,26 @@ public: class pdcp_interface_rrc { public: - virtual void reestablish() = 0; - virtual void reestablish(uint32_t lcid) = 0; - virtual void reset() = 0; - virtual void write_sdu(uint32_t lcid, srslte::unique_byte_buffer_t sdu, bool blocking) = 0; - virtual void add_bearer(uint32_t lcid, srslte::pdcp_config_t cnfg) = 0; - virtual void change_lcid(uint32_t old_lcid, uint32_t new_lcid) = 0; + virtual void reestablish() = 0; + virtual void reestablish(uint32_t lcid) = 0; + virtual void reset() = 0; + virtual void write_sdu(uint32_t lcid, srslte::unique_byte_buffer_t sdu, bool blocking) = 0; + virtual void add_bearer(uint32_t lcid, srslte::pdcp_config_t cnfg) = 0; + virtual void change_lcid(uint32_t old_lcid, uint32_t new_lcid) = 0; virtual void config_security(uint32_t lcid, uint8_t* k_rrc_enc_, uint8_t* k_rrc_int_, uint8_t* k_up_enc_, srslte::CIPHERING_ALGORITHM_ID_ENUM cipher_algo_, - srslte::INTEGRITY_ALGORITHM_ID_ENUM integ_algo_) = 0; + srslte::INTEGRITY_ALGORITHM_ID_ENUM integ_algo_) = 0; virtual void config_security_all(uint8_t* k_rrc_enc_, uint8_t* k_rrc_int_, uint8_t* k_up_enc_, srslte::CIPHERING_ALGORITHM_ID_ENUM cipher_algo_, - srslte::INTEGRITY_ALGORITHM_ID_ENUM integ_algo_) = 0; - virtual void enable_integrity(uint32_t lcid) = 0; - virtual void enable_encryption(uint32_t lcid) = 0; + srslte::INTEGRITY_ALGORITHM_ID_ENUM integ_algo_) = 0; + virtual void enable_integrity(uint32_t lcid, srslte::srslte_direction_t direction) = 0; + virtual void enable_encryption(uint32_t lcid, + srslte::srslte_direction_t direction = srslte::srslte_direction_t::DIRECTION_TXRX) = 0; }; // PDCP interface for RLC diff --git a/lib/include/srslte/upper/pdcp.h b/lib/include/srslte/upper/pdcp.h index 3aa39e68b..dd4133f48 100644 --- a/lib/include/srslte/upper/pdcp.h +++ b/lib/include/srslte/upper/pdcp.h @@ -61,8 +61,8 @@ public: uint8_t* k_up_enc, CIPHERING_ALGORITHM_ID_ENUM cipher_algo, INTEGRITY_ALGORITHM_ID_ENUM integ_algo); - void enable_integrity(uint32_t lcid); - void enable_encryption(uint32_t lcid); + void enable_integrity(uint32_t lcid, srslte_direction_t direction); + void enable_encryption(uint32_t lcid, srslte_direction_t direction); bool get_bearer_status(uint32_t lcid, uint16_t* dlsn, uint16_t* dlhfn, uint16_t* ulsn, uint16_t* ulhfn); // RLC interface diff --git a/lib/include/srslte/upper/pdcp_entity_base.h b/lib/include/srslte/upper/pdcp_entity_base.h index f39e129ab..fbada6a68 100644 --- a/lib/include/srslte/upper/pdcp_entity_base.h +++ b/lib/include/srslte/upper/pdcp_entity_base.h @@ -24,10 +24,11 @@ #include "srslte/common/buffer_pool.h" #include "srslte/common/common.h" +#include "srslte/common/interfaces_common.h" #include "srslte/common/log.h" #include "srslte/common/security.h" #include "srslte/common/threads.h" -#include "srslte/interfaces/ue_interfaces.h" +#include "srslte/common/timers.h" #include namespace srslte { @@ -51,6 +52,10 @@ typedef enum { } pdcp_d_c_t; static const char pdcp_d_c_text[PDCP_D_C_N_ITEMS][20] = {"Control PDU", "Data PDU"}; +// Specifies in which direction security (integrity and ciphering) are enabled for PDCP +typedef enum { DIRECTION_NONE = 0, DIRECTION_TX, DIRECTION_RX, DIRECTION_TXRX, DIRECTION_N_ITEMS } srslte_direction_t; +static const char srslte_direction_text[DIRECTION_N_ITEMS][6] = {"none", "tx", "rx", "tx/rx"}; + /**************************************************************************** * PDCP Entity interface * Common interface for LTE and NR PDCP entities @@ -68,8 +73,29 @@ public: bool is_drb() { return cfg.rb_type == PDCP_RB_IS_DRB; } // RRC interface - void enable_integrity() { do_integrity = true; } - void enable_encryption() { do_encryption = true; } + void enable_integrity(srslte_direction_t direction = DIRECTION_TXRX) + { + // if either DL or UL is already enabled, both are enabled + if (integrity_direction == DIRECTION_TX && direction == DIRECTION_RX) { + integrity_direction = DIRECTION_TXRX; + } else if (integrity_direction == DIRECTION_RX && direction == DIRECTION_TX) { + integrity_direction = DIRECTION_TXRX; + } else { + integrity_direction = direction; + } + } + + void enable_encryption(srslte_direction_t direction = DIRECTION_TXRX) + { + // if either DL or UL is already enabled, both are enabled + if (encryption_direction == DIRECTION_TX && direction == DIRECTION_RX) { + encryption_direction = DIRECTION_TXRX; + } else if (encryption_direction == DIRECTION_RX && direction == DIRECTION_TX) { + encryption_direction = DIRECTION_TXRX; + } else { + encryption_direction = direction; + } + } void config_security(uint8_t* k_rrc_enc_, uint8_t* k_rrc_int_, @@ -95,8 +121,8 @@ protected: bool active = false; uint32_t lcid = 0; - bool do_integrity = false; - bool do_encryption = false; + srslte_direction_t integrity_direction = DIRECTION_NONE; + srslte_direction_t encryption_direction = DIRECTION_NONE; pdcp_config_t cfg = {1, PDCP_RB_IS_DRB, diff --git a/lib/include/srslte/upper/pdcp_entity_nr.h b/lib/include/srslte/upper/pdcp_entity_nr.h index 3cb4c0645..a3b07ac26 100644 --- a/lib/include/srslte/upper/pdcp_entity_nr.h +++ b/lib/include/srslte/upper/pdcp_entity_nr.h @@ -29,6 +29,7 @@ #include "srslte/common/log.h" #include "srslte/common/security.h" #include "srslte/common/threads.h" +#include "srslte/interfaces/ue_interfaces.h" #include namespace srslte { diff --git a/lib/src/upper/pdcp.cc b/lib/src/upper/pdcp.cc index 5bf145c57..78932556d 100644 --- a/lib/src/upper/pdcp.cc +++ b/lib/src/upper/pdcp.cc @@ -229,20 +229,20 @@ void pdcp::config_security_all(uint8_t* k_rrc_enc, pthread_rwlock_unlock(&rwlock); } -void pdcp::enable_integrity(uint32_t lcid) +void pdcp::enable_integrity(uint32_t lcid, srslte_direction_t direction) { pthread_rwlock_rdlock(&rwlock); if (valid_lcid(lcid)) { - pdcp_array.at(lcid)->enable_integrity(); + pdcp_array.at(lcid)->enable_integrity(direction); } pthread_rwlock_unlock(&rwlock); } -void pdcp::enable_encryption(uint32_t lcid) +void pdcp::enable_encryption(uint32_t lcid, srslte_direction_t direction) { pthread_rwlock_rdlock(&rwlock); if (valid_lcid(lcid)) { - pdcp_array.at(lcid)->enable_encryption(); + pdcp_array.at(lcid)->enable_encryption(direction); } pthread_rwlock_unlock(&rwlock); } diff --git a/lib/src/upper/pdcp_entity_base.cc b/lib/src/upper/pdcp_entity_base.cc index 3c31185db..a7ea8ec1a 100644 --- a/lib/src/upper/pdcp_entity_base.cc +++ b/lib/src/upper/pdcp_entity_base.cc @@ -22,6 +22,7 @@ #include "srslte/upper/pdcp_entity_base.h" #include "srslte/common/int_helpers.h" #include "srslte/common/security.h" +#include namespace srslte { diff --git a/lib/src/upper/pdcp_entity_lte.cc b/lib/src/upper/pdcp_entity_lte.cc index 2742e279f..1de2850bf 100644 --- a/lib/src/upper/pdcp_entity_lte.cc +++ b/lib/src/upper/pdcp_entity_lte.cc @@ -45,8 +45,8 @@ void pdcp_entity_lte::init(uint32_t lcid_, pdcp_config_t cfg_) active = true; tx_count = 0; rx_count = 0; - do_integrity = false; - do_encryption = false; + integrity_direction = DIRECTION_NONE; + encryption_direction = DIRECTION_NONE; if (is_srb()) { reordering_window = 0; @@ -101,11 +101,11 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking) { log->info_hex(sdu->msg, sdu->N_bytes, - "TX %s SDU, SN: %d, do_integrity = %s, do_encryption = %s", + "TX %s SDU, SN=%d, integrity=%s, encryption=%s", rrc->get_rb_name(lcid).c_str(), tx_count, - (do_integrity) ? "true" : "false", - (do_encryption) ? "true" : "false"); + srslte_direction_text[integrity_direction], + srslte_direction_text[encryption_direction]); { std::unique_lock lock(mutex); @@ -113,7 +113,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking) write_data_header(sdu, tx_count); uint8_t mac[4] = {}; - if (do_integrity) { + if (integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX) { integrity_generate(sdu->msg, sdu->N_bytes, tx_count, mac); } @@ -122,7 +122,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking) append_mac(sdu, mac); } - if (do_encryption) { + if (encryption_direction == DIRECTION_TX || encryption_direction == DIRECTION_TXRX) { cipher_encrypt( &sdu->msg[cfg.hdr_len_bytes], sdu->N_bytes - cfg.hdr_len_bytes, tx_count, &sdu->msg[cfg.hdr_len_bytes]); log->info_hex(sdu->msg, sdu->N_bytes, "TX %s SDU (encrypted)", rrc->get_rb_name(lcid).c_str()); @@ -138,11 +138,11 @@ void pdcp_entity_lte::write_pdu(unique_byte_buffer_t pdu) { log->info_hex(pdu->msg, pdu->N_bytes, - "RX %s PDU (%d B), do_integrity = %s, do_encryption = %s", + "RX %s PDU (%d B), integrity=%s, encryption=%s", rrc->get_rb_name(lcid).c_str(), pdu->N_bytes, - (do_integrity) ? "true" : "false", - (do_encryption) ? "true" : "false"); + srslte_direction_text[integrity_direction], + srslte_direction_text[encryption_direction]); // Sanity check if (pdu->N_bytes <= cfg.hdr_len_bytes) { @@ -185,7 +185,7 @@ void pdcp_entity_lte::handle_srb_pdu(srslte::unique_byte_buffer_t pdu) } // Perform decription - if (do_encryption) { + if (encryption_direction == DIRECTION_RX || encryption_direction == DIRECTION_TXRX) { cipher_decrypt(&pdu->msg[cfg.hdr_len_bytes], pdu->N_bytes - cfg.hdr_len_bytes, count, &pdu->msg[cfg.hdr_len_bytes]); log->info_hex(pdu->msg, pdu->N_bytes, "RX %s PDU (decrypted)", rrc->get_rb_name(lcid).c_str()); } @@ -195,7 +195,7 @@ void pdcp_entity_lte::handle_srb_pdu(srslte::unique_byte_buffer_t pdu) extract_mac(pdu, mac); // Perfrom integrity checks - if (do_integrity) { + if (integrity_direction == DIRECTION_RX || integrity_direction == DIRECTION_TXRX) { if (not integrity_verify(pdu->msg, pdu->N_bytes, count, mac)) { log->error_hex(pdu->msg, pdu->N_bytes, "%s Dropping PDU", rrc->get_rb_name(lcid).c_str()); return; // Discard @@ -233,7 +233,7 @@ void pdcp_entity_lte::handle_um_drb_pdu(srslte::unique_byte_buffer_t pdu) } uint32_t count = (rx_hfn << cfg.sn_len) | sn; - if (do_encryption) { + if (encryption_direction == DIRECTION_RX || encryption_direction == DIRECTION_TXRX) { cipher_decrypt(pdu->msg, pdu->N_bytes, count, pdu->msg); log->debug_hex(pdu->msg, pdu->N_bytes, "RX %s PDU (decrypted)", rrc->get_rb_name(lcid).c_str()); } @@ -245,9 +245,8 @@ void pdcp_entity_lte::handle_um_drb_pdu(srslte::unique_byte_buffer_t pdu) } // Pass to upper layers - log->info_hex(pdu->msg, pdu->N_bytes, "RX %s PDU SN: %d", rrc->get_rb_name(lcid).c_str(), sn); + log->info_hex(pdu->msg, pdu->N_bytes, "RX %s PDU SN=%d", rrc->get_rb_name(lcid).c_str(), sn); gw->write_pdu(lcid, std::move(pdu)); - return; } // DRBs mapped on RLC AM, without re-ordering (5.1.2.1.2) @@ -260,7 +259,7 @@ void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu) int32_t sn_diff_last_submit = sn - last_submitted_pdcp_rx_sn; int32_t sn_diff_next_pdcp_rx_sn = sn - next_pdcp_rx_sn; - log->debug("RX HFN: %d, SN: %d, Last_Submitted_PDCP_RX_SN: %d, Next_PDCP_RX_SN %d\n", + log->debug("RX HFN: %d, SN=%d, Last_Submitted_PDCP_RX_SN=%d, Next_PDCP_RX_SN=%d\n", rx_hfn, sn, last_submitted_pdcp_rx_sn, @@ -306,7 +305,7 @@ void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu) last_submitted_pdcp_rx_sn = sn; // Pass to upper layers - log->info_hex(pdu->msg, pdu->N_bytes, "RX %s PDU SN: %d", rrc->get_rb_name(lcid).c_str(), sn); + log->info_hex(pdu->msg, pdu->N_bytes, "RX %s PDU SN=%d", rrc->get_rb_name(lcid).c_str(), sn); gw->write_pdu(lcid, std::move(pdu)); return; } diff --git a/lib/src/upper/pdcp_entity_nr.cc b/lib/src/upper/pdcp_entity_nr.cc index 467fb96f2..96dd61752 100644 --- a/lib/src/upper/pdcp_entity_nr.cc +++ b/lib/src/upper/pdcp_entity_nr.cc @@ -44,8 +44,8 @@ void pdcp_entity_nr::init(uint32_t lcid_, pdcp_config_t cfg_) lcid = lcid_; cfg = cfg_; active = true; - do_integrity = false; - do_encryption = false; + integrity_direction = DIRECTION_NONE; + encryption_direction = DIRECTION_NONE; window_size = 1 << (cfg.sn_len - 1); @@ -88,10 +88,10 @@ void pdcp_entity_nr::write_sdu(unique_byte_buffer_t sdu, bool blocking) // Log SDU log->info_hex(sdu->msg, sdu->N_bytes, - "TX %s SDU, do_integrity = %s, do_encryption = %s", + "TX %s SDU, integrity=%s, encryption=%s", rrc->get_rb_name(lcid).c_str(), - (do_integrity) ? "true" : "false", - (do_encryption) ? "true" : "false"); + srslte_direction_text[integrity_direction], + srslte_direction_text[encryption_direction]); // Check for COUNT overflow if (tx_overflow) { @@ -146,11 +146,11 @@ void pdcp_entity_nr::write_pdu(unique_byte_buffer_t pdu) // Log PDU log->info_hex(pdu->msg, pdu->N_bytes, - "RX %s PDU (%d B), do_integrity = %s, do_encryption = %s", + "RX %s PDU (%d B), integrity=%s, encryption=%s", rrc->get_rb_name(lcid).c_str(), pdu->N_bytes, - (do_integrity) ? "true" : "false", - (do_encryption) ? "true" : "false"); + srslte_direction_text[integrity_direction], + srslte_direction_text[encryption_direction]); // Sanity check if (pdu->N_bytes <= cfg.hdr_len_bytes) { diff --git a/lib/test/upper/pdcp_lte_test.h b/lib/test/upper/pdcp_lte_test.h index a1509fa71..54e23c03b 100644 --- a/lib/test/upper/pdcp_lte_test.h +++ b/lib/test/upper/pdcp_lte_test.h @@ -82,8 +82,8 @@ public: pdcp.init(0, cfg); pdcp.config_security( sec_cfg.k_enc_rrc, sec_cfg.k_int_rrc, sec_cfg.k_enc_up, sec_cfg.k_int_up, sec_cfg.enc_algo, sec_cfg.int_algo); - pdcp.enable_integrity(); - pdcp.enable_encryption(); + pdcp.enable_integrity(srslte::DIRECTION_TXRX); + pdcp.enable_encryption(srslte::DIRECTION_TXRX); } void set_pdcp_initial_state(pdcp_lte_initial_state init_state) diff --git a/lib/test/upper/pdcp_nr_test.h b/lib/test/upper/pdcp_nr_test.h index bb050a71a..2b92c7167 100644 --- a/lib/test/upper/pdcp_nr_test.h +++ b/lib/test/upper/pdcp_nr_test.h @@ -103,8 +103,8 @@ public: pdcp.init(0, cfg); pdcp.config_security( sec_cfg.k_enc_rrc, sec_cfg.k_int_rrc, sec_cfg.k_enc_up, sec_cfg.k_int_up, sec_cfg.enc_algo, sec_cfg.int_algo); - pdcp.enable_integrity(); - pdcp.enable_encryption(); + pdcp.enable_integrity(srslte::DIRECTION_TXRX); + pdcp.enable_encryption(srslte::DIRECTION_TXRX); } void set_pdcp_initial_state(pdcp_initial_state init_state) diff --git a/srsenb/src/stack/upper/pdcp.cc b/srsenb/src/stack/upper/pdcp.cc index 468168058..c23af7320 100644 --- a/srsenb/src/stack/upper/pdcp.cc +++ b/srsenb/src/stack/upper/pdcp.cc @@ -127,14 +127,14 @@ void pdcp::config_security(uint16_t rnti, void pdcp::enable_integrity(uint16_t rnti, uint32_t lcid) { pthread_rwlock_rdlock(&rwlock); - users[rnti].pdcp->enable_integrity(lcid); + users[rnti].pdcp->enable_integrity(lcid, srslte::DIRECTION_TXRX); pthread_rwlock_unlock(&rwlock); } void pdcp::enable_encryption(uint16_t rnti, uint32_t lcid) { pthread_rwlock_rdlock(&rwlock); - users[rnti].pdcp->enable_encryption(lcid); + users[rnti].pdcp->enable_encryption(lcid, srslte::DIRECTION_TXRX); pthread_rwlock_unlock(&rwlock); } diff --git a/srsue/src/stack/rrc/rrc.cc b/srsue/src/stack/rrc/rrc.cc index 222047731..fb26cf60e 100644 --- a/srsue/src/stack/rrc/rrc.cc +++ b/srsue/src/stack/rrc/rrc.cc @@ -1968,9 +1968,9 @@ void rrc::parse_dl_dcch(uint32_t lcid, unique_byte_buffer_t pdu) // Configure PDCP for security pdcp->config_security(lcid, k_rrc_enc, k_rrc_int, k_up_enc, cipher_algo, integ_algo); - pdcp->enable_integrity(lcid); + pdcp->enable_integrity(lcid, DIRECTION_TXRX); send_security_mode_complete(); - pdcp->enable_encryption(lcid); + pdcp->enable_encryption(lcid, DIRECTION_TXRX); break; case dl_dcch_msg_type_c::c1_c_::types::rrc_conn_recfg: transaction_id = c1->rrc_conn_recfg().rrc_transaction_id; @@ -2660,8 +2660,8 @@ void rrc::add_srb(srb_to_add_mod_s* srb_cnfg) pdcp->add_bearer(srb_cnfg->srb_id, make_srb_pdcp_config_t(srb_cnfg->srb_id, true)); if (RB_ID_SRB2 == srb_cnfg->srb_id) { pdcp->config_security(srb_cnfg->srb_id, k_rrc_enc, k_rrc_int, k_up_enc, cipher_algo, integ_algo); - pdcp->enable_integrity(srb_cnfg->srb_id); - pdcp->enable_encryption(srb_cnfg->srb_id); + pdcp->enable_integrity(srb_cnfg->srb_id, DIRECTION_TXRX); + pdcp->enable_encryption(srb_cnfg->srb_id, DIRECTION_TXRX); } // Setup RLC