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

Tree based app keyschedule #146

Merged
merged 5 commits into from Jul 8, 2019
Merged

Conversation

@psyoptix
Copy link
Contributor

@psyoptix psyoptix commented Apr 12, 2019

Basic idea:

  • Application Key Schedule consists of a left balanced binary tree of secrets (the "AS Tree") and one symmetric ratchet per group member. The AS Tree has the same node/edge structure as the ratchet tree for that epoch. Members are assigned the same leaves.

  • Each node in the AS Tree is assigned a secret. The root's secret = application_secret. The secrets of children are derived from that of their parent.

  • The secret of a leaf is the initial secret of a symmetric hash ratchet. The ratchet generates the key/nonce sequence used by the leaf's group member to encrypt messages during that epoch.

Other comments:

  • I included a "Deletion Schedule": keys, nonces are 'consumed' if they are used to encrypt or successfully decrypt a message. secrets are 'consumed' if value derived from it is consumed. Any consumed value must be immediately deleted for reasons of forward secrecy.

  • I was very generous with contexts for all calls to HKDF. E.g. I included Hash(GroupState_[n]) in the context of every call to HKDF. True, I dont think its neccesary to prove security against more coarse adversarial models (e.g. that only do all-or-nothing state leakage). Still, as a matter of the "defense in depth" principle I think including as much relevant context as possible during all key/secret derivation is a good idea. Albeit only as long as the price (in computation, complexity, etc) is not to high. To that end, I purposefully use Hash(GroupState_[n]) in the context as it is short, needs only to be computed once at the start of the epoch and can then be used to very cheaply to construct all contexts needed for the rest of the new application key schedule.

  • Disclaimer: I'm a bit of a noob when it comes to Markdown, RFCs and github so forgive me (and tell me!) if I've done something wrong here.

Co-contributors: Benjamin Beurbouche, Sandro Coretti, Yevgeniy Dodis,

@bifurcation bifurcation added this to the draft-06 milestone Apr 25, 2019
@beurdouche beurdouche modified the milestones: draft-06, draft-07 May 17, 2019
@@ -1429,12 +1429,20 @@ all arrive at the following state:
A X Y D
~~~~~

# Message Protection
# Message Protection {#message-protection}

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

Nit: tags of this form (lower-cased, spaces-to-dashes) are automatically generated by the tooling. Same comment below.

draft-ietf-mls-protocol.md Outdated Show resolved Hide resolved
application_secret_[sender]_[0]
~~~~~
The Application schedule begins with the Application secrets which are arranged
in an "AS Tree"; a left balanced binary tree with the same set of nodes and

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

Might be helpful to expand "AS" on first use here. I assume you mean "application secret"?

~~~~
struct {
opaque gshash<0..2^32-1> = Hash(GroupState_[n]);
uint16 node_index;

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

We have been using uint32s for indices elsewhere, to be conservative about size.

V
Derive-Secret(., "astree-secret", ASTreeContext[V.rightChild])
= astree_node_[IndexOf(V.rightChild)]_secret
~~~~

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

What's the reasoning behind using the context you do here? It seems like it would be simpler if you didn't have to keep around that context data. For example, suppose you did something like the following:

           astree_node_[IndexOf(V)]_secret
                     |
                     +--> HKDF-Expand-Label(.,"left", "", Hash.length)
                     |    = astree_node_[IndexOf(V.leftChild)]_secret
                     |
                     +--> HKDF-Expand-Label(.,"right", "", Hash.length)
                          = astree_node_[IndexOf(V.rightChild)]_secret

That would assure you have the diversity you need because every leaf in the tree would be derived via a different "left" / "right" path.

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

Nit: Stylistically:

  • Slightly nicer to have both results on the right as above
  • I think we've been using more snake_case, so, e.g., V.right_child

This comment has been minimized.

@psyoptix

psyoptix Jul 7, 2019
Author Contributor

Here was my thought process which lead to the schedule in the PR.

A property of left/right tree schedule i'm not a fan of: if nodes v and u end up with the same key then their children (and grandchildren, etc.) will have the same key too.

So add IndexOf(V.left_child) as context to Derive-Secret. However this still means if, in 2 different epochs a node in the AS Tree ends has the same key then so will its children. Thats why I added the Hash(GroupState_[n]).

To be clear, its not like I have a concrete attack. Its more of a "best practice" & "defense in depth" situation. IMO different pairs of keys having the same relation to each other make me nervous.

I also hoped that since Hash(GroupState_[n]) only needs to computed once, is a short string (e.g. fits into even lower cache levels) and can be blindly plugged in to all Derive-Secret calls the price for this approach wasn't too great.

So for now I've not made any changes here. However, I dont fell all that strongly about it. So if you do, or there's push back from others as well then feel free to change it.

This comment has been minimized.

@bifurcation

bifurcation Jul 7, 2019
Collaborator

I can see why it's appealing to have each derivation entail a different relation. Maybe we can accommodate that a bit more elegantly by moving the GroupContext hash into the definition of Derive-Secret (since that's all that is used for context anyway), and defining a Derive-Application-Secret function that gives us the knobs we need to vary things as we go down the tree and the ratchets. Here's a sketch that should be copy-paste-able into the text here:

HKDF-Expand-Label(Secret, Label, Context, Length) =
    HKDF-Expand(Secret, HkdfLabel, Length)

Where HkdfLabel is specified as:

struct {
    uint16 length = Length;
    opaque label<7..255> = "mls10 " + Label;
    opaque context<0..2^32-1> = Context;
} HkdfLabel;

Derive-Secret(Secret, Label) =
    HKDF-Expand-Label(Secret, Label, Hash(GroupContext_[n]), Hash.length)

struct {
  uint32 node;
  uint32 generation;
  opaque role<0..255>;
} ApplicationContext;

Derive-Application-Secret(secret, node, generation, role) =
  DeriveSecret(secret, ApplicationContext(node, generation, role) 

astree_node_[IndexOf(V)]_secret
        |
        |
        +--> Derive-Application-Secret(., V.left_child, 0, "tree")
        |    = astree_node_[IndexOf(V.left_child)]_secret
        |
        +--> Derive-Application-Secret(., V.right_child, 0, "tree")
             = astree_node_[IndexOf(V.right_child)]_secret

application_[i]_[j]_secret
      |
      +--> Derive-Application-Secret(., 2*i, j, "key")
      |    = application_[i]_[j+1]_nonce
      |
      +--> Derive-Application-Secret(., 2*i, j, "nonce")
      |    = application_[i]_[j+1]_key
      |
      V
HKDF-Application-Secret(., 2*i, j, "secret")
= application_[i]_[j+1]_secret

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

Here is an proposal for this which is an hybrid of your two suggestions:

HKDF-Expand-Label(Secret, Label, Context, Length) =
    HKDF-Expand(Secret, HkdfLabel, Length)

Where HkdfLabel is specified as:

struct {
    uint16 length = Length;
    opaque label<7..255> = "mls10 " + Label;
    opaque context<0..2^32-1> = Context;
} HkdfLabel;

Derive-Secret(Secret, Label) =
    HKDF-Expand-Label(Secret, Label, Hash(GroupContext_[n]), Hash.length)

struct {
  uint32 node;
  uint32 generation;
  opaque role<0..255>;
} ApplicationContext;

Derive-Application-Secret(secret, node, generation, role) =
  DeriveSecret(secret, ApplicationContext(node, generation, role) 

astree_node_[IndexOf(V)]_secret
        |
        |
        +--> Derive-Application-Secret(., V.left_child, 0, "tree")
        |    = astree_node_[IndexOf(V.left_child)]_secret
        |
        +--> Derive-Application-Secret(., V.right_child, 0, "tree")
             = astree_node_[IndexOf(V.right_child)]_secret


application_[i]_[j]_secret
      |
      +--> HKDF-Expand-Label(.,"nonce", "", nonce_length)
      |    = write_nonce_[i]_[j]
      |
     +--> HKDF-Expand-Label(.,"key", "", key_length)
     |    = write_key_[i]_[j] 
    V
Derive-Application-Secret(., 2*i, 0, "sender")
= application_[i]_[j+1]_secret

This is slightly different in the sense that we have basically don't change our current way of deriving keys and nonces from an application secret.

opaque leaf_index<0..2^16-1>;
uint32 ratchet_position;
} RatchetContext
~~~~

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

Same comment here about context. Why can't we just use a fixed label like TLS does?

       application_traffic_secret_N+1 =
           HKDF-Expand-Label(application_traffic_secret_N,
                             "traffic upd", "", Hash.length)

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

Also, a leaf_index should just be a uint32. Did you mean for that to be an identity?

This comment has been minimized.

@psyoptix

psyoptix Jul 7, 2019
Author Contributor

My reasoning here is the same as I explained above: no 2 keys should have the same relation -> defence in depth.

IMO TLS is designed for greater packets per second than MLS (and probably aims to run on weaker devices like IoT stuff too) so it makes sense for TLS to care a lot about being efficient. MLS though is meant for lower packet rate (on at least a cellphone level device) so I think its worth spending a small number of extra cycles and memory access for reasons of best practice.

Just as before, I'll leave it up to you which way to go though.

This comment has been minimized.

@psyoptix

psyoptix Jul 7, 2019
Author Contributor

Oh. yeah, that wasn't too consistent. I mean leaf_index to denote the number of the leaf assigned to the owner of the ratchet, not their identity.

All I really care about for that the value is that its unique to a given ratchet and that its easy to determine the value for any client in the group. So using a full identity seems like an over kill. The index of the client's leaf should be enough.

I edited things to (hopefully) make that clearer and more consistent.

This comment has been minimized.

@psyoptix

psyoptix Jul 7, 2019
Author Contributor

FYI I also removed the hashing of HashRatCont[i,j]. Now it just goes in the context as is instead of being hashed first.

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

I agree with Richard here, this is introducing too much complexity for debatable reasons. Would we claim the probability of colliding KDF outputs is gonna be reduced by this ? I guess not. astree_node_[i]_secret already mixed in the index of the leaf of the ASTree.

} RatchetContext
~~~~

The identity field is copied from the eponymous field in the BasicCredential

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

BasicCredential is an instance of Credential, so just refer to the identity in the credential. Or just eliminate all of this :)

This comment has been minimized.

@psyoptix

psyoptix Jul 7, 2019
Author Contributor

Its gone. Full identity wasn't needed. Leaf Index will do.

* encrypt and send a message or
* to successfully decrypt and authenticate a received message.

More generally, a secret S is "consumed" if any key, nonce or secret derived

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

Nit: Unfortunate resonance between "More precisely... More generally..."

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

Seems like you could express this by saying a secret is consumed if:

  • It is used to encrypt or decrypt a message
  • Any secret derived from it has been consumed

This comment has been minimized.

@psyoptix

psyoptix Jul 7, 2019
Author Contributor

Agreed on compression encrypt/decrypt into one line.

For the second point I went with "any key, secret or nonce" instead of "any secret" just to be clear that nonce's are included here (as they're normally not thought of as secrets).

* all node secrets in the AS Tree on the path from the root to the leaf with
index i,
* the first j secrets in the i-th ratchet and
* application_[i]\_[j]\_key and application_[i]\_[j]\_nonce.

This comment has been minimized.

@bifurcation

bifurcation Jul 1, 2019
Collaborator

A diagram might help here. E.g., in the following scenario...

      G
    /   \
   /     \
  E       F
 / \     / \
A0  B0  C0  D0 -+- KD0
            |   |
            |   +- ND0
            |
            D1 -+- KD1
            |   |
            |   +- ND1
            |
            D2 -+- KD2
                |
                +- ND2

When KD1/ND1 are used to decrypt a message, the following are:

  • Consumed: G, F, D0, D1, KD1, ND1
  • Not necessarily consumed: E, A0, B0, C0, KD0, ND0, D2, KD2, ND2

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

D2 needs to be consumed because you already derived KD2 and ND2. D3 is missing.

This comment has been minimized.

@psyoptix

psyoptix Jul 8, 2019
Author Contributor

Deriving a value doesnt mean the parent needs to be consumed. Only when the derived values are actually used/consumed does the parent need to be consumed. My understanding of this example is that only KD1/ND1 are used but not KD2/ND2. So D2 doesn't need to be consumed yet.

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

Again, if D2 gets leaked, it strictly worse than leaking D3 because you get KD2 and ND2 as well...

## Further Restrictions {#further-restrictions}

During each epoch senders MUST NOT encrypt more messages than permitted by the
security bounds of the AEAD scheme used.

This comment has been minimized.

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

It is not "during each epoch" it is for each encryption, and it is not a number of messages but an amount of data that counts. Since you derive a fresh key and nonce, your AEAD bound is fresh for each application message anyway, so it is for each application message.

Copy link
Member

@beurdouche beurdouche left a comment

Oki. I have some doubts about many complexities introduced in order to add redundant context in the key derivations which also diverge from the TLS style. I propose to merge this now, as we agreed on the main design at the interim, and do a run of cleanup-simplifications among editors before releasing the draft so that we restore simplicity. We'll then be able to debate over the details as part of our next round of reviews on the draft.

key schedule is used to derive nonces and encryption keys for the Message
Protection Layer according to the Application Key Schedule. That is, each epoch
is equipped with a fresh Application Key Schedule which consist of a tree of
Application Secrets as well as one symmetric ratchet per group member.

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

It is either "at most" one symmetric ratchet per group member or you can say one for each sender in the epoch.

is equipped with a fresh Application Key Schedule which consist of a tree of
Application Secrets as well as one symmetric ratchet per group member.

Each client maintains their own local copy of (parts of) the Application Key

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

It is not the "Application Key Schedule" which is the way to compute the keys but the "Application Secret Tree" which is the data structure that contains computed secrets and keys. Also, I don't think you can escape keeping the entire Application Secret Tree because as a receiver you have to be able to compute any Application Secret Chain.


## Application Key Schedule {#key-schedule-application}
~~~~

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

This block is not necessary, it adds no value as it is perfectly described in the text above.

to be used for its own sending chain:
The secret of any other node in the tree is derived from its parent's secret
using a call to Derive-Secret. The context for the call is the (hash of the)
Group state of the epoch and the index of the node whose secret is being

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

This is called the GroupContext

[[OPEN ISSUE: The HKDF context field is left empty for now.
A proper security study is needed to make sure that we do not need
more information in the context to achieve the security goals.]]
* gshash = Hash(GroupState_[n])

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

s/GroupState/GroupContext

* all node secrets in the AS Tree on the path from the root to the leaf with
index i,
* the first j secrets in the i-th ratchet and
* application_[i]\_[j]\_key and application_[i]\_[j]\_nonce.

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

D2 needs to be consumed because you already derived KD2 and ND2. D3 is missing.

| +- ND1
|
D2 -+- KD2
|

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

This will not render properly

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

D3 is missing from the diagram


Then if a client uses key KD1 and nonce ND1 during epoch n then it must consume
(at least) values G, F, D0, D1, KD1, ND1 as well as the update_secret and
init_secret used to derive G (i.e. the application_secret).

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

Again, if KD2 and ND2 have been derived, D2 has been killed and D3 is missing

This comment has been minimized.

@bifurcation

bifurcation Jul 8, 2019
Collaborator

That's wrong. D2 isn't consumed until KD2 or ND2 is consumed.

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

Hmm, I disagree that breaks FS. D3, KD2 and ND2 are derived at the same time from D2 which is consumed at this occasion.

## Further Restrictions {#further-restrictions}

During each epoch senders MUST NOT encrypt more messages than permitted by the
security bounds of the AEAD scheme used.

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

It is not "during each epoch" it is for each encryption, and it is not a number of messages but an amount of data that counts. Since you derive a fresh key and nonce, your AEAD bound is fresh for each application message anyway, so it is for each application message.

V
Derive-Secret(., "astree-secret", ASTreeContext[V.rightChild])
= astree_node_[IndexOf(V.rightChild)]_secret
~~~~

This comment has been minimized.

@beurdouche

beurdouche Jul 8, 2019
Member

Here is an proposal for this which is an hybrid of your two suggestions:

HKDF-Expand-Label(Secret, Label, Context, Length) =
    HKDF-Expand(Secret, HkdfLabel, Length)

Where HkdfLabel is specified as:

struct {
    uint16 length = Length;
    opaque label<7..255> = "mls10 " + Label;
    opaque context<0..2^32-1> = Context;
} HkdfLabel;

Derive-Secret(Secret, Label) =
    HKDF-Expand-Label(Secret, Label, Hash(GroupContext_[n]), Hash.length)

struct {
  uint32 node;
  uint32 generation;
  opaque role<0..255>;
} ApplicationContext;

Derive-Application-Secret(secret, node, generation, role) =
  DeriveSecret(secret, ApplicationContext(node, generation, role) 

astree_node_[IndexOf(V)]_secret
        |
        |
        +--> Derive-Application-Secret(., V.left_child, 0, "tree")
        |    = astree_node_[IndexOf(V.left_child)]_secret
        |
        +--> Derive-Application-Secret(., V.right_child, 0, "tree")
             = astree_node_[IndexOf(V.right_child)]_secret


application_[i]_[j]_secret
      |
      +--> HKDF-Expand-Label(.,"nonce", "", nonce_length)
      |    = write_nonce_[i]_[j]
      |
     +--> HKDF-Expand-Label(.,"key", "", key_length)
     |    = write_key_[i]_[j] 
    V
Derive-Application-Secret(., 2*i, 0, "sender")
= application_[i]_[j+1]_secret

This is slightly different in the sense that we have basically don't change our current way of deriving keys and nonces from an application secret.

@bifurcation
Copy link
Collaborator

@bifurcation bifurcation commented Jul 8, 2019

Merging this despite outstanding comments. @beurdouche and I will handle our requested edits in a couple of follow-up PRs.

@bifurcation bifurcation merged commit 811235c into mlswg:master Jul 8, 2019
0 of 5 checks passed
0 of 5 checks passed
@netlify
Header rules Deploy failed
Details
@netlify
Mixed content Deploy failed
Details
@netlify
Pages changed Deploy failed
Details
@netlify
Redirect rules Deploy failed
Details
@netlify
deploy/netlify Deploy preview failed.
Details
@bifurcation bifurcation mentioned this pull request Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants