project-everest / mitls-fstar

Verified implementation of TLS 1.3 in F*
https://www.mitls.org
Other
173 stars 16 forks source link

Roadmap for a unified QUIC/TLS Multistream atop EverCrypt AEAD #227

Open s-zanella opened 5 years ago

s-zanella commented 5 years ago

We want to implement a unified Multistream API atop EverCrypt AEAD that can be used for both QUIC and TLS.

Requirements:

Tasks:

Tasks in hacl-star we depend on:

Issues to remember:

ad-l commented 5 years ago

Starting on Mem now, locking that module

msprotz commented 5 years ago

We chatted about key expansion this morning. It's on the radar of @parno. I don't know if @karthikbhargavan's C implementation of AES-GCM also features key expansion.

Can you elaborate on 3. -- what does "connect to HACL*" mean?

parno commented 5 years ago

@protz Do you mean IVs of non-standard length? I recall talking about that. I don't remember discussing key expansion.

msprotz commented 5 years ago

Ah sorry, I got confused between the two. My bad. Then I don't know why Spec.AEAD misses key expansion. I see it there: https://github.com/project-everest/hacl-star/blob/fstar-master/specs/Spec.AEAD.fsti#L114

parno commented 5 years ago

Regarding maximum input length, for AES-GCM, I believe @R1kM has a branch where he's working on increasing that to the NIST spec's limit. I'm not sure how far along he is.

R1kM commented 5 years ago

Regarding maximum input length, I had started around a week ago, but I was preempted and it still needs some more work. I'll look at it in the next few days. Thanks for the reminder!

s-zanella commented 5 years ago

@protz: Re defects in Spec.AEAD: these were mainly raised by @fournet, so I don't know if my interpretation is accurate.

  1. Maximum input length is too short: We all agree this refers to https://github.com/project-everest/hacl-star/blob/fstar-master/specs/Spec.AEAD.fsti#L62.
  2. Output length is too large: I don't know what this means.
  3. Connect to Hacl: I think this means proving that a portable Hacl implementation of AES-GCM satisfies Spec.AEAD. The experimental implementation I see in _dev is proved memory safe only. I see this other comment that may be relevant: https://github.com/project-everest/hacl-star/blob/fstar-master/providers/evercrypt/fst/EverCrypt.AEAD.fst#L54
  4. Misses key expansion: I was under the impression that this was implemented and as Jonathan pointed out it indeed is. Maybe Cédric meant something else.
ad-l commented 5 years ago
  1. refers to https://github.com/project-everest/hacl-star/blob/fstar-master/specs/Spec.AEAD.fsti#L117 - the max size of cipher should be max_length a + tag_length a, i.e. pow2 20 - 1.
  2. Spec.AEAD has expand, but it is not used in EverCrypt.AEAD (for create).
  3. Spec.AEAD is missing a correctness lemma, i.e.
    val lemma_encrypt_decrypt: #a:supported_alg -> k:kva -> n:iv a -> aad:ad a -> p:plain a ->
    Lemma (decrypt k n aad (encrypt k n aad p) == Some p)
ad-l commented 5 years ago

Tasks for upgrading the memory model:

s-zanella commented 5 years ago

@ad-l

  1. refers to https://github.com/project-everest/hacl-star/blob/fstar-master/specs/Spec.AEAD.fsti#L117 - the max size of cipher should be max_length a + tag_length a, i.e. pow2 20 - 1.

I don't see how this relates to

  1. Output length is too large

I think this is another flaw, but also a case of 1. Maximum input length is too short.

  1. Spec.AEAD has expand, but it is not used in EverCrypt.AEAD (for create).

Although EverCrypt.AEAD.fsti doesn't use expand explicitly, it is part of the abstract invariant in the implementation: https://github.com/project-everest/hacl-star/blob/ca3e12f21d1879f192f19105dcc43ffe2f6e350b/providers/evercrypt/fst/EverCrypt.AEAD.fst#L61 I'm still not sure what this point means.

I guess you meant 4 instead of 3

  1. Spec.AEAD is missing a correctness lemma, i.e.
    
    val lemma_encrypt_decrypt: #a:supported_alg -> k:kva -> n:iv a -> aad:ad a -> p:plain a ->
    Lemma (decrypt k n aad (encrypt k n aad p) == Some p)```

And this is yet another defect to add to the list.

R1kM commented 5 years ago

@ad-l , I pushed a change to fstar-master that doesn't restrict the length of the input or output for the Vale AES-GCM, which should address points 1 and 2. Let me know if you have any other issue with this.

ad-l commented 5 years ago

@s-zanella in 8d0b4076260f502160999d72a29a4a11845cf6c4 Pkg, DefineTable and Mem are now fully verified. Can you take care of Idx? I'm moving to KDF now

ad-l commented 5 years ago

@s-zanella Some problems I identified when looking at Idx this morning:

s-zanella commented 5 years ago

@ad-l I'm afraid that if we define honesty in terms of witnessed many lemmas will only be provable in ST because of the need to bring up the witness to testify. That said, some logical manipulations on witnesses are possible, so some lemmas will be provable without lifting to the ST effect.

lemma_honest_parent is provable in ST without any axioms:

val lemma_honest_parent (i:regid) (lbl:label) (ctx:context)
  : ST unit (requires fun h0 ->
            wellformed_derive i lbl ctx /\ 
            registered (derive i lbl ctx) /\ 
            ~(honest_idh ctx) /\ 
            honest (derive i lbl ctx))
          (ensures fun h0 _ h1 -> h0 == h1 /\ honest i)
let lemma_honest_parent i lbl ctx =
  if model then
    let log : i_honesty_table = honesty_table in
    testify (MDM.contains log (derive i lbl ctx) true);
    let x = MDM.lookup log i in
    ()
  else ()
s-zanella commented 5 years ago

@ad-l It's also provable as a Lemma ~assuming this very reasonable axiom (it might be actually derivable from what's in FStar.HyperStack already):~ Edit: of course it is

val lemma_witnessed_true (p:mem_predicate) :
  Lemma (requires (forall h. p h)) (ensures witnessed p)
let lemma_witnessed_true p =
  lemma_witnessed_constant True;
  weaken_witness (fun _ -> True) p
val lemma_honest_parent (i:regid) (lbl:label) (ctx:context)
  : Lemma (requires
            wellformed_derive i lbl ctx /\ 
            registered (derive i lbl ctx) /\ 
            ~(honest_idh ctx) /\ 
            honest (derive i lbl ctx))
          (ensures honest i)
let lemma_honest_parent i lbl ctx =
  if model then
    let log : i_honesty_table = honesty_table in
    lemma_witnessed_true (fun h -> MDM.contains log (derive i lbl ctx) true h ==> MDM.contains log i true h);
    lemma_witnessed_impl (MDM.contains log (derive i lbl ctx) true) (MDM.contains log i true)
  else ()
s-zanella commented 5 years ago

Idx.fst fully verifies (see https://github.com/project-everest/mitls-fstar/commit/a3259976e981d4c57bf4ce89f68848d4464e5f06 and https://github.com/project-everest/mitls-fstar/commit/07ab3e8ccabfdbb3829706db50dce1846ce43f4b#diff-a051a34719121d8ff29570c947d090f1). We managed to replace all required stateful lemmas with pure versions and so there are no axioms left.

We still rely on compatibility lemmas to bridge between old style (FStar.HyperStack) and new style (LowStar.Monotonic.Buffer) modifies clauses because FStar.Monotonic.DependentMap hasn't been ported yet. I added this as a new item to the list of modules to upgrade.

ad-l commented 5 years ago

I propagated the Idx changes. As discussed, I'm preparing to merge to dev just to ensure the other ideal functionalities like CRF use the latest memory model and libraries. I'm working on restoring extraction on adl_memory_model now