doc: DRkey documentation #4102

JordiSubira · 2021-08-12T07:48:03Z

This PR contains the updated documentation for DRKey based on the discussion #4039.

[doc]

This change is

mlegner

Thanks for writing this up. Just finished my review until (excluding) key establishment.

Reviewable status: 0 of 2 files reviewed, 22 unresolved discussions (waiting on @JordiSubira)

doc/cryptography/DRKeyInfra.rst, line 15 at r1 (raw file):

DRKey is used for the following systems:

- SCMP

Indirectly through the SPAO.

doc/cryptography/DRKeyInfra.rst, line 16 at r1 (raw file):

- SCMP
- SIBRA / COLIBRI

I suggest removing the name "SIBRA"

doc/cryptography/DRKeyInfra.rst, line 17 at r1 (raw file):

- SCMP
- SIBRA / COLIBRI
- Security Extension

This is now the "SCION Packet Authenticator Option (SPAO)"

doc/cryptography/DRKeyInfra.rst, line 18 at r1 (raw file):

- SIBRA / COLIBRI
- Security Extension
- PISKES

No longer used.

doc/cryptography/DRKeyInfra.rst, line 19 at r1 (raw file):

- Security Extension
- PISKES
- OPT

No longer used.

doc/cryptography/DRKeyInfra.rst, line 31 at r1 (raw file):

    _                     subscript

    KDF_K (X)             key derivation function using key K and taking X as an input

Maybe emphasize the difference to the PRF somewhere? Both allow key derivation but they differ in their efficiency.

doc/cryptography/DRKeyInfra.rst, line 34 at r1 (raw file):

    PRF_K (X)             pseudorandom function using key K and taking X as an input

    A                     autonomous system

Identified by the pair ISD and AS number?

doc/cryptography/DRKeyInfra.rst, line 35 at r1 (raw file):

    A                     autonomous system
    H_A                   end host identified by their IP address

Can also be a different address type.

doc/cryptography/DRKeyInfra.rst, line 52 at r1 (raw file):

=================

In the DRKey protocol, the key establishment is offloaded to the certificate server

Maybe "system" would be better than "protocol".

Also, use "service" instead of "server". Not sure what the conventions are for "control service" vs. individual services like certificate service.

doc/cryptography/DRKeyInfra.rst, line 55 at r1 (raw file):

(CS). The certificate server leverages efficient key derivation and
pseudorandom functions to derive keys in the hierarchy from top
to bottom.

Nit: trailing whitespaces here and in some other places.

doc/cryptography/DRKeyInfra.rst, line 58 at r1 (raw file):

The first secret in the hierarchy (``SV_A``) is derived from a long-term ``master_secret``.
How often the ``master_secret`` must be rotated is up for discussion.

This is very unclear. Is it up to the individual AS or is there simply no consensus yet?

doc/cryptography/DRKeyInfra.rst, line 64 at r1 (raw file):

=================

We define two types of derivation the :ref:`drkey-specific-derivation` derivation and

Add a :.

doc/cryptography/DRKeyInfra.rst, line 68 at r1 (raw file):

4-level derivation
--------------------

Nit: These should be as long as the heading text (also in other places).

doc/cryptography/DRKeyInfra.rst, line 75 at r1 (raw file):

   * - Identifier
     - Derivation

As the derivation is different depending on the hierarchy type, I suggest describing this separately.

doc/cryptography/DRKeyInfra.rst, line 81 at r1 (raw file):

     - Level 0 key, secret value
   * - ``K_{A->B}``
     - ``PRF_{SV_A^{proto}}(B)``

Why is "proto" only included here?

doc/cryptography/DRKeyInfra.rst, line 87 at r1 (raw file):

     - Level 2 "to" key, AS-to-host
   * - ``K_{A:H_A->B}``
     - ``PRF_{K_{A->B}}(H_A)``

In the "generic" hierarchy, the second-level derivations also include the protocol.

doc/cryptography/DRKeyInfra.rst, line 99 at r1 (raw file):

`proto` can be defined as a fixed-size value, e.g. a 2-byte identifier.

The PRF derivation for every key includes the *type* ("AS-to-AS", ""AS-to-host",

How does it include it? Ideally, this would be added in the table above.

doc/cryptography/DRKeyInfra.rst, line 100 at r1 (raw file):

The PRF derivation for every key includes the *type* ("AS-to-AS", ""AS-to-host", 
"host-to-AS" and "host-to-host"). This is useful to avoid deriving the same values

Sometimes this is called "domain separation", see for example here.

doc/cryptography/DRKeyInfra.rst, line 141 at r1 (raw file):

.. code-block:: text
    
    SV_A^{proto}          (0th level)

Are the levels on the right aligned here? (Sorry, I'm only looking at the source code, not the rendered result.)

doc/cryptography/DRKeyInfra.rst, line 175 at r1 (raw file):

This derivation scheme allows applications to define "niche" protocols. 
`protocol` is a (variable or fixed size) value identifying this "niche" protocol.

To be able to use it in the SPAO, a fixed-size value would be preferred.

doc/cryptography/DRKeyInfra.rst, line 199 at r1 (raw file):

single packet request/response use cases (e.g. a few seconds).

Validity periods

The heading doesn't really seem to fit the following text.

doc/cryptography/DRKeyInfra.rst, line 376 at r1 (raw file):

The superscript ``SCMP`` is omitted for brevity. 

.. image:: fig/scmp_auth.png

Nit: missing newline

matzf

Thanks for the updated document! I had already started with the review when Markus published his comments; I tried to remove the comments where I said the same thing, sorry if there are still some left -- and sorry that it's so many comments now.

Reviewable status: 0 of 2 files reviewed, 41 unresolved discussions (waiting on @JordiSubira and @mlegner)

doc/cryptography/DRKeyInfra.rst, line 1 at r1 (raw file):

********************

Nit, the file name convention for the .rst files "kebab case" (lower case, words separated with dashes). Or just shorten it todrkey.rst.

doc/cryptography/DRKeyInfra.rst, line 29 at r1 (raw file):

    |                     bitstring concatenation
    ^                     superscript
    _                     subscript

RST supports math notation, this would improve the readability in this document.

doc/cryptography/DRKeyInfra.rst, line 42 at r1 (raw file):

Note that the arrow notation does *not* refer to the communication direction of the key, but

Nit. The arrow notation has not been introduced at this point. This would make more sense further down, where the keys are actually introduced.
Perhaps this would also be a good place to introduce some terminology for the "from" and "to" sides -- I really dislike these terms as they are utterly confusing in the context of communication. I'd suggest something like "deriving side" and "requesting side", or "fast" and "slow" sides.
In fact, the arrow notation is rather unfortunate too -- maybe we can just chose a different delimiter; simply using a comma would work best I think.

doc/cryptography/DRKeyInfra.rst, line 110 at r1 (raw file):

to derive keys on the fly for the specified protocol.

**1st level key**

I think using the "alternate" names for these section titles would be clearer, e.g. "AS-to-AS Key", "AS-to-Host Key (Level 2)", "Host-to-AS Key (Level 2)", "Host-To-Host Key (Level 3)". Even though I dislike the "to" terminology, as mentioned above -- perhaps we could just change these names, drop the "to"; "AS--AS", "AS--Host" etc.

doc/cryptography/DRKeyInfra.rst, line 113 at r1 (raw file):

These keys are derived locally in the AS_A certificate server (CS_A) and exchanged
with remote certificate servers (e.g. ``K_{A->B}`` is shared with the CS_B).

Here and for other keys; who the key is shared with should not be an example, this seems like an important part of the definition.

doc/cryptography/DRKeyInfra.rst, line 141 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Are the levels on the right aligned here? (Sorry, I'm only looking at the source code, not the rendered result.)

Agreed + if we use math notation for the keys, this won't work for this diagram and it will stand out even more. I don't know what would be easiest; maybe some TeX drawing, or creative use of a table in RST or just embed an image?
This diagram seems important enough to make it readable.

doc/cryptography/DRKeyInfra.rst, line 166 at r1 (raw file):

    +-----------------------------------------+
    |                                         |
    K_{A:H_A->B}^{protocol}           K_{A->B:H_B}^{protocol}         (2nd level)

Nit; protocol vs proto used above.

doc/cryptography/DRKeyInfra.rst, line 172 at r1 (raw file):

The generic-protocol derivation can be thought of as a special case of the 
protocol-specific derivation for the 0th and 1st level keys. 
For instance, using a special `proto` value in the protocol-specific scheme, e.g. 0 or "".

Either leave this out or clarify that the details of the first two derivation steps are an AS local decision.

doc/cryptography/DRKeyInfra.rst, line 175 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

To be able to use it in the SPAO, a fixed-size value would be preferred.

Agreed + this is not a local decision and this must be specified in this document. I've been assuming we'd use a 16-bit identifier in the SPAO specs.

doc/cryptography/DRKeyInfra.rst, line 175 at r1 (raw file):

This derivation scheme allows applications to define "niche" protocols. 
`protocol` is a (variable or fixed size) value identifying this "niche" protocol.

Somewhere, this document needs to define how the choice is made between the two hierarchies. It should say that,

the Lvl1 requests for the generic hierarchy work normally, with the protocol identifier set to GENERIC = 0.
the Lvl2/3 requests always include only the final protocol identifier. The key service chooses the protocol-specific derivation if it exists or the generic derivation otherwise.

doc/cryptography/DRKeyInfra.rst, line 190 at r1 (raw file):

Defining a reasonable lower bound for the epoch length used in DRKey to avoid 
nonsensical scenarios is up for discussion.

This document should define this now. (There are a few other similar sentences; I think this document should really be a specification, stuff like this should not be left open. If it's really unclear, add a note that this might still be changed, but at least provide a choice for now).

doc/cryptography/DRKeyInfra.rst, line 210 at r1 (raw file):

We suggest establishing a global fixed period of 15 minutes.

Whether ASes are allowed to request/serve keys for past epochs is up for discussion.

Drop this or make it a note.

doc/cryptography/DRKeyInfra.rst, line 251 at r1 (raw file):

letting all keys expire at the same time would lead to peaks in key requests.
Such peaks can be avoided by spreading out key expiration, which in turn will
lead to spreading out the fetching requests. To this end, we introduce the

Maybe stupid question, but isn't it easier to just say that the requesters "SHOULD" add a random offset (say, in [0, 10 minutes]) to the prefetch time?

doc/cryptography/DRKeyInfra.rst, line 261 at r1 (raw file):

that uniformly maps the AS identifiers of the source in AS A and the destination
in AS B to a range between 0 and `t`. `t` should be carefully chosen so that gaps cannot
exist even if the epoch duration for a given SV changes.

If you want to keep this (instead of the suggestion with random offset for the prefetch time); I'd change this to [0, EpochDuration)and just say that additonaly care must be taken if the epoch duration should be changeable.

Btw, this scheme seems like a somewhat tricky thing to do, as it needs to be implemented identically in both the key server and the nodes that use the protocol secret to derive keys.

doc/cryptography/DRKeyInfra.rst, line 284 at r1 (raw file):

.. code-block:: text

    message DRKeySVRequest{

Names; this will be in a package proto.drkey.v1; the DRKey name prefix can be dropped.

doc/cryptography/DRKeyInfra.rst, line 288 at r1 (raw file):

      Timestamp val_time = 1;
      // Protocol-specific value.
      Enum protocol_id = 2;

This Enum needs to be defined;

enum Protocol {
    GENERIC = 0;
    SCMP = 1;
    ...
    reserved 65536 to max; // only 16-bit values allowed
}

message SVRequest {
   Timestamp val_time = 1;
   Protocol protocol = 2;
}

doc/cryptography/DRKeyInfra.rst, line 297 at r1 (raw file):

      Timestamp epoch_end = 2;
      // SV Key.
      bytes drkey = 3;

Just key?

doc/cryptography/DRKeyInfra.rst, line 321 at r1 (raw file):

      Enum protocol_id = 1;
      // Requested DRKeyType.
      Enum req_type = 2;

Isn't the type implied by the presence of src_host/dst_host? Otherwise, this enum needs a definition.

doc/cryptography/DRKeyInfra.rst, line 329 at r1 (raw file):

      uint64 dst_ia = 5;
      // Src Host of the request DRKey (optional).
      bytes src_host = 6;

In the other protobuf definitions, this is usually represented as a string. It doesn't strike me as an ideal choice either, but perhaps consistency is still a plus. I wonder whether we need to worry about ambiguous address types (i.e. addresses of different types having same textual representation).

doc/cryptography/DRKeyInfra.rst, line 333 at r1 (raw file):

      bytes dst_host = 7;
      // Additional information (optional).
      bytes misc = 8;

Drop this misc; protobuf is extensible by design, adding in an unspecified field for possible later use is just unnecessary.

doc/cryptography/DRKeyInfra.rst, line 353 at r1 (raw file):

(without querying the required key at the certificate server). In the following,
we discuss an example usage of DRKey where key derivation is performed on border
routers (BR).

I think this example can safely be dropped; this will be covered by the SPAO and authenticated SCMP specs. Perhaps just add links.

mlegner

Reviewable status: 0 of 2 files reviewed, 41 unresolved discussions (waiting on @JordiSubira, @matzf, and @mlegner)

doc/cryptography/DRKeyInfra.rst, line 141 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Agreed + if we use math notation for the keys, this won't work for this diagram and it will stand out even more. I don't know what would be easiest; maybe some TeX drawing, or creative use of a table in RST or just embed an image?
This diagram seems important enough to make it readable.

In the SCION book, we currently have this figure, which you could maybe adapt:

\begin{tikzpicture}[level/.style={level distance=25mm,sibling distance=4cm},scale=.45,edge from parent/.style={draw,-latex}]
	\node (sv){($\sv{A}$)}
	child[sibling distance=8cm] {node (p1) {$\sv{A}^{p}$}
		child {node (1B) {$K_{A\to B}^{p}$}
			child {node (2from) {$K_{A:H_A\to B}^{p}$}
				child {node (3B) {$K_{A:H_A\to B:H_B}^{p}$}}
			}
			child {node (2to) {$K_{A\to B:H_B}^{p}$}}
		}
	}
	child[sibling distance=8cm] { node (ps) {$\sv{A}^{\star}$}
		child {node (1s) {$K_{A\to B}^{\star}$}
			child {node (2spfrom) {$K_{A:H_A\to B}^{\star,p}$}
				child {node (3sp) {$K_{A:H_A\to B:H_B}^{\star,p}$}}
			}
			child {node (2spto) {$K_{A\to B:H_B}^{\star,p}$}}
		}
	}
	;
	\path[font=\small] (8,0)
		+(ps|-ps) node {0th level}
		+(ps|-1s) node {1st level}
		+(ps|-2spto) node {2nd level}
		+(ps|-3sp) node {3rd level};
\end{tikzpicture}

JordiSubira

Reviewable status: 0 of 3 files reviewed, 36 unresolved discussions (waiting on @JordiSubira, @matzf, and @mlegner)

doc/cryptography/DRKeyInfra.rst, line 15 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Indirectly through the SPAO.

Done.

doc/cryptography/DRKeyInfra.rst, line 16 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

I suggest removing the name "SIBRA"

Done.

doc/cryptography/DRKeyInfra.rst, line 17 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

This is now the "SCION Packet Authenticator Option (SPAO)"

Done.

doc/cryptography/DRKeyInfra.rst, line 18 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

No longer used.

Done.

doc/cryptography/DRKeyInfra.rst, line 19 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

No longer used.

Done.

doc/cryptography/DRKeyInfra.rst, line 29 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

RST supports math notation, this would improve the readability in this document.

Done.

doc/cryptography/DRKeyInfra.rst, line 31 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Maybe emphasize the difference to the PRF somewhere? Both allow key derivation but they differ in their efficiency.

Done.

doc/cryptography/DRKeyInfra.rst, line 34 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Identified by the pair ISD and AS number?

Done.

doc/cryptography/DRKeyInfra.rst, line 35 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Can also be a different address type.

Done.

doc/cryptography/DRKeyInfra.rst, line 52 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Maybe "system" would be better than "protocol".

Also, use "service" instead of "server". Not sure what the conventions are for "control service" vs. individual services like certificate service.

Done.

doc/cryptography/DRKeyInfra.rst, line 58 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

This is very unclear. Is it up to the individual AS or is there simply no consensus yet?

Done.

doc/cryptography/DRKeyInfra.rst, line 64 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Add a :.

Done.

doc/cryptography/DRKeyInfra.rst, line 75 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

As the derivation is different depending on the hierarchy type, I suggest describing this separately.

Done.

doc/cryptography/DRKeyInfra.rst, line 81 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Why is "proto" only included here?

Now, it should be more clear.

doc/cryptography/DRKeyInfra.rst, line 87 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

In the "generic" hierarchy, the second-level derivations also include the protocol.

Now, it should be more clear.

doc/cryptography/DRKeyInfra.rst, line 99 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

How does it include it? Ideally, this would be added in the table above.

Done.

doc/cryptography/DRKeyInfra.rst, line 100 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

Sometimes this is called "domain separation", see for example here.

Done.

doc/cryptography/DRKeyInfra.rst, line 110 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

I think using the "alternate" names for these section titles would be clearer, e.g. "AS-to-AS Key", "AS-to-Host Key (Level 2)", "Host-to-AS Key (Level 2)", "Host-To-Host Key (Level 3)". Even though I dislike the "to" terminology, as mentioned above -- perhaps we could just change these names, drop the "to"; "AS--AS", "AS--Host" etc.

Done.

doc/cryptography/DRKeyInfra.rst, line 113 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Here and for other keys; who the key is shared with should not be an example, this seems like an important part of the definition.

Done.

doc/cryptography/DRKeyInfra.rst, line 141 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

In the SCION book, we currently have this figure, which you could maybe adapt:

\begin{tikzpicture}[level/.style={level distance=25mm,sibling distance=4cm},scale=.45,edge from parent/.style={draw,-latex}]
	\node (sv){($\sv{A}$)}
	child[sibling distance=8cm] {node (p1) {$\sv{A}^{p}$}
		child {node (1B) {$K_{A\to B}^{p}$}
			child {node (2from) {$K_{A:H_A\to B}^{p}$}
				child {node (3B) {$K_{A:H_A\to B:H_B}^{p}$}}
			}
			child {node (2to) {$K_{A\to B:H_B}^{p}$}}
		}
	}
	child[sibling distance=8cm] { node (ps) {$\sv{A}^{\star}$}
		child {node (1s) {$K_{A\to B}^{\star}$}
			child {node (2spfrom) {$K_{A:H_A\to B}^{\star,p}$}
				child {node (3sp) {$K_{A:H_A\to B:H_B}^{\star,p}$}}
			}
			child {node (2spto) {$K_{A\to B:H_B}^{\star,p}$}}
		}
	}
	;
	\path[font=\small] (8,0)
		+(ps|-ps) node {0th level}
		+(ps|-1s) node {1st level}
		+(ps|-2spto) node {2nd level}
		+(ps|-3sp) node {3rd level};
\end{tikzpicture}

I decided to use rst tables and include the information in the previous scheme within the table. I think the math notation is easy to follow, otherwise we can investigate some other alternative.

doc/cryptography/DRKeyInfra.rst, line 172 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Either leave this out or clarify that the details of the first two derivation steps are an AS local decision.

Done.

doc/cryptography/DRKeyInfra.rst, line 175 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Agreed + this is not a local decision and this must be specified in this document. I've been assuming we'd use a 16-bit identifier in the SPAO specs.

Done.

doc/cryptography/DRKeyInfra.rst, line 175 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Somewhere, this document needs to define how the choice is made between the two hierarchies. It should say that,

the Lvl1 requests for the generic hierarchy work normally, with the protocol identifier set to GENERIC = 0.

the Lvl2/3 requests always include only the final protocol identifier. The key service chooses the protocol-specific derivation if it exists or the generic derivation otherwise.

I included it within the exchange sections, let me know what you think.

doc/cryptography/DRKeyInfra.rst, line 190 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

This document should define this now. (There are a few other similar sentences; I think this document should really be a specification, stuff like this should not be left open. If it's really unclear, add a note that this might still be changed, but at least provide a choice for now).

Done.

doc/cryptography/DRKeyInfra.rst, line 199 at r1 (raw file):

Previously, mlegner (Markus Legner) wrote…

The heading doesn't really seem to fit the following text.

Done.

doc/cryptography/DRKeyInfra.rst, line 210 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Drop this or make it a note.

Done.

doc/cryptography/DRKeyInfra.rst, line 251 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Maybe stupid question, but isn't it easier to just say that the requesters "SHOULD" add a random offset (say, in [0, 10 minutes]) to the prefetch time?

I also thought about that and I agree that if we consider the prefetcher as part of the specification (before it wasn't) we can use specify this expected behavior. For the implementation, I'd say that we can even consider not implementing this "spreading" mechanism at the moment. The prefetcher will be implemented as a periodic.Runner and I guess we could assume that different Runners (for different ASes) start at different time, i.e. they will request Lvl1 keys also at different points in time.

doc/cryptography/DRKeyInfra.rst, line 284 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Names; this will be in a package proto.drkey.v1; the DRKey name prefix can be dropped.

Done.

doc/cryptography/DRKeyInfra.rst, line 288 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

This Enum needs to be defined;

enum Protocol {
    GENERIC = 0;
    SCMP = 1;
    ...
    reserved 65536 to max; // only 16-bit values allowed
}

message SVRequest {
   Timestamp val_time = 1;
   Protocol protocol = 2;
}

Done.

doc/cryptography/DRKeyInfra.rst, line 297 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Just key?

Done.

doc/cryptography/DRKeyInfra.rst, line 321 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Isn't the type implied by the presence of src_host/dst_host? Otherwise, this enum needs a definition.

Done.

doc/cryptography/DRKeyInfra.rst, line 329 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

In the other protobuf definitions, this is usually represented as a string. It doesn't strike me as an ideal choice either, but perhaps consistency is still a plus. I wonder whether we need to worry about ambiguous address types (i.e. addresses of different types having same textual representation).

Done.

doc/cryptography/DRKeyInfra.rst, line 333 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Drop this misc; protobuf is extensible by design, adding in an unspecified field for possible later use is just unnecessary.

Done.

doc/cryptography/DRKeyInfra.rst, line 353 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

I think this example can safely be dropped; this will be covered by the SPAO and authenticated SCMP specs. Perhaps just add links.

Done.

mlegner

Reviewed 1 of 2 files at r2.
Reviewable status: 1 of 3 files reviewed, 16 unresolved discussions (waiting on @JordiSubira and @matzf)

fstreun

Reviewable status: 1 of 3 files reviewed, 18 unresolved discussions (waiting on @JordiSubira and @matzf)

doc/cryptography/drkey.rst, line 78 at r2 (raw file):

:math:`CS_B`.

**AS-host (Level 2)**

I think this is supposed to be host-AS?

doc/cryptography/drkey.rst, line 86 at r2 (raw file):

infrastructure.

**host-AS (Level 2)**

and this is supposed to be AS-host?

JordiSubira

Reviewable status: 1 of 3 files reviewed, 18 unresolved discussions (waiting on @fstreun, @JordiSubira, and @matzf)

doc/cryptography/drkey.rst, line 78 at r2 (raw file):

Previously, fstreun wrote…

I think this is supposed to be host-AS?

Indeed, my bad. Thank you for pointing that out.

doc/cryptography/drkey.rst, line 86 at r2 (raw file):

Previously, fstreun wrote…

and this is supposed to be AS-host?

Ditto

matzf

Reviewed 1 of 2 files at r1, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @fstreun and @JordiSubira)

doc/cryptography/drkey.rst, line 31 at r4 (raw file):

     - key derivation function using key K and taking X as an input
   * - :math:`PRF_K (X)`
     - pseudorandom function using key K and taking X as an input

The choice of this PRF should be specified somewhere (as AES-CBC MAC). A related detail that's not currently defined is the exact, byte-for-byte, input format to the PRF (width of the type and protocol fields, representation of ISD-AS and address type/length). Perhaps add an additional subsection to "Derivation scheme" where we specify these details for the relevant derivation steps.

doc/cryptography/drkey.rst, line 48 at r4 (raw file):

(CS). The certificate service leverages efficient key derivation and
pseudorandom functions to derive keys in the hierarchy from top
to bottom.

Nit, maybe combine this with "Overview"?

Also, I think this section could introduce the ideas of exchanging of top-level keys and local derivation of the key hierarchy -- it's all defined below, but we can mention it in this introduction.

doc/cryptography/DRKeyInfra.rst, line 175 at r1 (raw file):

Previously, JordiSubira wrote…

I included it within the exchange sections, let me know what you think.

👍

doc/cryptography/DRKeyInfra.rst, line 251 at r1 (raw file):

Previously, JordiSubira wrote…

I also thought about that and I agree that if we consider the prefetcher as part of the specification (before it wasn't) we can use specify this expected behavior. Even, for the implementation I'd say that we can consider even not implementing this "spreading" mechanism at the moment. The prefetcher will be implemented as a periodic.Runner and I guess we could assume that different Runners (for different ASes) start at different time, i.e. they will request Lvl1 keys also at different points in time.

👍

matzf

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @fstreun and @JordiSubira)

doc/cryptography/drkey.rst, line 1 at r4 (raw file):

**************************************************

Note that after renaming the file, you need to update the corresponding entry in cryptography/index.rst

JordiSubira

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @fstreun, @JordiSubira, and @matzf)

doc/cryptography/drkey.rst, line 31 at r4 (raw file):

Previously, matzf (Matthias Frei) wrote…

The choice of this PRF should be specified somewhere (as AES-CBC MAC). A related detail that's not currently defined is the exact, byte-for-byte, input format to the PRF (width of the type and protocol fields, representation of ISD-AS and address type/length). Perhaps add an additional subsection to "Derivation scheme" where we specify these details for the relevant derivation steps.

I can add the section, no problem. However we should also include the choice for the KDF and "byte-for-byte" input format for it, shouldn't we?

Related to that, do you (or anyone else in the discussion) have a strong opinion about prepending the master secret length? I inherited this input format and I am not sure about its purpose. PBKDF2 internally applies HMAC that does seem resistant to length extension attacks [1] (if that was the idea behind it).

[1] https://cseweb.ucsd.edu/~mihir/papers/kmd5.pdf

JordiSubira

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @fstreun, @JordiSubira, and @matzf)

doc/cryptography/drkey.rst, line 31 at r4 (raw file):

Previously, JordiSubira wrote…

I can add the section, no problem. However we should also include the choice for the KDF and "byte-for-byte" input format for it, shouldn't we?

Related to that, do you (or anyone else in the discussion) have a strong opinion about prepending the master secret length? I inherited this input format and I am not sure about its purpose. PBKDF2 internally applies HMAC that does seem resistant to length extension attacks [1] (if that was the idea behind it).

[1] https://cseweb.ucsd.edu/~mihir/papers/kmd5.pdf

Revisiting the message format we have defined (host are variable-size strings at the moment) I guess if we want to use CBC MAC ( which is a "good" PRF under fixed length input space) we should fix the size for the host field (I do not know if that could be a problem). Maybe I am missing something in my reasoning...

marcfrei · 2021-09-08T19:23:21Z

doc/cryptography/drkey.rst, line 121 at r4 (raw file):

   * - :math:`K_{A:H_A,B}^{protocol}`
     - :math:`PRF_{K_{A,B}^{protocol}}(type||H_A)`
     - host-AS (Level 2)

host-AS key (Level 2)?

marcfrei · 2021-09-08T19:24:03Z

doc/cryptography/drkey.rst, line 124 at r4 (raw file):

   * - :math:`K_{A:H_A,B:H_B}^{protocol}`
     - :math:`PRF_{K_{A:H_A,B}^{protocol}}(type||H_B)`
     - host-host (Level 3)

host-host key (Level 3)?

marcfrei · 2021-09-08T19:25:21Z

doc/cryptography/drkey.rst, line 165 at r4 (raw file):

   * - :math:`K_{A:H_A,B}^{protocol}`
     - :math:`PRF_{K_{A,B}}(protocol||type||H_A)`
     - host-AS (Level 2)

host-AS key (Level 2)?

marcfrei · 2021-09-08T19:25:54Z

doc/cryptography/drkey.rst, line 168 at r4 (raw file):

   * - :math:`K_{A:H_A,B:H_B}^{protocol}`
     - :math:`PRF_{K_{A:H_A,B}^{protocol}}(type||H_B)`
     - host-host (Level 3)

host-host key (Level 3)?

marcfrei · 2021-09-08T19:29:34Z

doc/cryptography/drkey.rst, line 182 at r4 (raw file):

must not overlap. Thus, a secret value is associated with exactly one epoch.

In the design, every AS can define different epoch lengths for each

Remove the prefix In the design, ?

marcfrei · 2021-09-08T19:36:17Z

doc/cryptography/drkey.rst, line 241 at r4 (raw file):

Even though Level 0/2/3 key exchange happens within the same AS (i.e. intra-AS communication),
the protocol should establish a secure channel. This would avoid that unintended hosts in the

Remove unintended?

marcfrei · 2021-09-08T19:37:43Z

doc/cryptography/drkey.rst, line 242 at r4 (raw file):

Even though Level 0/2/3 key exchange happens within the same AS (i.e. intra-AS communication),
the protocol should establish a secure channel. This would avoid that unintended hosts in the
AS can eavesdrop on symmetric keys that are not intended to them.

"intended for"

marcfrei · 2021-09-08T19:39:52Z

doc/cryptography/drkey.rst, line 237 at r4 (raw file):

the `generic-protocol` hierarchy or to the protocol number for the `protocol-specific` derivation.

Level 0/2/3 level key establishment

Remove the second "level".

marcfrei · 2021-09-08T19:47:10Z

doc/cryptography/drkey.rst, line 320 at r4 (raw file):

      Protocol protocol_id = 1;
      // Point in time where requested DRKey is valid.
      Timestamp val_time = 2;

Use the same order for the fields protocol_id and val_time as in the other request types?

Plus use everywhere the same comment to describe the same type of field.

marcfrei · 2021-09-08T19:48:37Z

doc/cryptography/drkey.rst, line 331 at r4 (raw file):

    }

    message Lvl2Response{

Use the same order of fields as in the other response types?

marcfrei

Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @fstreun, @JordiSubira, and @matzf)

matzf

Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @fstreun and @JordiSubira)

doc/cryptography/drkey.rst, line 31 at r4 (raw file):

Previously, JordiSubira wrote…

Revisiting the message format we have defined (host are variable-size strings at the moment) I guess if we want to use CBC MAC ( which is a "good" PRF under fixed length input space) we should fix the size for the host field (I do not know if that could be a problem). Maybe I am missing something in my reasoning...

The KDF is not as critical as it is a local decision and does not need to be standardized. We can still suggest something though, I guess.

For CBC-MAC it's enough to include the length of the address field (probably best in the form of the length/type field as in the scion address header) in the first block, before the address.

JordiSubira

Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @fstreun, @marcfrei, and @matzf)

doc/cryptography/drkey.rst, line 1 at r4 (raw file):

Previously, matzf (Matthias Frei) wrote…

Note that after renaming the file, you need to update the corresponding entry in cryptography/index.rst

Done.

doc/cryptography/drkey.rst, line 31 at r4 (raw file):

Previously, matzf (Matthias Frei) wrote…

The KDF is not as critical as it is a local decision and does not need to be standardized. We can still suggest something though, I guess.

For CBC-MAC it's enough to include the length of the address field (probably best in the form of the length/type field as in the scion address header) in the first block, before the address.

👍

doc/cryptography/drkey.rst, line 121 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

host-AS key (Level 2)?

Done.

doc/cryptography/drkey.rst, line 124 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

host-host key (Level 3)?

Done.

doc/cryptography/drkey.rst, line 165 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

host-AS key (Level 2)?

Done.

doc/cryptography/drkey.rst, line 168 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

host-host key (Level 3)?

Done.

doc/cryptography/drkey.rst, line 182 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

Remove the prefix In the design, ?

Done.

doc/cryptography/drkey.rst, line 237 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

Remove the second "level".

Done.

doc/cryptography/drkey.rst, line 241 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

Remove unintended?

Done.

doc/cryptography/drkey.rst, line 242 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

"intended for"

Done.

doc/cryptography/drkey.rst, line 320 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

Use the same order for the fields protocol_id and val_time as in the other request types?

Plus use everywhere the same comment to describe the same type of field.

Done.

doc/cryptography/drkey.rst, line 331 at r4 (raw file):

Previously, marcfrei (Marc Frei) wrote…

Use the same order of fields as in the other response types?

Done.

matzf

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @fstreun, @JordiSubira, and @marcfrei)

doc/cryptography/drkey.rst, line 204 at r5 (raw file):

The derivation using CBC MAC is safe since all fields are of a fixed size,
expect for the host address whose length is prepended.

Nit, typo expect -> except

matzf

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @fstreun and @marcfrei)

JordiSubira · 2021-10-21T11:36:16Z

@marcfrei @fstreun I've seen there are some pending discussions, if you are happy with the responses/modifications could you please resolve them. Cheers!

marcfrei · 2021-10-21T11:45:46Z

marcfrei · 2021-10-21T11:52:56Z

fstreun

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)

oncilla

Reviewed 1 of 2 files at r5, 1 of 1 files at r6.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @JordiSubira)

doc/cryptography/drkey.rst, line 19 at r6 (raw file):

- EPIC

In the DRKey system, the key establishment is offloaded to the certificate service

the certificate service does not exist anymore.
Replace this with SCION Control Service

(please also update the rest of this document in this regard.)

doc/cryptography/drkey.rst, line 351 at r6 (raw file):

    // DRKeyLvl2Request encompasses 2nd and 3rd level key requests
    message Lvl2Request{

Why do we pack the 2nd and 3rd level key requests in the same message?
I would separate them, such that they can evolve independently over time.

oncilla

Reviewed 1 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @JordiSubira)

JordiSubira

Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @matzf and @oncilla)

doc/cryptography/drkey.rst, line 19 at r6 (raw file):

Previously, oncilla (Dominik Roos) wrote…

the certificate service does not exist anymore.
Replace this with SCION Control Service

(please also update the rest of this document in this regard.)

Done.

doc/cryptography/drkey.rst, line 351 at r6 (raw file):

Previously, oncilla (Dominik Roos) wrote…

Why do we pack the 2nd and 3rd level key requests in the same message?
I would separate them, such that they can evolve independently over time.

I'd rather change the naming for the keys, i.e. using "end-host keys" instead of Lvl2 and Lvl3, since both are keys that the end-host will request to the CS. The information within the request is what distinguishes between the three possible cases (AS-HOST, HOST-AS, HOST-HOST). In other words, Lvl3 key is another name for the HOST-HOST key. In case we really want to create a different message definition, I guess it would make more sense to have three, one for each case, rather than one for Lvl2 and Lvl3.

oncilla

Reviewed all commit messages.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @JordiSubira, @matzf, and @oncilla)

doc/cryptography/drkey.rst, line 351 at r6 (raw file):

Previously, JordiSubira wrote…

I'd rather change the naming for the keys, i.e. using "end-host keys" instead of Lvl2 and Lvl3, since both are keys that the end-host will request to the CS. The information within the request is what distinguishes between the three possible cases (AS-HOST, HOST-AS, HOST-HOST). In other words, Lvl3 key is another name for the HOST-HOST key. In case we really want to create a different message definition, I guess it would make more sense to have three, one for each case, rather than one for Lvl2 and Lvl3.

Yes, let's do this then. One request and one response per "end-host key".
I prefer being explicit in the type we want, rather then derive it implicitly from the fields that are set in the request.

JordiSubira

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @matzf and @oncilla)

doc/cryptography/drkey.rst, line 351 at r6 (raw file):

Previously, oncilla (Dominik Roos) wrote…

Yes, let's do this then. One request and one response per "end-host key".
I prefer being explicit in the type we want, rather then derive it implicitly from the fields that are set in the request.

Done.

oncilla

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @oncilla)

oncilla

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JordiSubira)

a discussion (no related file):
Thank you for the contribution.
This will be merged in the next open sourcing run.

JordiSubira

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @oncilla)

a discussion (no related file):

Previously, oncilla (Dominik Roos) wrote…

Thank you for the contribution.
This will be merged in the next open sourcing run.

Brilliant, thank you!

DRKey version 2 contains the following changes to DRKey, some of them breaking changes and not back-compatible: - New derivation scheme (as discussed in scionproto#4102) - New protobuf messages (as discussed in scionproto#4102) - Refactoring of SVStore (adds persistance in it) - Adding EKU check in presented certificates - Dynamic x509Cert/Key provider - Adding LRU Cache for the prefetcher - Metrics for DRKey - Global default epoch duration (configurable for debugging/testing purposes) This version does not include the following features: - Configurable epoch duration per AS+SV - Spreading Lvl1Key prefetching - Grace period

first draft DRKey in RST

803dfaf

mlegner reviewed Aug 20, 2021

View reviewed changes

matzf reviewed Aug 20, 2021

View reviewed changes

mlegner reviewed Aug 23, 2021

View reviewed changes

review update

ebe93fd

JordiSubira commented Aug 25, 2021

View reviewed changes

mlegner reviewed Aug 26, 2021

View reviewed changes

fstreun reviewed Aug 30, 2021

View reviewed changes

fix naming

a1a7e0e

JordiSubira commented Aug 30, 2021

View reviewed changes

rephrase key derivation + typo

dcf000d

matzf reviewed Sep 8, 2021

View reviewed changes

JordiSubira commented Sep 8, 2021

View reviewed changes

marcfrei reviewed Sep 8, 2021

View reviewed changes

matzf reviewed Sep 8, 2021

View reviewed changes

update pr

bc2eadd

JordiSubira commented Sep 13, 2021

View reviewed changes

matzf reviewed Sep 14, 2021

View reviewed changes

fix typo

74dce40

matzf reviewed Sep 14, 2021

View reviewed changes

JordiSubira changed the title ~~DRkey documentation~~ [doc] DRkey documentation Sep 14, 2021

JordiSubira changed the title ~~[doc] DRkey documentation~~ doc: DRkey documentation Sep 14, 2021

fstreun reviewed Oct 21, 2021

View reviewed changes

oncilla requested changes Nov 11, 2021

View reviewed changes

oncilla reviewed Nov 11, 2021

View reviewed changes

replacing CertServ with CS

e798707

JordiSubira commented Nov 15, 2021

View reviewed changes

oncilla requested changes Nov 15, 2021

View reviewed changes

modifying proto Lvl2/3 messages

ef9e972

JordiSubira commented Nov 16, 2021

View reviewed changes

oncilla reviewed Nov 16, 2021

View reviewed changes

JordiSubira mentioned this pull request Nov 16, 2021

DRKey version 2 netsec-ethz/scion#106

Merged

oncilla approved these changes Nov 17, 2021

View reviewed changes

JordiSubira commented Nov 17, 2021

View reviewed changes

lukedirtwalker closed this in 146b862 Nov 18, 2021

JordiSubira deleted the update_drkey_doc branch April 19, 2022 07:42

doc: DRkey documentation #4102

doc: DRkey documentation #4102

Conversation

JordiSubira commented Aug 12, 2021 • edited Loading

mlegner left a comment

Choose a reason for hiding this comment

matzf left a comment

Choose a reason for hiding this comment

mlegner left a comment

Choose a reason for hiding this comment

JordiSubira left a comment • edited Loading

Choose a reason for hiding this comment

mlegner left a comment

Choose a reason for hiding this comment

fstreun left a comment

Choose a reason for hiding this comment

JordiSubira left a comment

Choose a reason for hiding this comment

matzf left a comment

Choose a reason for hiding this comment

matzf left a comment

Choose a reason for hiding this comment

JordiSubira left a comment

Choose a reason for hiding this comment

JordiSubira left a comment

Choose a reason for hiding this comment

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei commented Sep 8, 2021

marcfrei left a comment

Choose a reason for hiding this comment

matzf left a comment

Choose a reason for hiding this comment

JordiSubira left a comment

Choose a reason for hiding this comment

matzf left a comment

Choose a reason for hiding this comment

matzf left a comment

Choose a reason for hiding this comment

JordiSubira commented Oct 21, 2021

marcfrei commented Oct 21, 2021

marcfrei commented Oct 21, 2021

fstreun left a comment

Choose a reason for hiding this comment

oncilla left a comment

Choose a reason for hiding this comment

oncilla left a comment

Choose a reason for hiding this comment

JordiSubira left a comment

Choose a reason for hiding this comment

oncilla left a comment

Choose a reason for hiding this comment

JordiSubira left a comment

Choose a reason for hiding this comment

oncilla left a comment

Choose a reason for hiding this comment

oncilla left a comment

Choose a reason for hiding this comment

JordiSubira left a comment

Choose a reason for hiding this comment

JordiSubira commented Aug 12, 2021 •

edited

Loading

JordiSubira left a comment •

edited

Loading