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

Proposed updates #44

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

Proposed updates #44

wants to merge 31 commits into from

Conversation

knorrman
Copy link
Contributor

@knorrman knorrman commented Feb 6, 2023

No description provided.

…d in 5G and the exact function name is not important for this draft
Copy link
Contributor

@jariarkko jariarkko left a comment

Choose a reason for hiding this comment

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

Overall comments:

  • Generally all very good modifications, thank you!
  • A few points where we should discuss if a change is actually needed
  • Also, there's a parallel PR from Sean Turner which changed (editorially) some of the same sentences.

In more detail, per section:

Abstract:

  • All good
  • But wouldn't it make sense to keep the zero-trust mention?

Introduction:

  • All good
  • But is there a reason to remove the paragraph on this work being part of defending against pervasive surveillance?
  • Same question for changing the language from "large number of users" to "many users".

Protocol design:

  • All good

Background:

  • This specification does not introduce a new EAP method. This change seems incorrect.

AKA:

  • All good

EAP-AKA protocol:

  • All good

Attacks Against Long-Term Shared Secrets in Smart Cards:

  • Is there a reason to remove this section?

Protocol overview:

  • Is there a reason for removing the 7748 refernence and X25519?

EAP-AKA' FS Authentication Process:

  • What is AD and was that previously defined?
  • Why was this removed? "A privacy-friendly identifier SHALL be used"
  • This new text is not the usual definition of must be supported: "The term "support" here means that the group MUST be implemented and MUST be possible to use during a protocol run."
  • Otherwise all good

Security considerations:

  • Why was the discussion of attacks on key storage removed?
  • The new consequences discussion removed some aspects that I found important, like impersonation. Was there a reason?

(Run out of time to review the rest)

@knorrman
Copy link
Contributor Author

knorrman commented Apr 8, 2023

Abstract:

wouldn't it make sense to keep the zero-trust mention?

The present text just makes a general claim that assuming breach and minimizing its
impact are essential zero-trust principles. I changed that into text
directly relating compromises to what the protocol actually provides,
i.e., reduced impact of long-term key compromise. I can live with a ZT
reference, but please rephrase it so that it clearly relates to what the
protocol does and what definition of ZT is intended. That term has so many
different meanings to different people.

Introduction:

is there a reason to remove the paragraph on this work being part of
defending against pervasive surveillance?

It is not removed, only rewritten to focus on what the protocol does: provide
session key FS w.r.t the long-term key for AKA. Later in the introduction it is
mentioned that the protocol is part of IETF's goal to handle surveillance.

Same question on "large number of" ==> "many".

This was just text compression. I have no strong feeling for the change.

Background:

This specification does not introduce a new EAP method. This change
seems incorrect.

OK. You probably know that better. Please change it to "extension of EAP-method"
or whatever is the correct term.

Attacks Against Long-Term Shared Secrets in Smart Cards:

Is there a reason to remove this section?

The section mostly repeat information from the introduction. I moved the
information I missed to the introduction instead. Then it is all in one place
and once. The end of the section is also technically incorrect in what one
can expect from forward secrecy.

Protocol overview:

Is there a reason for removing the 7748 reference and X25519?

The clause gives an example of a group to use for the DH computation. It does
not specify which groups are required to be implemented, so I did not see how
it helps the reader to get a reference to an example of how to do the detailed
computations here. Also 7748 is referenced earlier in the same sentence.
If the idea is that here is where it is specified that implementations must
support X25519, then please remove the "e.g." preceding the requirement and
add a SHALL.

EAP-AKA' FS Authentication Process:

What is AD and was that previously defined?

AD is "Authentication Database" mentioned just above the figure. I forgot
to spell out that AD is the abbreviation. The point was to remove HSS, because
that is not used in 5G. Since EAP-AKA-FS could be used in 3G and 4G for
non-3GPP access, it felt wrong to switch completely to 5G terminology (and definitely
to stick with the HSS terminology). That was the reason for introducing a generic term, AD, here.

Why was this removed? "A privacy-friendly identifier SHALL be used"

Privacy-friendly identifiers are not defined, so it is unclear how to test compliance with the SHALL.
I instead specified that identifiers shall comply with the privacy
requirements for identifiers in RFC 9190, and that a non-NULL SUCI according to
33.501 would be an example.

This new text is not the usual definition of must be supported:
"The term "support" here means that the group MUST be implemented and MUST be
possible to use during a protocol run."

OK, can we point to a place where that is defined then? My point is to avoid
confusion regarding whether "support" means only implementation or whether it
means that it shall be possible to use during a protocol run, or even that it
shall be used during the run. We want to avoid these kind of compliance
discussions later.

Security considerations:

Why was the discussion of attacks on key storage removed?

The discussion was compressed because it is repeating text, some even verbatim from the
introduction, e.g., the general claims about zero-trust principles.

It also seems to give a skewed view. A discussion on compromised supply chains needs
to cover most of the important products handling the session keys (for proper deletion) and the
long-term keys. Also operational aspects of storage are important.
Either that should be added or the section should not focus on the supply chain of a single entity
in the system. Such a limited scope gives the reader a biased view. The question is
if this is the place for a long discussion on storage security.

The new consequences discussion removed some aspects that I found important,
like impersonation. Was there a reason?

This paragraph was technically incorrect. It claimed that a mechanism for FS
would prevent certain attacks in later sessions, but it is not the case:

The serious consequences of breach somewhere in the supply chain
or after delivery that are possible when 5G-AKA or EAP-AKA' is
used but not when something with forward secrecy like EAP-AKA-FS
is used are:
...
A passive attacker can eavesdrop (decrypt) all
future 5G communication (control and user plane both directions),

The above attack has nothing to do with forward secrecy. The reason EAP-AKA-FS protects against
it comes from how DH is used. We get more from DH than just FS. Protecting against what is described
above requires some form of post compromise security (PCS).

An active attacker can impersonate UE and
Network and inject messages in an ongoing 5G connection between
the real UE and the real network (control and user plane both
directions)

The above attack is possible even when EAP-AKA-FS is used. Since the adversary is active, it simply mounts
a MITM attack on the authentication protocol run and then injects traffic as it pleases on the air-link.
The adversary can also simply run EAP-AKA-FS as the real user and impersonate it towards the network, since it has the long-term key.

Also this attack is unrelated to forward secrecy and rather relates to PCS.

@emanjon
Copy link
Member

emanjon commented Apr 15, 2023

The XML does not compile. I also think the SVG is not possible to fix manually as the positions also change when the text is changes. HSS should be changes as that is 4G. And as the term seems to change every gen it is probably best with a general term like AD.

I will commit the changes that affect the figures "AD" and "long-term key" to master and regenerate the SVG.

@emanjon
Copy link
Member

emanjon commented Apr 16, 2023

Now everything from the IETF last call reviews (except explaining why ECDHE is not in some of the keys) are fixed in master. This PR needs som manual handling.

  • I made the HSS to AD change and recompiled the figures.
  • I think it was already quite clear exactly which zero trust principles we referred to. No they are backed up by references.
  • I also think the term "supply chain" should stay in some form. I was picked up in the directorate reviews.
  • I will try to merge some parts of this PR today, and provide comments on other parts if needed

@emanjon
Copy link
Member

emanjon commented Apr 24, 2023

Some comment


That is, after the compromise, the attacker must still perform a
man-in-the-middle (MITM) attack on exchanges it wishes to obtain the session key
from.

This is just one form of active attack. A attacker can also impersonate.


mitigating long-term key compromise

Changes this to
"minimizing the impact of long-term key compromise"


Changed some additional identity module to USIM


Authentication Server (AS) is not used. I suggest we don't introduce terms (AS )we don't use


Ephemeral public-key validation requirements are defined in Section 5 of .

The NIST ublic-key validation requirements are genereal, not for ephemeral.

I added this instead:
"At least partial public-key validation MUST be done for the ephemeral public keys."


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