Add specification for epoch data keys #5

schwabe · 2024-09-24T15:49:40Z

No description provided.

openvpn-wire-protocol.xml

mfil · 2024-09-25T16:39:06Z

openvpn-wire-protocol.xml

+
+    </references>
+    <references title='Informative References'>
+      <reference anchor="AEAD-LIMITS" target="http://www.isg.rhul.ac.uk/~kp/TLS-AEbounds.pdf">


I get a 404 at this link.

The new link works!

mfil · 2024-09-25T18:07:45Z

openvpn-wire-protocol.xml

+          The epoch key generation is inspired by the TLS key generation and
+          <xref target="RFC5869">HMAC-based Key Derivation Function (HKDF)</xref>
+          and the definition of HDKF-Expand-Label in TLS 1.3.


Might as well reference RFC 8446 here too.

Has been added!

mfil · 2024-09-25T18:12:43Z

openvpn-wire-protocol.xml

+          For OpenVPN the epoch 0 key E0 is the OpenVPN key, for any further key, the key derivation is
+          <artwork> E_N+1 = HKDF-Expand-Label(E_N, "OpenVPN data key update", "", 256)</artwork>
+
+
+          The per epoch data channel key is derived via
+          <artwork> K_i = HKDF-Expand-Label(E_i, "OpenVPN epoch key", "", 256) </artwork>


I'm pretty sure we don't want 256-byte keys, this should be 32 for a 256-bit key.

(We're both having trouble with bits and bytes lately it seems)

Actually this time, the 256 is intended as it is the size of the OpenVPN key block. I will add some more text to describe why these are so big.

flichtenheld

Some language review

flichtenheld · 2024-10-02T15:02:49Z

openvpn-wire-protocol.xml

+      <ttcol>packet size</ttcol>
+      <ttcol>Maximum number of packets</ttcol>
+      <c> 64 </c>
+      <c> 29.4 </c>


Missing 2^?

flichtenheld · 2024-10-02T15:03:51Z

openvpn-wire-protocol.xml

+      The receiver should count the number of packets that have been received but failed verification.
+      The number of packets that fail verification and we can tolerate is again specified differently
+      for DTLS 1.3 and the data usage limit document. For OpenVPN we set the number to 2^36 for all
+      ciphers, the same DTLS 1.3.  An implementation may choose a lower number.


"same as DTLS 1.3"

flichtenheld · 2024-10-02T15:04:54Z

openvpn-wire-protocol.xml

+          <xref target="RFC5869">HMAC-based Key Derivation Function (HKDF)</xref>
+          and the definition of HKDF-Expand-Label in TLS 1.3.
+
+          For OpenVPN the epoch 0 key E0 are the first 32 the OpenVPN key, for any further key, the key derivation is


"are the first 32 bytes of the OpenVPN key. For any"?

flichtenheld · 2024-10-02T15:06:53Z

openvpn-wire-protocol.xml

+} OvpnLabel;
+          </artwork>
+
+          Keep in mind that the &quot; characters, like in TLS 1.3 are part of the label prefix.


"characters are part of the label prefix, like in TLS 1.3."

flichtenheld · 2024-10-02T15:07:22Z

openvpn-wire-protocol.xml

+      </section>
+      <section title="Epoch key usage">
+        <t>
+          When sending packets, a peer will encrypt packet until the limit for the epoch K_i is reach. At this point,


flichtenheld · 2024-10-02T15:13:18Z

openvpn-wire-protocol.xml

+          packets is lost, the implementation will still be able to authenticate the packets.
+        </t>
+        <t>
+          Allowing multiple epochs keys, requires the receiving side to keep multiple keys to able to decrypt and


superfluous comma

flichtenheld · 2024-10-02T15:13:44Z

openvpn-wire-protocol.xml

+          verify packets with different epochs. An attacker can send a packet with a epoch that is a lot higher than
+          the current epoch. In order to avoid needing to the generate/keep arbitrary epoch decryption keys, the window
+          of future decryption keys that is acceptable should be limited.  At the same time, the window needs to be large
+          enough that a long packet loss that causes all packet to be dropped (e.g. a topology change) does not


"all packets"

flichtenheld · 2024-10-02T15:15:53Z

openvpn-wire-protocol.xml

+        <t>
+          The packet id is split into 16 bit bits of epoch and 48bit of counter for
+          that epoch. Each epoch uses a different a encryption/decryption key and a
+          different implicit_iv. The epoch select the key to use for decryption.


flichtenheld · 2024-10-02T15:16:19Z

openvpn-wire-protocol.xml

+        <t>
+          The epoch_counter of the packet_id together with the implicit IV for the epoch
+          forms the IV for decryption. The epoch_counter is also used for the replay
+          protection.  Implementation are encouraged to a have a per epoch replay protection.


"Implementations"

flichtenheld · 2024-10-02T15:16:52Z

openvpn-wire-protocol.xml

+          forms the IV for decryption. The epoch_counter is also used for the replay
+          protection.  Implementation are encouraged to a have a per epoch replay protection.
+          Using the whole packet_id as a flat 64bit counter for replay protection will work as
+          well but will reject reordered packets from the older epoch when a (valid) packet if


broken sentence

syzzer · 2024-11-02T20:23:57Z

openvpn-wire-protocol.xml

-            For details about this opcode see sections about <xref target="dataaead">AEAD</xref>
-            and <xref target="datacbc">CBC/OFB/CTR</xref> packet format.
+            For details about this opcode see the sections <xref target="dataaead">AEAD</xref>,
+            <xref target="data_epoch">AEAD</xref>,  and <xref target="datacbc">CBC/OFB/CTR</xref>


Reference ref text pasto

syzzer

Some comments from a quick scan, no full review (yet).

syzzer · 2024-11-02T20:26:09Z

openvpn-wire-protocol.xml

@@ -1462,7 +1463,8 @@ struct key_exchange {
            <li>bit 7: The client is capable of sending exit notification via control channel using <xref target="exitcc">EXIT</xref> message. The client is accpting the protocol-flags pushed option for the EKM capability</li>
            <li>bit 8: The client is capable of accepting <xref target="authfailed">AUTH_FAILED,TEMP</xref> messages.</li>
            <li>bit 9: The client is capable of dynamic tls-crypt</li>
-            <li>bit 10: The client is capable of AEAD tag at the end of a data channel packet and capable of using 64bit packet counters for AEAD ciphers.</li>
+            <li>bit 10: The client is capable of using the epoch data format that uses AEAD tag at the end
+            has a 64bit packet counters to allow for epoch ciphers.</li>


Sentence does not compile. Remnant of old text?

syzzer · 2024-11-02T20:28:27Z

openvpn-wire-protocol.xml

+      existing term key-id and since <xref target="RFC9147">DTLS 1.3</xref> uses the same terminology.
+    </t>
+    <t>
+      A sender should calculate a safe amount of packets to send/number of encryption that can be done.


s/encryption/encryptions/

syzzer · 2024-11-02T20:46:25Z

openvpn-wire-protocol.xml

+    </texttable>
+    <t>
+      Instead of (implicitly) only tracking only the number of packets, an implementation can also instead
+      track the number of plain text blocks s and the number of packets q:


Good point. It might actually make even more sense to present this first, and make the hard requirement for implementations to not exceed the limits you present here. The above section could then serve as an example of a possible implementation.

Yes, I will do that. My current implementation actually tracks the plaintext blocks and number of packets as that is easier to track then do the whole figuring out the maximum size we are willing to encrypt thing.

syzzer · 2024-11-02T20:47:14Z

openvpn-wire-protocol.xml

+      <artwork>q &le; (p^(1/2) * 2^(129/2) - 1) / (L + 1)></artwork>
+
+      gives us basically the same limit that TLS imposes:
+      <artwork> q &le; 24^(24.9999) </artwork>


2^24.9999 ?

syzzer · 2024-11-02T20:49:24Z

openvpn-wire-protocol.xml

+      necessary.
+    </t>
+    <t>
+      Implementations have to ensure to calculate the limit with the actual maximum size that they are


Might need some clarification that this is only relevant for implementations that do not track the actual amount of plain text blocks encrypted.

syzzer · 2024-11-02T20:53:05Z

openvpn-wire-protocol.xml

+          and the definition of HKDF-Expand-Label in TLS 1.3.
+
+          For OpenVPN the epoch 1 key E_1 are the first 32 the OpenVPN data key. Note that unlike when
+          when using the OpenVPN key directly for encrypting/decryption, E_1 is the same for both direction


This might need some extra wording to make it immediately clear that this does not mean we use the same key for both directions. Something like "E_1 is used for key expansion only and therefore the same [..]" perhaps?

Yeah. During the implementation I changed this to actually use different epoch keys for both direction as that was better suited and I wanted to eliminate that strange "generate a 256 byte blob and slice into key/iv" stuff as that is "fine" for internally in OpenVPN itself but I don't want that to bleed into DCO implmentations. They should rather do something saner.

syzzer · 2024-11-02T21:01:11Z

openvpn-wire-protocol.xml

+        </t>
+        <t>
+          Each peer is expected to hold the current epoch key K_i, the previous K_i-1 and a number of
+          future keys K_i+1, K_i+2,....,K_i+j. The number of future key j depends on the expected possible


future keyS

syzzer · 2024-11-02T21:06:14Z

openvpn-wire-protocol.xml

+          Each peer is expected to hold the current epoch key K_i, the previous K_i-1 and a number of
+          future keys K_i+1, K_i+2,....,K_i+j. The number of future key j depends on the expected possible
+          data rate of the implementation and needs to be large enough that even when a reasonable amount of
+          packets is lost, the implementation will still be able to authenticate the packets.


This seems to be an implementation consideration for implementations that are not able to derive future keys on demand from the data channel code (like DCO?). Consider being explicit about that.

And, doesn't the same hold for old keys? If you expect to roll over often, and want to be able to gracefully accept out-of-order packets, you might want to keep more old keys. Or is that not a valid use case?

Yeah, this is a tradeoff that I will explicility mention here. Not being able to decrypt an old keys only results a packet loss. Not being able to decrypt a future key results in complete loss of the connection.

openvpn-wire-protocol.xml

mfil · 2024-11-15T17:00:16Z

openvpn-wire-protocol.xml

+          As addition a peer cannot regard apparent overuse of a send key by other the other peer as error as the
+          limit can change if new information is available but the peer MAY move to a higher sending key epoch of its
+          own to try to trigger the same on the peer as outlined in the previous paragraph.


I'm not sure I understand this. In the future, there might be better bounds for GCM mode and we update our RFC to reflect them, and old implementations need to handle that situation?

Yes, basically the recommendation of the limits RFC is the same. If you see a peer using more packets than we currently consider safe, you have two options

error out and kill connection/drop packets

ignore that the peer sent too many packets and continue decrypting the packets.

We choose option 2 here, so that if it turns out that the limits were too pessimistic, we can use better limits in the future and don't have to fall back to the lowest limits that we ever put into a client/server.

openvpn-wire-protocol.xml

Signed-off-by: Arne Schwabe <[email protected]>

schwabe force-pushed the schwabe/epoch_data_keys branch 2 times, most recently from 560eebb to 198c1b9 Compare September 25, 2024 10:51

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

rzvncj reviewed Sep 25, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

schwabe force-pushed the schwabe/epoch_data_keys branch from 198c1b9 to 3643c9a Compare September 25, 2024 13:49

mfil reviewed Sep 25, 2024

View reviewed changes

schwabe force-pushed the schwabe/epoch_data_keys branch 3 times, most recently from 6cfb3e4 to 49d7cb2 Compare September 30, 2024 08:48

flichtenheld reviewed Oct 2, 2024

View reviewed changes

schwabe force-pushed the schwabe/epoch_data_keys branch 2 times, most recently from 9ada2e3 to 24e0ebe Compare October 22, 2024 14:55

syzzer reviewed Nov 2, 2024

View reviewed changes

schwabe force-pushed the schwabe/epoch_data_keys branch 2 times, most recently from 3471fa0 to fa0e65e Compare November 11, 2024 02:00

lstipakov reviewed Nov 13, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

lstipakov reviewed Nov 13, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

lstipakov reviewed Nov 13, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

schwabe force-pushed the schwabe/epoch_data_keys branch from fa0e65e to a0b85e3 Compare November 13, 2024 12:00

mfil reviewed Nov 15, 2024

View reviewed changes

mfil reviewed Nov 18, 2024

View reviewed changes

openvpn-wire-protocol.xml Show resolved Hide resolved

dsommers reviewed Nov 20, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

dsommers reviewed Nov 20, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

dsommers reviewed Nov 20, 2024

View reviewed changes

openvpn-wire-protocol.xml Outdated Show resolved Hide resolved

schwabe force-pushed the schwabe/epoch_data_keys branch from a0b85e3 to 34fa086 Compare November 20, 2024 16:13

Add specification for epoch data keys

b29639f

Signed-off-by: Arne Schwabe <[email protected]>

schwabe force-pushed the schwabe/epoch_data_keys branch from 34fa086 to b29639f Compare November 22, 2024 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add specification for epoch data keys #5

Add specification for epoch data keys #5

schwabe commented Sep 24, 2024

mfil Sep 25, 2024

mfil Nov 19, 2024

mfil Sep 25, 2024

mfil Nov 19, 2024

mfil Sep 25, 2024

schwabe Sep 26, 2024

flichtenheld left a comment

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

flichtenheld Oct 2, 2024

syzzer Nov 2, 2024

syzzer left a comment

syzzer Nov 2, 2024

syzzer Nov 2, 2024

syzzer Nov 2, 2024

schwabe Nov 4, 2024

syzzer Nov 2, 2024

syzzer Nov 2, 2024

syzzer Nov 2, 2024

schwabe Nov 4, 2024

syzzer Nov 2, 2024

syzzer Nov 2, 2024

schwabe Nov 4, 2024

mfil Nov 15, 2024

schwabe Nov 19, 2024

Add specification for epoch data keys #5

Are you sure you want to change the base?

Add specification for epoch data keys #5

Conversation

schwabe commented Sep 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flichtenheld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syzzer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment