fix: prevent AF crash when requestedQoS is missing

[ErikGaida]

Summary

NetworkAssistanceSession.requestedQoS has cardinality [0..1] and is therefore optional.

The code previously accessed subfields of requestedQoS without checking whether the object was present. When requestedQoS was omitted, this led to a NULL pointer dereference and caused the AF to crash.

Closes 2 Bugs Reported in #190

[ErikGaida]

Hi @jordijoangimenez,
Thank you for the feedback, please try a again.

Giving you new AF logs:

06/18 14:59:47.918: [msaf] INFO: policy template id: 597857ac-6b26-41f1-a527-7343ed4a5a6e (../src/5gmsaf/msaf-m1-sm.c:667)
06/18 14:59:47.918: [msaf] INFO: msaf_policy_template->policy_template->state: 1 (../src/5gmsaf/local.c:55)
06/18 14:59:47.918: [msaf] INFO: msaf_policy_template->policy_template->state: 1 (../src/5gmsaf/local.c:64)
06/18 14:59:47.918: [msaf] INFO: msaf_policy_template->policy_template->state: 3 (../src/5gmsaf/local.c:55)
06/18 14:59:47.918: [msaf] INFO: msaf_policy_template->policy_template->state: 3 (../src/5gmsaf/local.c:64)
06/18 15:01:32.927: [msaf] INFO: 10.45.0.5: dst_ip (../src/5gmsaf/network-assistance-session.c:319)
06/18 15:01:32.927: [core] ERROR: Unknown Event[8] (../subprojects/open5gs/lib/proto/event.c:77)
06/18 15:01:32.927: [sbi] WARNING: Try to discover [nbsf-management] (../subprojects/open5gs/lib/sbi/path.c:348)
06/18 15:01:32.929: [sbi] INFO: [54c47d6e-67be-41f1-b985-01072c01575a] (NF-discover) NF registered (../subprojects/open5gs/lib/sbi/nnrf-handler.c:833)
06/18 15:01:32.929: [sbi] INFO: [54c47d6e-67be-41f1-b985-01072c01575a] (NF-discover) NF Profile updated (../subprojects/open5gs/lib/sbi/nnrf-handler.c:860)
06/18 15:01:32.931: [bsf-service-consumer] ERROR: Unable to find PCF binding for 10.45.0.5 (../subprojects/rt-5gc-service-consumers/lib/bsf-service-consumer/nbsf-process.c:131)
06/18 15:01:32.931: [msaf] ERROR: Unable to retrieve PCF Binding. (../src/5gmsaf/network-assistance-session.c:856)
06/18 15:01:32.931: [msaf] ERROR: Unable to retrieve PCF Binding. (../src/5gmsaf/network-assistance-session.c:863)
06/18 15:01:32.931: [bsf-service-consumer] ERROR: BSF client callback failed (../subprojects/rt-5gc-service-consumers/lib/bsf-service-consumer/bsf-client-sess.c:137)
06/18 15:02:02.930: [sbi] INFO: [54c47d6e-67be-41f1-b985-01072c01575a] NF expired (../subprojects/open5gs/lib/sbi/nf-sm.c:318)
fivegmag@fivegmag:~/5GMS/rt-5gms-examples/5gms-docker-setup/recipe1_with_5GC$

[rjb1000]

Just a few suggestions for improvements to log messages.

[ErikGaida]

@rjb1000 Thank you for the feedback.

[davidjwbbc]

Looks good to me.

Had to convince myself that it was ok to send a media component without the bitrate, but after following the logic and consulting the specs it looks like this is ok.

I also noticed that update_media_component function fixes the MediaType of the media component as VIDEO even though there may be a different type provided in the NetworkAssistanceSession. I vaguely remember that the Open5GS PCF didn't work too well with other values, and I can't remember whether it required this field to have one of a specific set of values, even though it's an optional field. May be worth retesting that as it may have changed in the more up to date Open5GS implementations.

I also note that the code doesn't use the policyTemplateId field, I suspect its something we hadn't got around to implementing as we ran into issues testing with Open5GS (no traffic policing) and suspended development. The code probably ought to get the minimum required bit rates from the policy template if the requiredQoS field isn't set and the policyTemplateId is set (or maybe also use a default policy template if no policyTemplateId is set). It also ought to restrict the values requested in requestedQoS to the limits of the policy template. Again, this was probably something we were going to develop had we got chance to test this in a working QoS environment (i.e. traffic policing implemented) and continued development.

[ErikGaida]

I also noticed that update_media_component function fixes the MediaType of the media component as VIDEO even though there may be a different type provided in the NetworkAssistanceSession. I vaguely remember that the Open5GS PCF didn't work too well with other values, and I can't remember whether it required this field to have one of a specific set of values, even though it's an optional field. May be worth retesting that as it may have changed in the more up to date Open5GS implementations.

yes, in rt-5gms-application-function/subprojects/open5gs/src/pcf/npcf-handler.c there are 3 media typ allowed:

1. OpenAPI_media_type_AUDIO
2. OpenAPI_media_type_VIDEO
3. OpenAPI_media_type_CONTROL

everything else get rejected:

 if (media_component->media_type == OpenAPI_media_type_NULL) {
            strerror = ogs_msprintf("[%s:%d] Media-Type is Required",
                    pcf_ue->supi, sess->psi);
            status = OGS_SBI_HTTP_STATUS_BAD_REQUEST;
            goto cleanup;
        }

        switch(media_component->media_type) {
        case OpenAPI_media_type_AUDIO:
            qos_index = OGS_QOS_INDEX_1;
            break;
        case OpenAPI_media_type_VIDEO:
            qos_index = OGS_QOS_INDEX_2;
            break;
        case OpenAPI_media_type_CONTROL:
            qos_index = OGS_QOS_INDEX_5;
            break;
        default:
            strerror = ogs_msprintf("[%s:%d] Unknown Media-Type [%d]",
                    pcf_ue->supi, sess->psi, media_component->media_type);
            status = OGS_SBI_HTTP_STATUS_BAD_REQUEST;
            goto cleanup;
        }

spec:
TS 26.512

NetworkAssistanceSession:
... 
mediaType:
$ref: 'TS29514_Npcf_PolicyAuthorization.yaml#/components/schemas/MediaType'

TS 29.514

    MediaType:
      description: Indicates the media type of a media component.
      anyOf:
        - type: string
          enum:
            - AUDIO
            - VIDEO
            - DATA
            - APPLICATION
            - CONTROL
            - TEXT
            - MESSAGE
            - OTHER
        - type: string

@davidjwbbc As i understand only audio, video and control are implemented in Open5GS in the Branch we are using.