nrUE: simplify nr_rrc_ue_process_securityModeCommand()

The case securityModeFailure didn't seem to be implemented
properly, so I just removed it entirely.

The variable 'securityMode' did not make much sense, removed as well.
Plus for integrity, there was: securityMode |= 1 << 5 for nea1
and << 6 for nea2, which does not seem correct (I would expect << 4
and << 5 respectively), so it was properly incorrect.

'securityModeCommand->criticalExtensions.choice.securityModeCommand'
was accessed before checking that
'securityModeCommand->criticalExtensions.present'
is 'NR_SecurityModeCommand__criticalExtensions_PR_securityModeCommand',
which is wrong.

The tests 'securityMode >= NO_SECURITY_MODE' and 'securityMode != 0xff'
don't make sense/are unclear, so removed too.

So let's simplify this function, wrong in several places. And put
some AssertFatal() so that the code won't do weird things. The
AssertFatal() can be removed later and the function improved later
if needed. The case securityModeFailure can also be handled later,
but I think there is more work to do than just encoding the message
and send it to the gNB, so it's not bad to remove it for the moment.

openair2/RRC/NR_UE/rrc_UE.c

@@ -928,9 +928,15 @@ static void nr_rrc_ue_process_securityModeCommand(NR_UE_RRC_INST_t *ue_rrc,
                                                   NR_SecurityModeCommand_t *const securityModeCommand,
                                                   const uint8_t gNB_index)
 {
-  int securityMode = 0;
+  uint8_t security_mode;
+
   LOG_I(NR_RRC, "Receiving from SRB1 (DL-DCCH), Processing securityModeCommand (eNB %d)\n", gNB_index);
 
+  AssertFatal(securityModeCommand->criticalExtensions.present == NR_SecurityModeCommand__criticalExtensions_PR_securityModeCommand,
+        "securityModeCommand->criticalExtensions.present (%d) != "
+        "NR_SecurityModeCommand__criticalExtensions_PR_securityModeCommand\n",
+        securityModeCommand->criticalExtensions.present);
+
   NR_SecurityConfigSMC_t *securityConfigSMC =
       &securityModeCommand->criticalExtensions.choice.securityModeCommand->securityConfigSMC;
 
@@ -938,52 +944,34 @@ static void nr_rrc_ue_process_securityModeCommand(NR_UE_RRC_INST_t *ue_rrc,
     case NR_CipheringAlgorithm_nea0:
     case NR_CipheringAlgorithm_nea1:
     case NR_CipheringAlgorithm_nea2:
-      LOG_I(NR_RRC, "Security algorithm is set to nea%d\n", securityMode);
-      securityMode = securityConfigSMC->securityAlgorithmConfig.cipheringAlgorithm;
+      LOG_I(NR_RRC, "Security algorithm is set to nea%ld\n",
+            securityConfigSMC->securityAlgorithmConfig.cipheringAlgorithm);
       break;
     default:
-      LOG_W(NR_RRC, "Security algorithm is set to none\n");
-      securityMode = NR_CipheringAlgorithm_spare1;
-      break;
+      AssertFatal(0, "Security algorithm not known/supported\n");
   }
   ue_rrc->cipheringAlgorithm = securityConfigSMC->securityAlgorithmConfig.cipheringAlgorithm;
 
+  ue_rrc->integrityProtAlgorithm = 0;
   if (securityConfigSMC->securityAlgorithmConfig.integrityProtAlgorithm != NULL) {
     switch (*securityConfigSMC->securityAlgorithmConfig.integrityProtAlgorithm) {
+      case NR_IntegrityProtAlgorithm_nia0:
       case NR_IntegrityProtAlgorithm_nia1:
-        LOG_I(NR_RRC, "Integrity protection algorithm is set to nia1\n");
-        securityMode |= 1 << 5;
-        break;
-
       case NR_IntegrityProtAlgorithm_nia2:
-        LOG_I(NR_RRC, "Integrity protection algorithm is set to nia2\n");
-        securityMode |= 1 << 6;
+        LOG_I(NR_RRC, "Integrity protection algorithm is set to nia%ld\n", *securityConfigSMC->securityAlgorithmConfig.integrityProtAlgorithm);
         break;
-
       default:
-        LOG_I(NR_RRC, "Integrity protection algorithm is set to none\n");
-        securityMode |= 0x70;
-        break;
+        AssertFatal(0, "Integrity algorithm not known/supported\n");
     }
-
     ue_rrc->integrityProtAlgorithm = *securityConfigSMC->securityAlgorithmConfig.integrityProtAlgorithm;
   }
 
-  LOG_D(NR_RRC, "security mode is %x \n", securityMode);
   NR_UL_DCCH_Message_t ul_dcch_msg = {0};
 
-  // memset((void *)&SecurityModeCommand,0,sizeof(SecurityModeCommand_t));
   ul_dcch_msg.message.present = NR_UL_DCCH_MessageType_PR_c1;
   asn1cCalloc(ul_dcch_msg.message.choice.c1, c1);
 
-  if (securityMode >= NO_SECURITY_MODE) {
-    LOG_I(NR_RRC, "rrc_ue_process_securityModeCommand, security mode complete case \n");
-    c1->present = NR_UL_DCCH_MessageType__c1_PR_securityModeComplete;
-  } else {
-    LOG_I(NR_RRC, "rrc_ue_process_securityModeCommand, security mode failure case \n");
-    c1->present = NR_UL_DCCH_MessageType__c1_PR_securityModeFailure;
-    c1->present = NR_UL_DCCH_MessageType__c1_PR_securityModeComplete;
-  }
+  c1->present = NR_UL_DCCH_MessageType__c1_PR_securityModeComplete;
 
   uint8_t kRRCenc[16] = {0};
   uint8_t kUPenc[16] = {0};
@@ -992,67 +980,55 @@ static void nr_rrc_ue_process_securityModeCommand(NR_UE_RRC_INST_t *ue_rrc,
   nr_derive_key(RRC_ENC_ALG, ue_rrc->cipheringAlgorithm, ue_rrc->kgnb, kRRCenc);
   nr_derive_key(RRC_INT_ALG, ue_rrc->integrityProtAlgorithm, ue_rrc->kgnb, kRRCint);
 
-  log_dump(NR_RRC, ue_rrc->kgnb, 32, LOG_DUMP_CHAR, "driving kRRCenc, kRRCint and kUPenc from KgNB=");
+  log_dump(NR_RRC, ue_rrc->kgnb, 32, LOG_DUMP_CHAR, "deriving kRRCenc, kRRCint and kUPenc from KgNB=");
 
-  if (securityMode != 0xff) {
-    /* 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->perNB[gNB_index].Srb[i] == RB_ESTABLISHED)
-        nr_pdcp_config_set_security(ue_rrc->ue_id, i, security_mode, kRRCenc, kRRCint, kUPenc);
-    }
-  } else {
-    LOG_I(NR_RRC, "skipped pdcp_config_set_security() as securityMode == 0x%02x", securityMode);
+  /* for SecurityModeComplete, ciphering is not activated yet, only integrity */
+  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->perNB[gNB_index].Srb[i] == RB_ESTABLISHED)
+      nr_pdcp_config_set_security(ue_rrc->ue_id, i, security_mode, kRRCenc, kRRCint, kUPenc);
   }
 
-  if (securityModeCommand->criticalExtensions.present == NR_SecurityModeCommand__criticalExtensions_PR_securityModeCommand) {
-    asn1cCalloc(c1->choice.securityModeComplete, modeComplete);
-    modeComplete->rrc_TransactionIdentifier = securityModeCommand->rrc_TransactionIdentifier;
-    modeComplete->criticalExtensions.present = NR_SecurityModeComplete__criticalExtensions_PR_securityModeComplete;
-    asn1cCalloc(modeComplete->criticalExtensions.choice.securityModeComplete, ext);
-    ext->nonCriticalExtension = NULL;
-    LOG_I(NR_RRC,
-          "Receiving from SRB1 (DL-DCCH), encoding securityModeComplete (gNB %d), rrc_TransactionIdentifier: %ld\n",
-          gNB_index,
-          securityModeCommand->rrc_TransactionIdentifier);
-    uint8_t buffer[200];
-    asn_enc_rval_t enc_rval =
-        uper_encode_to_buffer(&asn_DEF_NR_UL_DCCH_Message, NULL, (void *)&ul_dcch_msg, buffer, sizeof(buffer));
-    AssertFatal(enc_rval.encoded > 0, "ASN1 message encoding failed (%s, %jd)!\n", enc_rval.failed_type->name, enc_rval.encoded);
-
-    if (LOG_DEBUGFLAG(DEBUG_ASN1)) {
-      xer_fprint(stdout, &asn_DEF_NR_UL_DCCH_Message, (void *)&ul_dcch_msg);
-    }
-    log_dump(NR_RRC, buffer, 16, LOG_DUMP_CHAR, "securityModeComplete payload: ");
-    LOG_D(NR_RRC, "securityModeComplete Encoded %zd bits (%zd bytes)\n", enc_rval.encoded, (enc_rval.encoded + 7) / 8);
-    ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NR_UL_DCCH_Message, &ul_dcch_msg);
+  asn1cCalloc(c1->choice.securityModeComplete, modeComplete);
+  modeComplete->rrc_TransactionIdentifier = securityModeCommand->rrc_TransactionIdentifier;
+  modeComplete->criticalExtensions.present = NR_SecurityModeComplete__criticalExtensions_PR_securityModeComplete;
+  asn1cCalloc(modeComplete->criticalExtensions.choice.securityModeComplete, ext);
+  ext->nonCriticalExtension = NULL;
+  LOG_I(NR_RRC,
+        "Receiving from SRB1 (DL-DCCH), encoding securityModeComplete (gNB %d), rrc_TransactionIdentifier: %ld\n",
+        gNB_index,
+        securityModeCommand->rrc_TransactionIdentifier);
+  uint8_t buffer[200];
+  asn_enc_rval_t enc_rval =
+      uper_encode_to_buffer(&asn_DEF_NR_UL_DCCH_Message, NULL, (void *)&ul_dcch_msg, buffer, sizeof(buffer));
+  AssertFatal(enc_rval.encoded > 0, "ASN1 message encoding failed (%s, %jd)!\n", enc_rval.failed_type->name, enc_rval.encoded);
 
-    for (int i = 0; i < (enc_rval.encoded + 7) / 8; i++) {
-      LOG_T(NR_RRC, "%02x.", buffer[i]);
-    }
-    LOG_T(NR_RRC, "\n");
-
-    // TODO the SecurityModeCommand message needs to pass the integrity protection check
-    //  for the UE to declare AS security to be activated
-    ue_rrc->as_security_activated = true;
-    int srb_id = 1; // SecurityModeComplete in SRB1
-    nr_pdcp_data_req_srb(ue_rrc->ue_id, srb_id, 0, (enc_rval.encoded + 7) / 8, buffer, deliver_pdu_srb_rlc, NULL);
-  } else
-    LOG_W(NR_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->perNB[gNB_index].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);
-    }
+  if (LOG_DEBUGFLAG(DEBUG_ASN1)) {
+    xer_fprint(stdout, &asn_DEF_NR_UL_DCCH_Message, (void *)&ul_dcch_msg);
+  }
+  log_dump(NR_RRC, buffer, 16, LOG_DUMP_CHAR, "securityModeComplete payload: ");
+  LOG_D(NR_RRC, "securityModeComplete Encoded %zd bits (%zd bytes)\n", enc_rval.encoded, (enc_rval.encoded + 7) / 8);
+  ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NR_UL_DCCH_Message, &ul_dcch_msg);
+
+  for (int i = 0; i < (enc_rval.encoded + 7) / 8; i++) {
+    LOG_T(NR_RRC, "%02x.", buffer[i]);
+  }
+  LOG_T(NR_RRC, "\n");
+
+  // TODO the SecurityModeCommand message needs to pass the integrity protection check
+  //  for the UE to declare AS security to be activated
+  ue_rrc->as_security_activated = true;
+  int srb_id = 1; // SecurityModeComplete in SRB1
+  nr_pdcp_data_req_srb(ue_rrc->ue_id, srb_id, 0, (enc_rval.encoded + 7) / 8, buffer, deliver_pdu_srb_rlc, NULL);
+
+  /* after encoding SecurityModeComplete we activate both ciphering and integrity */
+  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->perNB[gNB_index].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);
   }
 }