e1: no need to pass security information since it does not change

In Bearer Context Modification Request, the security information field is needed only if we change the PDCP algorithms/keys. Since we don't, no need to pass it. Let's keep the old code, maybe the current understanding of the specifications is wrong.
e1: change security settings only if parameters present
2026-07-13 12:40:28 +00:00 · 2025-01-22 16:45:14 +01:00 · 2025-01-22 16:42:59 +01:00 · 2025-01-22 16:42:59 +01:00 · 2025-01-22 16:42:48 +01:00
5 changed files with 60 additions and 8 deletions
--- a/openair2/COMMON/e1ap_messages_types.h
+++ b/openair2/COMMON/e1ap_messages_types.h
@@ -266,6 +266,7 @@ typedef struct pdu_session_to_setup_s {
 typedef struct e1ap_bearer_setup_req_s {
  uint32_t gNB_cu_cp_ue_id;
  uint32_t gNB_cu_up_ue_id; // Bearer Context Modification Request only
+  bool has_security_information;
  uint64_t cipheringAlgorithm;
  uint64_t integrityProtectionAlgorithm;
  char encryptionKey[E1AP_SECURITY_KEY_SIZE];
--- a/openair2/E1AP/e1ap.c
+++ b/openair2/E1AP/e1ap.c
@@ -653,6 +653,21 @@ static int fill_BEARER_CONTEXT_MODIFICATION_REQUEST(e1ap_bearer_setup_req_t *con
  ieC2->criticality                = E1AP_Criticality_reject;
  ieC2->value.present              = E1AP_BearerContextModificationRequestIEs__value_PR_GNB_CU_UP_UE_E1AP_ID;
  ieC2->value.choice.GNB_CU_UP_UE_E1AP_ID = bearerCxt->gNB_cu_up_ue_id;
+   /* optional */
+  /* Security Information */
+  if (bearerCxt->has_security_information) {
+    asn1cSequenceAdd(out->protocolIEs.list, E1AP_BearerContextModificationRequestIEs_t, ie);
+    ie->id = E1AP_ProtocolIE_ID_id_SecurityInformation;
+    ie->criticality = E1AP_Criticality_reject;
+    ie->value.present = E1AP_BearerContextModificationRequestIEs__value_PR_SecurityInformation;
+    E1AP_SecurityAlgorithm_t *securityAlgorithm = &ie->value.choice.SecurityInformation.securityAlgorithm;
+    E1AP_UPSecuritykey_t *uPSecuritykey = &ie->value.choice.SecurityInformation.uPSecuritykey;
+    securityAlgorithm->cipheringAlgorithm = bearerCxt->cipheringAlgorithm;
+    OCTET_STRING_fromBuf(&uPSecuritykey->encryptionKey, bearerCxt->encryptionKey, E1AP_SECURITY_KEY_SIZE);
+    asn1cCallocOne(securityAlgorithm->integrityProtectionAlgorithm, bearerCxt->integrityProtectionAlgorithm);
+    asn1cCalloc(uPSecuritykey->integrityProtectionKey, protKey);
+    OCTET_STRING_fromBuf(protKey, bearerCxt->integrityProtectionKey, E1AP_SECURITY_KEY_SIZE);
+  }
  /* optional */
  /* c3. E1AP_ProtocolIE_ID_id_System_BearerContextModificationRequest */
  asn1cSequenceAdd(out->protocolIEs.list, E1AP_BearerContextModificationRequestIEs_t, ieC3);
@@ -792,6 +807,8 @@ static void extract_BEARER_CONTEXT_MODIFICATION_REQUEST(const E1AP_E1AP_PDU_t *p

  LOG_D(E1AP, "Bearer context setup number of IEs %d\n", in->protocolIEs.list.count);

+  bearerCxt->has_security_information = false;
+
  for (int i = 0; i < in->protocolIEs.list.count; i++) {
    ie = in->protocolIEs.list.array[i];

@@ -808,6 +825,26 @@ static void extract_BEARER_CONTEXT_MODIFICATION_REQUEST(const E1AP_E1AP_PDU_t *p
        bearerCxt->gNB_cu_up_ue_id = ie->value.choice.GNB_CU_UP_UE_E1AP_ID;
        break;

+      case E1AP_ProtocolIE_ID_id_SecurityInformation: {
+        DevAssert(ie->criticality == E1AP_Criticality_reject);
+        DevAssert(ie->value.present == E1AP_BearerContextModificationRequestIEs__value_PR_SecurityInformation);
+        E1AP_SecurityInformation_t *sec = &ie->value.choice.SecurityInformation;
+        DevAssert(sec->uPSecuritykey.encryptionKey.size == E1AP_SECURITY_KEY_SIZE);
+        DevAssert(sec->uPSecuritykey.integrityProtectionKey == NULL
+                  || sec->uPSecuritykey.integrityProtectionKey->size == E1AP_SECURITY_KEY_SIZE);
+        bearerCxt->has_security_information = true;
+        bearerCxt->cipheringAlgorithm = sec->securityAlgorithm.cipheringAlgorithm;
+        bearerCxt->integrityProtectionAlgorithm = sec->securityAlgorithm.integrityProtectionAlgorithm == NULL
+                                                ? 0
+                                                : *sec->securityAlgorithm.integrityProtectionAlgorithm;
+        memcpy(bearerCxt->encryptionKey, sec->uPSecuritykey.encryptionKey.buf, E1AP_SECURITY_KEY_SIZE);
+        if (sec->securityAlgorithm.integrityProtectionAlgorithm == NULL)
+          memset(bearerCxt->integrityProtectionKey, 0, E1AP_SECURITY_KEY_SIZE);
+        else
+          memcpy(bearerCxt->integrityProtectionKey, sec->uPSecuritykey.integrityProtectionKey->buf, E1AP_SECURITY_KEY_SIZE);
+        break;
+      }
+
      case E1AP_ProtocolIE_ID_id_System_BearerContextModificationRequest:
        DevAssert(ie->criticality == E1AP_Criticality_reject);
        DevAssert(ie->value.present == E1AP_BearerContextModificationRequestIEs__value_PR_System_BearerContextModificationRequest);
--- a/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
+++ b/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
@@ -274,10 +274,16 @@ void e1_bearer_context_modif(const e1ap_bearer_mod_req_t *req)

      if (to_modif->pdcp_config.pDCP_Reestablishment) {
        nr_pdcp_entity_security_keys_and_algos_t security_parameters;
-        security_parameters.ciphering_algorithm = req->cipheringAlgorithm;
-        security_parameters.integrity_algorithm = req->integrityProtectionAlgorithm;
-        memcpy(security_parameters.ciphering_key, req->encryptionKey, NR_K_KEY_SIZE);
-        memcpy(security_parameters.integrity_key, req->integrityProtectionKey, NR_K_KEY_SIZE);
+        if (req->has_security_information) {
+          security_parameters.ciphering_algorithm = req->cipheringAlgorithm;
+          security_parameters.integrity_algorithm = req->integrityProtectionAlgorithm;
+          memcpy(security_parameters.ciphering_key, req->encryptionKey, NR_K_KEY_SIZE);
+          memcpy(security_parameters.integrity_key, req->integrityProtectionKey, NR_K_KEY_SIZE);
+        } else {
+          /* don't change security settings if not present in the Bearer Context Modification Request */
+          security_parameters.ciphering_algorithm = -1;
+          security_parameters.integrity_algorithm = -1;
+        }
        nr_pdcp_reestablishment(req->gNB_cu_up_ue_id,
                                to_modif->id,
                                false,
--- a/openair2/RRC/NR/rrc_gNB.c
+++ b/openair2/RRC/NR/rrc_gNB.c
@@ -802,10 +802,18 @@ static void cuup_notify_reestablishment(gNB_RRC_INST *rrc, gNB_RRC_UE_t *ue_p)
    pdu_e1->numDRB2Modify += 1;
  }

+  req.has_security_information = false;
+#if 0
+  /* According to current understanding of E1 specifications, it is not needed
+   * to send security information because this does not change.
+   * But let's keep the code here in case it's needed.
+   */
+  req.has_security_information = true;
  req.cipheringAlgorithm = rrc->security.do_drb_ciphering ? ue_p->ciphering_algorithm : 0;
  req.integrityProtectionAlgorithm = rrc->security.do_drb_integrity ? ue_p->integrity_algorithm : 0;
  nr_derive_key(UP_ENC_ALG, req.cipheringAlgorithm, ue_p->kgnb, (uint8_t *)req.encryptionKey);
  nr_derive_key(UP_INT_ALG, req.integrityProtectionAlgorithm, ue_p->kgnb, (uint8_t *)req.integrityProtectionKey);
+#endif

  /* Send E1 Bearer Context Modification Request (3GPP TS 38.463) */
  sctp_assoc_t assoc_id = get_existing_cuup_for_ue(rrc, ue_p);
--- a/openair2/RRC/NR/rrc_gNB_NGAP.c
+++ b/openair2/RRC/NR/rrc_gNB_NGAP.c
@@ -337,10 +337,10 @@ void trigger_bearer_setup(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, int n, pdusession
    session->nssai = sessions[i].nssai;
    decodePDUSessionResourceSetup(session);
    bearer_req.gNB_cu_cp_ue_id = UE->rrc_ue_id;
-    bearer_req.cipheringAlgorithm = UE->ciphering_algorithm;
-    bearer_req.integrityProtectionAlgorithm = UE->integrity_algorithm;
-    nr_derive_key(UP_ENC_ALG, UE->ciphering_algorithm, UE->kgnb, (uint8_t *)bearer_req.encryptionKey);
-    nr_derive_key(UP_INT_ALG, UE->integrity_algorithm, UE->kgnb, (uint8_t *)bearer_req.integrityProtectionKey);
+    bearer_req.cipheringAlgorithm = rrc->security.do_drb_ciphering ? UE->ciphering_algorithm : 0;
+    bearer_req.integrityProtectionAlgorithm = rrc->security.do_drb_integrity ? UE->integrity_algorithm : 0;
+    nr_derive_key(UP_ENC_ALG, bearer_req.cipheringAlgorithm, UE->kgnb, (uint8_t *)bearer_req.encryptionKey);
+    nr_derive_key(UP_INT_ALG, bearer_req.integrityProtectionAlgorithm, UE->kgnb, (uint8_t *)bearer_req.integrityProtectionKey);
    bearer_req.ueDlAggMaxBitRate = ueAggMaxBitRateDownlink;
    pdu_session_to_setup_t *pdu = bearer_req.pduSession + bearer_req.numPDUSessions;
    bearer_req.numPDUSessions++;
Author	SHA1	Message	Date
Cedric Roux	25af8f49b2	e1: no need to pass security information since it does not change In Bearer Context Modification Request, the security information field is needed only if we change the PDCP algorithms/keys. Since we don't, no need to pass it. Let's keep the old code, maybe the current understanding of the specifications is wrong.	2025-01-22 16:45:14 +01:00
Cedric Roux	181762978c	e1: change security settings only if parameters present If the Bearer Context Modification Request does not contain security information, the PDCP entity shall keep the currently configured security settings. Passing -1 for ciphering_algorithm and integrity_algorithm is the way to implement this logic.	2025-01-22 16:42:59 +01:00
Cedric Roux	e84adc6057	e1: security information is optional in bearer modification Introduce a boolean to deal with presence/absence of this information element in Bearer Context Modification Request and add encoding/decoding in the code. Note that for decodding the integrity settings may be absent, in which case we consider NIA0 to be used (no integrity protection). To be refined if needed. And for encoding, we always encode integrity settings, even if it's NIA0. May also be refined if needed.	2025-01-22 16:42:59 +01:00
Cedric Roux	76f7b29277	bugfix: use correct algorithm and derive keys accordingly This was not a big problem, but we were advertising wrong algorithm and deriving keys using this wrong algorithm in case DRB ciphering and/or integrity was not active. (Say SRBs are configured with NIA2 but DRBs are configured without integrity, we would advertise NIA2 and send the integrity key derived for NIA2, instead of NIA0 for both.) It was not a problem because in the PDU Session Resource To Setup item, there is "securityIndication" which we use to effectively activate/deactivate the ciphering and/or integrity. But it did not look clean to see a SecurityInformation with incorrect data when inspecting the message in wireshark. So let's use the correct values. We could also not include the SecurityInformation if both ciphering and integrity are not used, and only include ciphering if integrity is not used (because integrity settings are optional). But then if only integrity is used, we still need to include ciphering, setting algorithm to NEA0 (no ciphering), because the ciphering settings are always included. This logic is too complex, let's use the simple one to always include SecurityInformation with NEA0 and/or NIA0 if ciphering and/or integrity is not activated.	2025-01-22 16:42:48 +01:00