fix(mme): Replace insecure rand() with getrandom() for TMSI generation #15411

jorgevargasmor · 2024-04-14T21:45:29Z

Summary

This PR addresses the vulnerability related to predictable TMSI generation in Magma's Mobility Management Entity (MME). It replaces the current insecure method with a more robust approach, enhancing user privacy and security.

Test Plan

Validate the new TMSI generation method.

Security Considerations

The new TMSI generation method using getrandom() enhances security by providing unpredictable TMSI values, mitigating the risk of tracking user associations by adversaries.

github-actions · 2024-04-14T21:45:43Z

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by PR DCO check.
All PR titles must follow the semantic commits format. This is enforced by PR Check Title Or Commit Message.

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
Checks. All required CI checks must pass before merge.
Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first PR
CI Checks for points of contact for failing or flaky CI checks
Code Review Process for information on requesting reviews and contacting maintainers

github-actions · 2024-04-14T21:45:53Z

✔️ The Semantic PR check ended with status success. See instructions on formatting your commit and pull request titles.

github-actions · 2024-04-14T21:46:28Z

Oops! Looks like you failed the PR Check DCO. Be sure to sign all your commits.

Howto

♻️ Updated: ❌ The check is still failing the PR Check DCO after the last commit.

github-actions · 2024-04-14T21:47:12Z

lte/gateway/c/core/oai/tasks/nas/api/mme/mme_api.cpp

-  // note srand with seed is initialized at main
-  return (tmsi_t)rand();
+  tmsi_t tmsi = (tmsi_t) 0;
+  if (getrandom((void*) &tmsi, sizeof(tmsi_t), 0) != sizeof(tmsi_t) {


[cpplint] _{reported by reviewdog 🐶}
Using C-style cast. Use reinterpret_cast<void*>(...) instead [readability/casting] [4]

github-actions · 2024-04-14T21:47:12Z

lte/gateway/c/core/oai/tasks/nas/api/mme/mme_api.cpp

-  return (tmsi_t)rand();
+  tmsi_t tmsi = (tmsi_t) 0;
+  if (getrandom((void*) &tmsi, sizeof(tmsi_t), 0) != sizeof(tmsi_t) {
+    tmsi = (tmsi_t) rand(); // Fallback in case of fundamental system error


[cpplint] _{reported by reviewdog 🐶}
At least two spaces is best between code and comments [whitespace/comments] [2]

github-actions · 2024-04-14T21:50:23Z

FeG Lint & Test

    2 files 203 suites 39s ⏱️
374 tests 374 ✔️ 0 💤 0 ❌
388 runs 388 ✔️ 0 💤 0 ❌

Results for commit 046bc35.

♻️ This comment has been updated with latest results.

github-actions · 2024-04-14T22:04:23Z

DP Lint & Test

14 tests 14 ✔️ 2m 12s ⏱️
  1 suites   0 💤
  1 files   0 ❌

Results for commit 046bc35.

♻️ This comment has been updated with latest results.

panyogesh

Changes are fine. Please fix the issue reported in build-container job.

lucasgonze · 2024-04-15T14:32:39Z

Let's discuss the fallback to rand() on line 414. Can an attacker force one of these error conditions?

EAGAIN The requested entropy was not available, and getrandom()
              would have blocked if the GRND_NONBLOCK flag was not set.

       EFAULT The address referred to by buf is outside the accessible
              address space.

       EINTR  The call was interrupted by a signal handler; see the
              description of how interrupted [read(2)](https://www.man7.org/linux/man-pages/man2/read.2.html) calls on "slow"
              devices are handled with and without the SA_RESTART flag
              in the [signal(7)](https://www.man7.org/linux/man-pages/man7/signal.7.html) man page.

       EINVAL An invalid flag was specified in flags.

       ENOSYS The glibc wrapper function for getrandom() determined that
              the underlying kernel does not implement this system call.

github-actions · 2024-04-17T03:35:53Z

lte/gateway/c/core/oai/tasks/nas/api/mme/mme_api.cpp

-  // note srand with seed is initialized at main
-  return (tmsi_t)rand();
+  tmsi_t tmsi = (tmsi_t)0;
+  if (getrandom((void*) &tmsi, sizeof(tmsi_t), 0) != sizeof(tmsi_t)) {


[cpplint] _{reported by reviewdog 🐶}
Using C-style cast. Use reinterpret_cast<void*>(...) instead [readability/casting] [4]

github-actions · 2024-04-17T03:38:29Z

lte/gateway/c/core/oai/tasks/nas/api/mme/mme_api.cpp

-  // note srand with seed is initialized at main
-  return (tmsi_t)rand();
+  tmsi_t tmsi = (tmsi_t)0;
+  if (getrandom((void*)&tmsi, sizeof(tmsi_t), 0) != sizeof(tmsi_t)) {


[cpplint] _{reported by reviewdog 🐶}
Using C-style cast. Use reinterpret_cast<void*>(...) instead [readability/casting] [4]

mehul-jindal · 2024-04-17T04:11:21Z

Hi @jorgevargasmor please sync jorgevargasmor:jorge-fix-TMSI-generation with master and replay/rebase the changes of the latest commit on top of it. While committing please use the following command to resolve the remaining failing CI jobs.

git commit --signoff -s -m "fix(mme): Replace insecure rand with getrandom for TMSI generation"

jorgevargasmor requested a review from a team as a code owner April 14, 2024 21:45

jorgevargasmor requested a review from lucasgonze April 14, 2024 21:45

pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines. label Apr 14, 2024

github-actions bot added the component: agw Access gateway-related issue label Apr 14, 2024

github-actions bot reviewed Apr 14, 2024

View reviewed changes

panyogesh approved these changes Apr 15, 2024

View reviewed changes

panyogesh changed the title ~~Replace insecure rand() with getrandom() for TMSI generation~~ fix(mme): Replace insecure rand() with getrandom() for TMSI generation Apr 15, 2024

mehul-jindal force-pushed the jorge-fix-TMSI-generation branch from 9cd2b3e to 44dc21a Compare April 17, 2024 03:34

github-actions bot reviewed Apr 17, 2024

View reviewed changes

fix(mme): Replace insecure rand with getrandom for TMSI generation

046bc35

mehul-jindal force-pushed the jorge-fix-TMSI-generation branch from 44dc21a to 046bc35 Compare April 17, 2024 03:36

github-actions bot reviewed Apr 17, 2024

View reviewed changes

lucasgonze mentioned this pull request Apr 22, 2024

Proposal: Strengthening TMSI Generation #15399

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(mme): Replace insecure rand() with getrandom() for TMSI generation #15411

fix(mme): Replace insecure rand() with getrandom() for TMSI generation #15411

jorgevargasmor commented Apr 14, 2024

github-actions bot commented Apr 14, 2024

github-actions bot commented Apr 14, 2024 •

edited

github-actions bot commented Apr 14, 2024 •

edited

github-actions bot Apr 14, 2024

github-actions bot Apr 14, 2024

github-actions bot commented Apr 14, 2024 •

edited

github-actions bot commented Apr 14, 2024 •

edited

panyogesh left a comment •

edited

lucasgonze commented Apr 15, 2024

github-actions bot Apr 17, 2024

github-actions bot Apr 17, 2024

mehul-jindal commented Apr 17, 2024

fix(mme): Replace insecure rand() with getrandom() for TMSI generation #15411

Are you sure you want to change the base?

fix(mme): Replace insecure rand() with getrandom() for TMSI generation #15411

Conversation

jorgevargasmor commented Apr 14, 2024

Summary

Test Plan

Security Considerations

github-actions bot commented Apr 14, 2024

Howto

More info

github-actions bot commented Apr 14, 2024 • edited

github-actions bot commented Apr 14, 2024 • edited

Howto

github-actions bot Apr 14, 2024

Choose a reason for hiding this comment

github-actions bot Apr 14, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 14, 2024 • edited

FeG Lint & Test

github-actions bot commented Apr 14, 2024 • edited

DP Lint & Test

panyogesh left a comment • edited

Choose a reason for hiding this comment

lucasgonze commented Apr 15, 2024

github-actions bot Apr 17, 2024

Choose a reason for hiding this comment

github-actions bot Apr 17, 2024

Choose a reason for hiding this comment

mehul-jindal commented Apr 17, 2024

github-actions bot commented Apr 14, 2024 •

edited

github-actions bot commented Apr 14, 2024 •

edited

github-actions bot commented Apr 14, 2024 •

edited

github-actions bot commented Apr 14, 2024 •

edited

panyogesh left a comment •

edited