Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Authentication Failure - ngKSI already in use #307

Closed
wants to merge 4 commits into from
Closed

Authentication Failure - ngKSI already in use #307

wants to merge 4 commits into from

Conversation

ashithacdac
Copy link
Contributor

Issue:
Authentication failure with cause ngKSI already in use

Troubleshoot:
Identified the root cause: the core includes the same ngKSI value from the Registration Request
in the Authentication request

Fix:
Updated the ngKSI value after the 5G AKA procedure

Supporting Data:
Authentication_fail_ngksi_already_in_use.zip

thakurajayL
thakurajayL reviewed Oct 1, 2024
View reviewed changes
gmm/handler.go Outdated
@@ -1558,6 +1558,16 @@ func AuthenticationProcedure(ue *context.AmfUe, accessType models.AccessType) (b
ue.AuthenticationCtx = response
ue.ABBA = []uint8{0x00, 0x00} // set ABBA value as described at TS 33.501 Annex A.7.1

if ue.NgKsi.Tsc == models.ScType_NATIVE && ue.NgKsi.Ksi != 7 {
// As per the Specification 24.501 - 5.4.1.3.2 Authentication initiation by the network
if ue.NgKsi.Ksi < 6 { // ksi is range from 0 to 6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ue.NgKsi.Ksi = (ue.NgKsi.Ksi +1) % 7

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorporated the suggested changes.

thakurajayL
thakurajayL reviewed Oct 7, 2024
View reviewed changes
gmm/handler.go Outdated
@@ -1558,6 +1558,12 @@ func AuthenticationProcedure(ue *context.AmfUe, accessType models.AccessType) (b
ue.AuthenticationCtx = response
ue.ABBA = []uint8{0x00, 0x00} // set ABBA value as described at TS 33.501 Annex A.7.1

if ue.NgKsi.Tsc == models.ScType_NATIVE && ue.NgKsi.Ksi != 7 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ue.NgKsi.Tsc == models.ScType_NATIVE && ue.NgKsi.Ksi != 7 {
if ue.NgKsi.Tsc == models.ScType_NATIVE {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ue.NgKsi.Ksi will never be 7.

Copy link
Contributor

@gab-arrobo gab-arrobo Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Ajay's comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate clarification on the statement regarding Key lifetimes in the specification 33.501 section 6.2.3.3 which states 'In case the UE does not have a valid KAMF, an ngKSI with value "111" shall be sent by the UE to the network, which can initiate (re)authentication procedure to get a new KAMF based on a successful primary authentication.'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate clarification on the statement regarding Key lifetimes in the specification 33.501 section 6.2.3.3 which states 'In case the UE does not have a valid KAMF, an ngKSI with value "111" shall be sent by the UE to the network, which can initiate (re)authentication procedure to get a new KAMF based on a successful primary authentication.'

@thakurajayL what are yout thoughts about this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the section you mentioned. It just says that if UE sends 111 then it just means UE does not have Kamf and its requesting new keys. In my opinion its perfectly fine to remove the != 7 check in the above code and let the keys rotate from 0 to 6.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorporated the suggested changes.

gab-arrobo
gab-arrobo requested changes Oct 7, 2024
View reviewed changes
gmm/handler.go Outdated
// As per the Specification 24.501 - 5.4.1.3.2 Authentication initiation by the network
ue.NgKsi.Ksi = (ue.NgKsi.Ksi + 1) % 7
}
ue.GmmLog.Info("ngKSI after 5G-AKA: ", ue.NgKsi.Ksi)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ue.GmmLog.Info("ngKSI after 5G-AKA: ", ue.NgKsi.Ksi)
ue.GmmLog.Infoln("ngKSI after 5G-AKA:", ue.NgKsi.Ksi)

gmm/handler.go Outdated
@@ -1558,6 +1558,12 @@ func AuthenticationProcedure(ue *context.AmfUe, accessType models.AccessType) (b
ue.AuthenticationCtx = response
ue.ABBA = []uint8{0x00, 0x00} // set ABBA value as described at TS 33.501 Annex A.7.1

if ue.NgKsi.Tsc == models.ScType_NATIVE && ue.NgKsi.Ksi != 7 {
Copy link
Contributor

@gab-arrobo gab-arrobo Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Ajay's comment

gab-arrobo
gab-arrobo reviewed Oct 7, 2024
View reviewed changes
gmm/handler.go Outdated
@@ -1558,6 +1558,12 @@ func AuthenticationProcedure(ue *context.AmfUe, accessType models.AccessType) (b
ue.AuthenticationCtx = response
ue.ABBA = []uint8{0x00, 0x00} // set ABBA value as described at TS 33.501 Annex A.7.1

if ue.NgKsi.Tsc == models.ScType_NATIVE && ue.NgKsi.Ksi != 7 {
// As per the Specification 24.501 - 5.4.1.3.2 Authentication initiation by the network
Copy link
Contributor

@gab-arrobo gab-arrobo Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, should not it be 33.501 the correct document to reference?

Suggested change
// As per the Specification 24.501 - 5.4.1.3.2 Authentication initiation by the network
// As per the Specification 33.501 - 6.2.3.2 Key identification

@thakurajayL thakurajayL self-assigned this Oct 19, 2024
@gab-arrobo
Copy link
Contributor

@ashithacdac any update about this?

@ashithacdac
Copy link
Contributor Author

@gab-arrobo Incorporated the suggested changes.

gab-arrobo
gab-arrobo requested changes Nov 4, 2024
View reviewed changes
Copy link
Contributor

@gab-arrobo gab-arrobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please address my last comment and then rebase your PR. Thanks!

onf-bot and others added 4 commits November 5, 2024 10:36
@onf-bot @gab-arrobo @ashithacdac
Update version (#289)
f930758 
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: gab-arrobo <[email protected]>
Signed-off-by: AshithaCDAC <[email protected]>
@ashithacdac
authentication fail due to ngksi already in use fix
cc13287 
Signed-off-by: AshithaCDAC <[email protected]>
@ashithacdac
modified ngksi already in use fix
0832fa4 
Signed-off-by: AshithaCDAC <[email protected]>
@ashithacdac
modified ngksi already in use fix
5d338b5 
Signed-off-by: AshithaCDAC <[email protected]>
@gab-arrobo
Copy link
Contributor

@ashithacdac, FYI, it looks like your PR was not properly rebased

@ashithacdac ashithacdac closed this by deleting the head repository Nov 7, 2024
@thakurajayL thakurajayL mentioned this pull request Nov 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thakurajayL thakurajayL thakurajayL left review comments

@gab-arrobo gab-arrobo gab-arrobo requested changes

Assignees

@thakurajayL thakurajayL

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashithacdac @gab-arrobo @thakurajayL @onf-bot