Protocol framing improvement

[jakubtrnka]

Summary

Currently suggested framing of the stratum-v2 protocol with noise-security layer is sub-optimal.

First, plaintext stratum-v2 messages are framed as follows https://github.com/stratum-mining/sv2-spec/blob/main/03-Protocol-Overview.md#32-framing

+----------------+-------------+---------------------------------------------------------------------------------------+
| Protocol Type  | Byte Length | Description                                                                           |
+----------------+-------------+---------------------------------------------------------------------------------------+
| extension_type | U16         | Unique identifier of the extension describing this protocol message.                  |
|                |             | ...                                                                                   |
+----------------+-------------+---------------------------------------------------------------------------------------+
| msg_type       | U8          | Unique identifier of the extension describing this protocol message                   |
+----------------+-------------+---------------------------------------------------------------------------------------+
| msg_length     | U24         | Length of the protocol message, not including this header                             |
+----------------+-------------+---------------------------------------------------------------------------------------+
| payload        | BYTES       | Message-specific payload of length msg_length. ...                                    |
+----------------+-------------+---------------------------------------------------------------------------------------+

That is, we have 6-bytes header followed some specific number of bytes of payload.

This in itself is fine.

Now, the encryption layer is applied where the serialized stratum-message is encrypted with AEAD cipher and framed again using B0_64 primitive. https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md#456-transport-message-encryption-and-format

NOISE_FRAME
+-------------+-----------+--------------------------------------------------------------------------------------------+
| Field Name  | Data Type | Description                                                                                |
+-------------+-----------+--------------------------------------------------------------------------------------------+
| ciphertext  | B0_64K    | AEAD ciphertext including 16 bytes MAC                                                     |
+-------------+-----------+--------------------------------------------------------------------------------------------+
Message length: <Plaintext length> + 18 bytes = <Plaintext length> + <MAC length> + <Type length prefix>

Maximum message length = 65537 bytes
Maximum ciphertext length = 65535 bytes
Maximum plaintext length 65519 bytes

Why is this bad?

• This approach reveals the individual frame lengths in the TCP-traffic, since the B0_64 contains two-byte length prefix which is sent in clear-text and is not even protected by the cryptographic MAC of the encryption scheme.
• Makes an impression of architecture stratum-v2 message stream on top of optional generic encrypted connection. The optionality is explicitly mentioned: Using the handshake protocol to establish secured communication is optional on the local network. I think this is bad as well. There is no real benefit in using plaintext stratum-v2, because the binary protocol is not human readable anyways. For debugging purposes, noise-proxy can be used or session keys can be dumped and stream decrypted, similarly as SSLKEYLOGFILE is used for debugging TLS connections. Therefore, in my opinion the standard SHOULD NOT allow using an insecure stratum-v2.
• It has redundant frame-length information.
  • First, two-byte length field (in plaintext) in the noise frame is read
  • the value is parsed as Little-Endian length and that number of bytes is then read next and decrypted. The decrypted plaintext represents partial stratum-v2 message (might be full message if it fits into the limit of 65535 bytes - and currently always is)
  • The decrypted bytes (partial messages) from individual noise-frames are meant to be concatenated, which results in original plaintext-stream that is parsed as stratum-v2 communication.

Due to the last point it is quite difficult and unnecessarily complex to work with large stratum messages (note that Stratum-v2 frames may be up to 2²⁴ bytes long, while noise-frames only 2¹⁶ - 1 bytes long. Moreover no implementation currently exists that is able to parse sv2-messages longer than 65535 bytes (including header), so currently supported use-cases doesn't even need the length-field in the stratum-v2 message (I'm not sure if anybody is interested in implementing the full-generic case for arbitrarily-long messages).

Proposed solution:

Make the security an integral part of stratum framing as follows:

+--------------------------------------------------+-------------------------------------------------------------------+
| Extended noise header                            | Encrypted stratum-message payload                                 |
+--------------------------------------------------+-------------------+-------------------+---------------------------+
| Header AEAD ciphertext                           | Noise block 1     | Noise block 2     | Last Noise block          |
| 22 Bytes                                         | 65535 Bytes       | 65535 Bytes       | 17 - 65535 Bytes          |
+----------------------------------------+---------+-----------+-------+-----------+-------+---------------+-----------+
| Encrypted Stratum message Header       | MAC     | ct_pld_1  | MAC_1 | ct_pld_2  | MAC_2 | ct_pld_rest   | MAC_rest  |
| 6 Bytes                                | 16 B    | 65519 B   | 16 B  | 65519 B   | 16 B  | 1 - 65519 B   | 16 Bytes  |
+================+==========+============+=========+===========+=======+===========+=======+===============+===========+
| extension_type | msg_type | pld_length | <padd   | pt_pld_1  | <padd | pt_pld_2  | <padd | pt_pld_rest   | <padding> |
| U16            | U8       | U24        |   ing>  | 65519 B   |  ing> | 65519 B   |  ing> | 1 - 65519 B   |           |
+----------------+----------+------------+---------+-------------------------------------------------------------------+

Serialized stratum-v2 body (payload) is split into 65519-byte chunks and encrypted to form 65535-bytes AEAD ciphertexts,
where `ct_pld_N` is the N-th ciphertext block of payload and `pt_pld_N` is the N-th plaintext block of payload.

• This structure would hide and protect the frame length field, making the TCP-transcript fully uniform and random looking.
• It largely simplifies implementation of large-message encoding.
• Doesn't contain redundant length information. Because parsers work as follows:
  1. read exactly 22 Bytes.
  2. Decrypt as an Extended Noise-Header or exit with error.
  3. parse Stratum Message Header as hdr
  4. read next hdr.pld_length bytes as a payload
  5. decrypt the payload in 65535-bytes chunks which is the maximum that noise-protocol allows in a single DecryptWithAd call

[jakubtrnka]

Btw, note that there is a typo in the sect. 3 of the spec.

+----------------+-------------+---------------------------------------------------------------------------------------+
| Protocol Type  | Byte Length | Description                                                                           |
+----------------+-------------+---------------------------------------------------------------------------------------+

should actually be

+----------------+-------------+---------------------------------------------------------------------------------------+
| Field name     | Byte Length | Description                                                                           |
+----------------+-------------+---------------------------------------------------------------------------------------+

or similar

[Fi3]

I think this would improve the Sv2 protocol. I guess that Sv2 do not require encryption on local networks in order to have less complex mining device. I don't think this is a good enough reason to nack this proposal but I would like to hear someone else opinion on it.

About SRI specific pro e cons, implement this proposal will likely make the codebase less complex but some refactor will be needed because that and because it do not make sense to go with the first release without this improvement is very likely that we must delay first release. Again I do not think that this is a good enough reason to nack this proposal.

Below sentence is not true I guess:

Moreover no implementation currently exists that is able to parse sv2-messages longer than 65535 bytes

https://github.com/stratum-mining/stratum/blob/24ec4d6e66c5650b2d2d73222f000214fca72869/protocols/v2/codec-sv2/src/decoder.rs#L112

[jakubtrnka]

Below sentence is not true I guess:

My apology. You guys did better job than I thought. I've got a TODO in my code for that case for almost two years, lol.

Regarding the insecure mode - that was rather a side-note. Let's keep the insecure mode as an option, for now.

Main question was whether the proposed framing and encryption format would be something we agree on.

[Fi3]

Main question was whether the proposed framing and encryption format would be something we agree on.

for me yes

[pavlenex]

This one was done in #40