From f85e3adc5ad033c3fc5e5057626134c9520d5049 Mon Sep 17 00:00:00 2001 From: Cedric Roux Date: Fri, 9 Feb 2024 09:41:25 +0100 Subject: [PATCH] nrUE: bugfix/improvement in security mode handling The use of security_mode_completed in the PDCP entity was a hack. Plus it was not working at all with the deregistration request when nea2 is used, sent when doing ctrl+c in the nrUE. So let's remove it. And let's handle activation of integrity and ciphering more in accordance with what 38.331 says. SecurityModeComplete has to be sent integrity protected, but not ciphered. Only after should the ciphering be activated. (See 38.331 5.3.4.3.) --- openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c | 4 +--- openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h | 13 ------------- openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c | 1 - openair2/RRC/NR_UE/rrc_UE.c | 14 +++++++++++++- 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c index 2a197c3c4f..374d1cfd55 100644 --- a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c +++ b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.c @@ -261,13 +261,11 @@ static int nr_pdcp_entity_process_sdu(nr_pdcp_entity_t *entity, memset(buf + header_size + size, 0, PDCP_INTEGRITY_SIZE); } - if (entity->has_ciphering && (entity->is_gnb || entity->security_mode_completed)) { + if (entity->has_ciphering) { entity->cipher(entity->security_context, (unsigned char *)buf + header_size + sdap_header_size, size + integrity_size - sdap_header_size, entity->rb_id, count, entity->is_gnb ? 1 : 0); - } else { - entity->security_mode_completed = true; } entity->tx_next++; diff --git a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h index aae1ffb317..82f701f037 100644 --- a/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h +++ b/openair2/LAYER2/nr_pdcp/nr_pdcp_entity.h @@ -167,19 +167,6 @@ typedef struct nr_pdcp_entity_t { int rx_maxsize; nr_pdcp_statistics_t stats; - // WARNING: This is a hack! - // 3GPP TS 38.331 (RRC) version 15.3 - // Section 5.3.4.3 Reception of the SecurityModeCommand by the UE - // The UE needs to send the Security Mode Complete message. However, the message - // needs to be sent without being ciphered. - // However: - // 1- The Security Mode Command arrives to the UE with the cipher algo (e.g., nea2). - // 2- The UE is configured with the cipher algo. - // 3- The Security Mode Complete message is sent to the itti task queue. - // 4- The ITTI task, forwards the message ciphering (e.g., nea2) it. - // 5- The gNB cannot understand the ciphered Security Mode Complete message. - bool security_mode_completed; - /* Keep tracks of whether the PDCP entity was suspended or not */ bool entity_suspended; } nr_pdcp_entity_t; diff --git a/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c b/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c index b2380e3b2f..8d3ce255e3 100644 --- a/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c +++ b/openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c @@ -1056,7 +1056,6 @@ void nr_pdcp_config_set_security(ue_id_t ue_id, ciphering_algorithm = security_modeP & 0x0f; rb->set_security(rb, integrity_algorithm, (char *)kRRCint_pP, ciphering_algorithm, (char *)kRRCenc_pP); - rb->security_mode_completed = false; nr_pdcp_manager_unlock(nr_pdcp_ue_manager); } diff --git a/openair2/RRC/NR_UE/rrc_UE.c b/openair2/RRC/NR_UE/rrc_UE.c index 42540222f0..d508bdaa09 100644 --- a/openair2/RRC/NR_UE/rrc_UE.c +++ b/openair2/RRC/NR_UE/rrc_UE.c @@ -1073,7 +1073,8 @@ static void nr_rrc_ue_process_securityModeCommand(NR_UE_RRC_INST_t *ue_rrc, log_dump(NR_RRC, ue_rrc->kgnb, 32, LOG_DUMP_CHAR, "driving kRRCenc, kRRCint and kUPenc from KgNB="); if (securityMode != 0xff) { - uint8_t security_mode = ue_rrc->cipheringAlgorithm | (ue_rrc->integrityProtAlgorithm << 4); + /* for SecurityModeComplete, ciphering is not activated yet, only integrity */ + uint8_t security_mode = ue_rrc->integrityProtAlgorithm << 4; // configure lower layers to apply SRB integrity protection and ciphering for (int i = 1; i < NR_NUM_SRB; i++) { if (ue_rrc->Srb[i] == RB_ESTABLISHED) @@ -1119,6 +1120,17 @@ static void nr_rrc_ue_process_securityModeCommand(NR_UE_RRC_INST_t *ue_rrc, "securityModeCommand->criticalExtensions.present (%d) != " "NR_SecurityModeCommand__criticalExtensions_PR_securityModeCommand\n", securityModeCommand->criticalExtensions.present); + + if (securityMode != 0xff) { + /* after encoding SecurityModeComplete we activate both ciphering and integrity */ + uint8_t security_mode = ue_rrc->cipheringAlgorithm | (ue_rrc->integrityProtAlgorithm << 4); + // configure lower layers to apply SRB integrity protection and ciphering + for (int i = 1; i < NR_NUM_SRB; i++) { + if (ue_rrc->Srb[i] == RB_ESTABLISHED) + /* pass NULL to keep current keys */ + nr_pdcp_config_set_security(ue_rrc->ue_id, i, security_mode, NULL, NULL, NULL); + } + } } static void handle_meas_reporting_remove(rrcPerNB_t *rrc, int id, NR_UE_Timers_Constants_t *timers)