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.

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

Write supiOrSuci to log on re-sync failure #13

Merged
merged 3 commits into from
Jul 5, 2022

Conversation

thevirg
Copy link
Contributor

@thevirg thevirg commented Jul 3, 2022

Hello, I am Virgil, a security researcher from the WSPR (Wolfpack Security and Privacy Research) lab at North Carolina State University.

As part of an ongoing 5G security project, we have done some error handling analysis of free5GC. One finding involves the writing of supi to log in the event of a re-sync error. This occurs at internal/sbi/producer/generate_auth_data.go Line 297. supi is considered a sensitive identifier, as per TS 33.501 Section 5.2.5.

This PR changes the supi log write to supiOrSuci and adds a write of the UDM public key for ease of configuration identification.

The change to writing supiOrSuci ensures the encrypted SUCI is written to log rather than SUPI, if it is present.

The addition of the UDM public key allows for easier suciProfile configuration YAML identification, in the event of multiple UDMs. The public key is in the SUCI, but this prevents an auditor from having to parse the SUCI to identify the neccessary keys for decryption.

The writing of SUCI instead of SUPI (if present) still allows for the debugging of the re-sync error (as the operator should have access to the private key), but prevents the SUPI from being written to log.

Will open submodule hash update PR in main repo if accepted.

free5gc-org
free5gc-org previously approved these changes Jul 4, 2022
// Check if suci
if suciPart := strings.Split(supiOrSuci, "-"); suciPart[0] == "suci" {
// Get SuciProfile index and write public key
keyIndex, err := strconv.Atoi(suciPart[suci.HNPublicKeyIDPlace])
Copy link
Contributor

Choose a reason for hiding this comment

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

need to rename err (ex: err1) to prevent golangci-linter fail: shadow: declaration of "err" shadows declaration at line 158 (govet)

@free5gc-org free5gc-org dismissed their stale review July 4, 2022 04:18

need to fix lint error

@thevirg
Copy link
Contributor Author

thevirg commented Jul 4, 2022

Pushed commit w/ rename. Sorry about that!

@free5gc-org free5gc-org merged commit c83b3aa into free5gc:main Jul 5, 2022
@Wa3c
Copy link

Wa3c commented Jan 9, 2024

مرحبًا، أنا فيرجيل، باحث أمني من مختبر WSPR (أبحاث الأمن والخصوصية Wolfpack) في جامعة ولاية كارولينا الشمالية.

كجزء من مشروع أمان 5G المستمر، قمنا ببعض التحليلات لمعالجة الأخطاء في free5GC. تتضمن إحدى النتائج كتابة supiتسجيل الدخول في حالة حدوث خطأ في إعادة المزامنة. يحدث هذا في internal/sbi/producer/generate_auth_data.goالسطر 297. supiويعتبر معرفًا حساسًا، وفقًا للمواصفة TS 33.501 القسم 5.2.5.

يغير هذا العلاقات العامة supiكتابة السجل supiOrSuciويضيف كتابة للمفتاح العام UDM لتسهيل التعرف على التكوين.

يضمن التغيير في الكتابة supiOrSuciكتابة SUCI المشفرة للتسجيل بدلاً من SUPI، إذا كان موجودًا.

تسمح إضافة المفتاح العام UDM بتكوين أسهل suciProfileلتحديد YAML، في حالة وجود UDMs متعددة. المفتاح العام موجود في SUCI، لكن هذا يمنع المدقق من الاضطرار إلى تحليل SUCI لتحديد المفاتيح الضرورية لفك التشفير.

لا تزال كتابة SUCI بدلاً من SUPI (إن وجدت) تسمح بتصحيح خطأ إعادة المزامنة (حيث يجب أن يكون لدى المشغل حق الوصول إلى المفتاح الخاص)، ولكنها تمنع كتابة SUPI في السجل.

سيتم فتح تحديث تجزئة الوحدة الفرعية PR في الريبو الرئيسي إذا تم قبوله.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants