-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/integrity check #109
Conversation
@@ -215,6 +216,18 @@ func UEContextTransferRequest( | |||
} | |||
if transferReason == models.TransferReason_INIT_REG || transferReason == models.TransferReason_MOBI_REG { | |||
var buf bytes.Buffer | |||
if err = binary.Write(&buf, binary.BigEndian, ue.SecurityHeader.ProtocolDiscriminator); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to simply store the original regReq message (the byte array before unmarshalling) and fill it into req.BinaryDataN1Message?
_, integrityProtected, err := nas_security.Decode(ue, UeContextTransferReqData.AccessType, | ||
ueContextTransferRequest.BinaryDataN1Message, true) | ||
if err != nil { | ||
ue.NASLog.Errorln(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the procedure keep going if err is nil? (Should function return here when there's an error?)
internal/sbi/producer/ue_context.go
Outdated
problemDetails := &models.ProblemDetails{ | ||
Status: http.StatusForbidden, | ||
Cause: "INTEGRITY_CHECK_FAIL", | ||
InvalidParams: []models.InvalidParam{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is used to indicate there is an invalid param called "reason" for this api request, which in my opnion is not the case here.
internal/sbi/producer/ue_context.go
Outdated
@@ -336,11 +376,64 @@ func UEContextTransferProcedure(ueContextID string, ueContextTransferRequest mod | |||
return ueContextTransferResponse, nil | |||
} | |||
|
|||
func buildUEContextModel(ue *context.AmfUe) *models.UeContext { | |||
func buildUEContextModel(ue *context.AmfUe, integrityProtected bool) *models.UeContext { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to TS 29.518 5.2.2.2.1.1
2a. On success:
- if the reason attribute is "INIT_REG" and integrity check is successful, the (source) AMF shall respond with
the status code "200 OK". The payload of the response shall be an object of "UeContextTransferRspData"
data type, containing:
case a) the representation of the requested UE Context without PDU Session Contexts; or
case b) the representation of the requested UE Context only containing the "supi" attribute, if the UE is
registered in a different access type in the (source) AMF and the source AMF determines based on the
PLMN ID of the (target) AMF that there is no possibility for relocating the N2 interface to the (target)
AMF.- If the reason attribute is "MOBI_REG" and integrity check is successful, the (source) AMF shall respond
with the status code "200 OK". The payload of the response shall be an object of
"UeContextTransferRspData" data type, containing the representation of the complete UE Context including
available PDU Session Contexts.
and 5.2.2.2.1.2
- Same as step 2a of figure 5.2.2.2.1.1-1, with following differences:
- The (source) AMF shall skip integrity check and shall respond with the status code "200 OK "with the UE
Context excluding SeafData and including available PDU Session Contexts
I think transfer reason is more suitable than the integrityProtected
to control the items to be filled in here. If you pass the transfer reason instead of the integrityProtected
in, you can even move the sessionContextList
parts in case models.TransferReason_MOBI_REG_UE_VALIDATED
into this api.
internal/sbi/producer/ue_context.go
Outdated
_, integrityProtected, err := nas_security.Decode(ue, UeContextTransferReqData.AccessType, | ||
ueContextTransferRequest.BinaryDataN1Message, false) | ||
if err != nil { | ||
ue.NASLog.Errorln(err) | ||
} | ||
if integrityProtected { | ||
ueContextTransferRspData.UeContext = buildUEContextModel(ue, integrityProtected) | ||
} else { | ||
problemDetails := &models.ProblemDetails{ | ||
Status: http.StatusForbidden, | ||
Cause: "INTEGRITY_CHECK_FAIL", | ||
InvalidParams: []models.InvalidParam{ | ||
{ | ||
Param: "reason", | ||
}, | ||
}, | ||
} | ||
return nil, problemDetails | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this part a common api since it duplicates with that of case models.TransferReason_MOBI_REG_UE_VALIDATED
.
For
and step 9a.
In my opnion, currently our implementation can't follow the spec requests to perform the following proceduew
|
I think this may take some efforts to implement, so it doesn't need to be included in this PR. Please add this as a TODO. |
internal/sbi/producer/ue_context.go
Outdated
ueContext := new(models.UeContext) | ||
ueContext.Supi = ue.Supi | ||
ueContext.SupiUnauthInd = ue.UnauthenticatedSupi | ||
|
||
if integrityProtected { | ||
var mmcontext models.MmContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmContext
internal/gmm/handler.go
Outdated
@@ -561,7 +562,7 @@ func HandleRegistrationRequest(ue *context.AmfUe, anType models.AccessType, proc | |||
// if failed, give up to retrieve the old context and start a new authentication procedure. | |||
ue.ServingAmfChanged = false | |||
context.GetSelf().AllocateGutiToUe(ue) // refresh 5G-GUTI | |||
ue.SecurityContextAvailable = false // need to start authentication procedure later | |||
// ue.SecurityContextAvailable = false // need to start authentication procedure later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unused code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamelisahi missing this comment
internal/gmm/handler.go
Outdated
return nil | ||
} | ||
|
||
func IdentityVerification(ue *context.AmfUe) bool { | ||
return ue.Supi != "" || len(ue.Suci) != 0 | ||
// return len(ue.Suci) != 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unused code
@@ -190,6 +194,17 @@ func ReleaseUEContextProcedure(ueContextID string, ueContextRelease models.UeCon | |||
|
|||
return nil | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix lint error
internal/sbi/producer/ue_context.go
Outdated
@@ -190,6 +194,17 @@ func ReleaseUEContextProcedure(ueContextID string, ueContextRelease models.UeCon | |||
|
|||
return nil | |||
} | |||
func HandleMobiRegUe(ue *context.AmfUe, ueContextTransferRspData *models.UeContextTransferRspData, ueContextTransferResponse *models.UeContextTransferResponse) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix lint error
} | ||
ueContext.MmContextList = append(ueContext.MmContextList, mmContext) | ||
} | ||
if Reason == models.TransferReason_MOBI_REG_UE_VALIDATED || Reason == models.TransferReason_MOBI_REG { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix lint error
internal/sbi/producer/ue_context.go
Outdated
*sessionContextList = append(*sessionContextList, pduSessionContext) | ||
return true | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix lint error
* check integrity of the registration request included in ueContextTransferRequest
* copy security context to new amf
* handle INTEGRITY_CHECK_FAIL Modify IdentityVerification Modify HandleRegistrationRequest: * assign value to ue.PlmnId in case "nasMessage.MobileIdentity5GSType5gGuti"
* since the MACs vary when the integrity check algorithms differ, the identical comparison should only be applied to nas.message.GmmMessage
* change input parameter from integrityProtected to models.TransferReason
* add TODO and delete comments
7f2baae
to
1a753f3
Compare
Enhance the integrity check in the HandleUEContextTransferRequest function and incorporate the UE security context into the response.