bip324: Remove garbage authentication packet (breaking change) #1498

pull real-or-random wants to merge 1 commits into bitcoin:master from real-or-random:202309-0324-garbauth changing 1 files +33 −28
  1. real-or-random commented at 1:30 pm on September 24, 2023: contributor

    by merging it with the version packet. Or more accurately, by merging it with the first packet sent after garbage termination, which may be a decoy packet or the version packet.

    The new protocol simplifies implementations:

    • A protocol state machine won’t need separate states for garbage authentication and version phases.
    • The special case of “ignoring the ignore bit” is removed.
    • The freedom to choose the contents of the garbage authentication packet is removed. This simplifies testing.

    The reason for having a separate garbage authentication packet was to materialize the separation of the key exchange phase and version negotiation phase even in the bytestream on the wire. However, this is not necessary, and arguably, these phases are still properly separated: Since the AEAD will ensure that AAD (=garbage) is checked before looking at the contents (=version), the peers won’t interpret version data before having authenticated the garbage.

  2. real-or-random marked this as a draft on Sep 24, 2023
  3. real-or-random commented at 1:31 pm on September 24, 2023: contributor
  4. real-or-random referenced this in commit 22d3f610d4 on Sep 24, 2023
  5. real-or-random referenced this in commit 796d21cdfd on Sep 24, 2023
  6. real-or-random referenced this in commit 8753b6af9b on Sep 24, 2023
  7. sipa commented at 4:34 pm on September 24, 2023: member

    ACK, though I want to wait for some more opinions - the BIP is fine as is, and I’m not sure whether this minor improvement is worth the breaking change at this point.

    For a bit of history, this was a conscious decision earlier, to separate the garbage phase from the version negotiation phase. But arguably, the result isn’t all that consistent either. The idea was to use the “packet” interface for everything after garbage terminator, but the abstraction is not perfect: packets have a decoy bit, but the decoy bit of the garbage authentication is ignored. In retrospect, seeing both specification and (one) implementation, it appears simpler to merge the auth packet and version packet.

    As for conceptual way of looking at it, I believe this makes sense:

    • We see the packet abstraction as consisting of two layers:
      • The lower-level length encoding + aead
      • The higher-level notion of header byte and decoy packets.
    • The change here is moving the garbage auth functionality to the lower-level layer: whenever additional authentication is needed, the first packet (at this layer decoy or not does not matter, so the first whether that’s a decoy or not) after it does that authentication. And the higher-level header/decoy functionality remains for version and application level packets, where this feature makes sense.
  8. naumenkogs commented at 12:28 pm on September 25, 2023: member

    ACK I reviewed changes to the BIP and changes to the codebase (bitcoin#28525). I think at this point, it’s worth breaking the current version, it makes everything cleaner and better (although I see the intuition behind the initial logic).

    Either peer may send such decoy packets at any point after this.

    Should be clearer saying that a garbage authentication package could be a decoy package (answering “after which point exactly?”).

    So the first two phases together, jointly called the handshake, comprise just 1.5 roundtrips:

    Kinda confusing now that garbage authentication is not there.

  9. real-or-random force-pushed on Sep 25, 2023
  10. real-or-random commented at 1:04 pm on September 25, 2023: contributor

    Either peer may send such decoy packets at any point after this.

    Should be clearer saying that a garbage authentication package could be a decoy package (answering “after which point exactly?”).

    Rephrased and restructured this to improve clarity.

    So the first two phases together, jointly called the handshake, comprise just 1.5 roundtrips:

    Kinda confusing now that garbage authentication is not there.

    Happy to improve here, but I can’t follow. It’s still 3 phases altogether, and the first two are 1.5 roundtrips. What do you find confusing?

  11. real-or-random commented at 1:05 pm on September 25, 2023: contributor

    ACK I reviewed changes to the BIP and changes to the codebase (bitcoin#28525)

    Want to leave a review or (Concept) ACK also in https://github.com/bitcoin/bitcoin/pull/28525 then? :)

  12. naumenkogs commented at 8:33 am on September 26, 2023: member

    Happy to improve here, but I can’t follow. It’s still 3 phases altogether, and the first two are 1.5 roundtrips. What do you find confusing?

    It was not immediately clear why garbage authentication is not in the list following comprise just 1.5 roundtrips. Now I agree that it doesn’t add much value in that description.

  13. ajtowns commented at 9:00 am on September 26, 2023: contributor

    Concept ACK – fewer states seems like a win, and it seems like the simpler this is, the more easily it could be used for other applications

    The 1.5 round trip handshake could arguably be more obscure if described/implemented as:

    • the initiator sends public key + garbage-part-1
    • the responder sends public key + garbage + garbage terminator + version packet + decoy packet
    • the initiator sends garbage-part-2 + garbage terminator + version packet + decoy packet

    (otherwise the “garbage-terminator + version packet” may be identifiable as a fixed size response?)

    Does it make sense to have a decoy packet before the version packet rather than after it? The version packet tells you how to interpret the header, so it seems slightly odd to be parsing headers before you’ve actually figured out what the header format is.

    Perhaps it would it be simpler but still workable to just have the first packet after the garbage terminator always be the version packet, and have its entire contents (with no header) be interpreted as the version negotiation? In that case, an initial “empty” version message could literally be empty, with no header and no content. In that case after the first version message, you’d pass every message’s content to a version-specific decoder that splits out the header, and routes the body to the appropriate application handler.

    (In order for the responder to be able to append decoy packets before receiving the initiator’s version packet, you’d still need to guarantee that packets with the high-bit of the first byte are ignored no matter what version ends up being negotiated)

    It might be slightly clearer to make the pseudocode more stateful as far as the aad info goes, ie drop it as a parameter to v2_enc_packet and v2_receive_packet and instead initialize it based on peer.sent_garbage and peer.recv_garbage, and clear them after first use?

  14. real-or-random referenced this in commit f93f8b457f on Sep 26, 2023
  15. real-or-random commented at 11:40 am on September 26, 2023: contributor

    The 1.5 round trip handshake could arguably be more obscure if described/implemented as:

    Just for my understanding: You’re saying that the handshake could look more obscure to wiretappers, right? And not that the description of the handshake in the BIP would be more obscure to readers? ;)

    (otherwise the “garbage-terminator + version packet” may be identifiable as a fixed size response?)

    I don’t think so. The “garbage-terminator + version packet” is always preceded by some form “garbage”. So if you want to get rid of the fixed-size response, you can also just send garbage. And yes, if you need more bytes, you can always send more decoys after the version packet, but I don’t think this needs to be in the handshake description. (You can always send decoys.)

    Does it make sense to have a decoy packet before the version packet rather than after it? The version packet tells you how to interpret the header, so it seems slightly odd to be parsing headers before you’ve actually figured out what the header format is.

    I agree that it seems odd to parse headers before you’ve agreed on a version. And @sipa had similar remarks when we talked about this somewhere else. The thing is that we bootstrap communications, so we build up the layers step by step. And yes, you can argue that at the point when we’ve just received the garbage terminator, we have not agreed on a header format yet.

    And I agree that the possibility to send decoy packets before the version packet doesn’t really add functionality. Instead of ignoring the ignore bit (what we do in the current BIP), we could also get rid of this by dropping the header byte entirely (what you suggest). Another way is to simply disallow packages where the ignore bit is set. Though, I don’t believe that this will make things simpler. It’s adding back a special rule again.

    So I think if we agree that it makes sense to drop the explicit garbage authentication packet and get rid of one state in the state machine, then I see the following possibilities after the garbage terminator.

    1. Switch to “normal” packet handling, but the first packet has AAD set: This is what this PR proposes.
    2. Like 1, but additionally ignore the ignore bit: This is what the current BIP says, and it has a special case which could be avoided.
    3. Like 1, but additionally reject the first packet if it’s a decoy packet and disconnect: On one hand, that’s just a different special case. On the other hand, then it’s then always the case that the AAD is in the version packet.
    4. Switch to a simpler headerless packet format just for the version negotiation: This is what you propose. It seems conceptually cleaner, but this requires that we expose a way to parse headerless packets to the high-level net code. In that sense. I don’t see how a rule “parse a raw version packet and delegate further packets to version-specific decoder” is much simpler than a rule “parse a v2 version packet, and delegate further packets to version-specific decoder”. The former will always need one parser more, namely one for raw packets.

    Among these approaches, I like 1 and 3 most.

    I think approach 1 (in this PR) is conceptually nice because it decouples the AAD stuff from the protocol states. You simply keep a variable around that tells what you expect as AAD in the next received packet [1]. Approach 3 does not need this variable and just needs an if (ignore) return false in the VERSION state in the C code. That’s also pretty simple. Happy to switch to this approach, I guess it’s really on par with approach 1.

    It might be slightly clearer to make the pseudocode more stateful as far as the aad info goes, ie drop it as a parameter to v2_enc_packet and v2_receive_packet and instead initialize it based on peer.sent_garbage and peer.recv_garbage, and clear them after first use?

    Will probably give this a shot later.

    [1] Note that when you wrote your comment here, the way I was implementing this (https://github.com/real-or-random/bitcoin/blob/8753b6af9bf8767be50d71d1bf7e3711738fcfd2/src/net.cpp#L1239-L1278) technically did not achieve this decoupling yet. I’ve improved this now in the implementation PR this by renaming the m_recv_buffer to m_recv_aad and handling AAD independently of whether the receiver is in VERSION state or not. This also means that the handling of the ignore bit does not depend on the state.

  16. sipa commented at 11:58 am on September 26, 2023: member

    The purpose of having decoy functionality available during the VERSION phase is mostly for future extensions that may extend the number of messages that can be sent in that phase. I agree it’s rather pointless right now, but with the prospect of extensions, I think it’s cleaner to have it available for the entirety of the phase.

    Just as a note, if we’d go for an approach with 2 layers, where some parts use headerless packets and other parts use headerful packets, the length descriptor semantics need to change too (because they’re currently excluding the header byte length). @ajtowns I don’t believe there is a benefit to splitting up the garbage into a part sent before and after the other side’s key; whatever you sent as garbage part 2 can instead be sent as decoy data (even if only possible after the version packet).

  17. ajtowns commented at 1:04 pm on September 26, 2023: contributor

    Just for my understanding: You’re saying that the handshake could look more obscure to wiretappers, right? And not that the description of the handshake in the BIP would be more obscure to readers? ;)

    Yeah, that’s what I mean, and your further reply makes sense. Still kinda think mentioning decoy packets as well as garbage in the 1.5 round trip summary would be clearer; but the rest of the BIP already makes it pretty clear anyway, so :man_shrugging:

    The purpose of having decoy functionality available during the VERSION phase is mostly for future extensions that may extend the number of messages that can be sent in that phase.

    Hmm, I guess I look at the layers as:

    • “over the wire”:
      • negotiate a key
      • send packets
      • handle rekeying
      • negotiate “application support” for future extensibility
      • do all that while looking like a pseudorandom stream
    • “application layer”:
      • is dispatched to different applications (decoy -> /dev/null, bitcoin p2p -> net_processing)
      • application layer just sees packet contents (and the session key)
      • someday, when the decoy “application” actually sends data, it might become an actual complicated application in its own right, eg in order to mimic traffic patterns of some other protocol
      • someday, if we get really worried about obscuring things, messages being sent might go through a third layer that delays them a bit to try to make it harder to do timing analysis or figure out packet boundaries

    ie, I guess I think “negotiate for future extensibility” is essentially a “meta application” that’s built into the bip324 (or future extension bip) implementation, that while it gets data from packets just like a regular application, also controls how data from packets goes to applications, so it’s really a low-level thing, not a high-level thing. Maybe “built-in as part of the implementation” vs “user of the API” is more the distinction that I’m thinking of.

    Just as a note, if we’d go for an approach with 2 layers, where some parts use headerless packets and other parts use headerful packets, the length descriptor semantics need to change too (because they’re currently excluding the header byte length).

    Yeah, I guess that makes sense: better to be able to have a full 0xFF_FFFF bytes of application data in a packet than be off by two instead of just one. I also guess that’s easy enough to extend in some future version if it’s ever desirable:

    • 16MiB isn’t enough: if bit 6 of the header is set, concatenate the contents of the next packet, and repeat, until you can send gigameg blocks back to the application as a single unit
    • 8 bits of header isn’t enough: if bit 5 of the header is set, the first however many bytes of the contents is a varint header

    LGTM :+1:

  18. sipa commented at 1:49 pm on September 26, 2023: member

    I’m really not convinced that separating headerless and headerful packets is worth it.

    At a conceptual level you can say:

    • (1) In the BIP right now, garbage auth packets are headerless, while version and application packets are headerful. The rationale arguably is that the initial phase has its own traffic shaping mechanism (garbage) so it has no need for decoy packets anyway. However, the instantiation of the garbage auth packets still used header bytes, leading to the inconsistency (ignoring decoy bits) that led to this PR.
    • (2) In the proposed BIP change in this PR, garbage auth packets are gone, and the remaining packets (version and application) are headerful both conceptually and in the way they are instantiated. While it’s true that the version phase right now needs no traffic shaping (because it’s so short the shaping can be done by the phase that follows), it’s not true conceptually that version negotiation needs no shaping entirely (something that could become apparent with future extensions).
    • (3) In what we’re discussing now, version packets become conceptually headerless, but application packets stay headerful. That can be done with or without getting rid of the header byte entirely, but I’m not sure any either is all that appealing:
      • (3a) Version packets retain a header byte, but it is ignored (resulting in a similar inconsistency to how garbage auth packets has ignored header bytes right now).
      • (3b) Version packets have no header byte at all (meaning implementations gain some complexity due to distinguishing the two types of packets). This means changing the meaning of length descriptors (either make them depend on the phase, or deal with application level packets that are too short to contain a header). Version extensions may need a traffic shaping mechanism, but this could be done by enabling headers mid-phase, or having their own ad-hoc shaping mechanism (or extensions could of course overhaul the decoy mechanism entirely).

    So while it does make sense to see the version negotiation as the thing that negotiates there being a decoy mechanism, and so logically that phase itself shouldn’t already have them, I don’t think this practically gains us anything:

    • Right now (ignoring future extensions) the specification and implementation seem simpler with (2) simply because there is only one type of packet, and its semantics are always the same.
    • For future extensions, it may or may not be desirable to have the same decoy mechanism present during the version negotiation, but if it isn’t, it can still be changed.
  19. in bip-0324.mediawiki:126 in 3d0f725b22 outdated
    124+#** Send their 16-byte garbage terminator.<ref name="why_garbage_term">'''Why does the protocol need a garbage terminator?''' While it is in principle possible to use the first packet after the garbage directly as a terminator (scan until a valid packet follows), this would be significantly slower than just scanning for a fixed byte sequence, as it would require recomputing a Poly1305 tag after every received byte.</ref>
    125 #** Receive up to 4111 bytes, stopping when encountering the garbage terminator.
    126-#** Receive an encrypted packet, verify that it decrypts correctly with associated data set to the garbage received, and then ignore its contents.
    127-#* At this point, both parties have the same keys, and all further communication proceeds in the form of encrypted packets. Packets have an '''ignore bit''', which makes them '''decoy packets''' if set. Decoy packets are to be ignored by the receiver apart from verifying they decrypt correctly. Either peer may send such decoy packets at any point after this. These form the primary shapability mechanism in the protocol. How and when to use them is out of scope for this document.
    128+#* At this point, both parties have the same keys, and all further communication proceeds in the form of '''encrypted packets'''.
    129+#** Encrypted packets have an '''ignore bit''', which makes them '''decoy packets''' if set. Decoy packets are to be ignored by the receiver apart from verifying they decrypt correctly. Either peer may send such decoy packets at any point from here now. These form the primary shapability mechanism in the protocol. How and when to use them is out of scope for this document.
    


    sipa commented at 7:06 pm on September 26, 2023:
    Nit: “from here now” -> “from here on”?

    real-or-random commented at 9:11 am on September 27, 2023:
    fixed
  20. in bip-0324.mediawiki:372 in 3d0f725b22 outdated
    368@@ -370,18 +369,21 @@ def complete_handshake(peer, initiating):
    369     ecdh_secret = v2_ecdh(peer.privkey_ours, ellswift_theirs, peer.ellswift_ours,
    370                           initiating=initiating)
    371     initialize_v2_transport(peer, ecdh_secret, initiating=True)
    372-    # Send garbage terminator + garbage authentication packet + version packet.
    373-    send(peer, peer.send_garbage_terminator +
    374-               v2_enc_packet(peer, b'', aad=peer.sent_garbage) +
    375-               v2_enc_packet(peer, TRANSPORT_VERSION))
    376+    # Send garbage terminator + version packet.
    


    sipa commented at 7:11 pm on September 26, 2023:
    The mention of version packet here is a bit confusing, as the version packet isn’t sent until after the # Optionally send decoy packets ... comment below.

    real-or-random commented at 9:12 am on September 27, 2023:
    Fixed by splitting the comment and moving the “Send version packet” part where it belongs
  21. real-or-random force-pushed on Sep 27, 2023
  22. real-or-random force-pushed on Sep 27, 2023
  23. real-or-random commented at 9:32 am on September 27, 2023: contributor

    I’ve forced-push to address the comments. I’m undrafting the PR because there seems to be broad agreement that we want to do this. @ajtowns

    Still kinda think mentioning decoy packets as well as garbage in the 1.5 round trip summary would be clearer; but the rest of the BIP already makes it pretty clear anyway, so 🤷‍♂️

    Ok, that makes sense. We mentioned it everywhere in the BIP except in this summary. Added it to the summary for consistency.

    It might be slightly clearer to make the pseudocode more stateful as far as the aad info goes, ie drop it as a parameter to v2_enc_packet and v2_receive_packet and instead initialize it based on peer.sent_garbage and peer.recv_garbage, and clear them after first use?

    I tried this, but I’m not convinced that this will make it clearer: Keeping the state local to complete_handshake also has its merits and keeping the AAD logic there keeps it close to where this logic is explained in prose.

  24. real-or-random marked this as ready for review on Sep 27, 2023
  25. real-or-random force-pushed on Sep 27, 2023
  26. real-or-random referenced this in commit a90bde593c on Sep 27, 2023
  27. real-or-random referenced this in commit b0f5175c04 on Sep 27, 2023
  28. sipa commented at 11:55 am on September 27, 2023: member
    LGTM
  29. naumenkogs commented at 1:16 pm on September 27, 2023: member
    ACK
  30. in bip-0324.mediawiki:127 in ddb6a5ca6c outdated
    125 #** Receive up to 4111 bytes, stopping when encountering the garbage terminator.
    126-#** Receive an encrypted packet, verify that it decrypts correctly with associated data set to the garbage received, and then ignore its contents.
    127-#* At this point, both parties have the same keys, and all further communication proceeds in the form of encrypted packets. Packets have an '''ignore bit''', which makes them '''decoy packets''' if set. Decoy packets are to be ignored by the receiver apart from verifying they decrypt correctly. Either peer may send such decoy packets at any point after this. These form the primary shapability mechanism in the protocol. How and when to use them is out of scope for this document.
    128+#* At this point, both parties have the same keys, and all further communication proceeds in the form of '''encrypted packets'''.
    129+#** Encrypted packets have an '''ignore bit''', which makes them '''decoy packets''' if set. Decoy packets are to be ignored by the receiver apart from verifying they decrypt correctly. Either peer may send such decoy packets at any point from here on. These form the primary shapability mechanism in the protocol. How and when to use them is out of scope for this document.
    130+#** For each of the two directions, the first encrypted packet that will be sent in that direction (regardless of it being a decoy packet or not) will make use of the associated authenticated data (AAD) feature of the AEAD to authenticate the garbage that has been sent in that direction.<ref name="why_garbage_auth">'''Why does the protocol authenticate the garbage?''' Without garbage authentication, the garbage would be modifiable by a third party without consequences. We want to force any active attacker to have to maintain a full protocol state. In addition, such malleability without the consequence of connection termination could enable protocol fingerprinting.</ref>
    


    naumenkogs commented at 1:17 pm on September 27, 2023:
    I don’t understand what kind of fingerprinting will be possible. Maybe it’s just me. Can you explain in more detail?

    real-or-random commented at 1:25 pm on September 27, 2023:
    It would be very uncommon for a fully encrypted protocol not to abort in case of an attacker modifying data on the wire. So as an attacker, you could flip some bits in the garbage, notice that the connection is not dropped, and then conclude that v2 is running. (edit: Note that I’m touching this sentence.)

    cacrowley commented at 5:17 pm on September 29, 2023:
    thank you
  31. naumenkogs commented at 1:27 pm on September 27, 2023: member
    One thing you may want to improve is making it clear that garbage-terminator is not a part of garbage when doing aad. This is something i had to double-check everywhere manually on my own :) A BIP could be explicit about this.
  32. Sjors commented at 4:24 pm on September 27, 2023: member
    Concept ACK, but haven’t studied the BIP changes, only the code change.
  33. bip324: Remove garbage authentication packet (breaking change)
    by merging it with the version packet. Or more accurately, by merging
    it with the first packet sent after garbage termination, which may be
    a decoy packet or the version packet.
    
    The new protocol simplifies implementations:
     - A protocol state machine won't need separate states for garbage
       authentication and version phases.
     - The special case of "ignoring the ignore bit" is removed.
     - The freedom to choose the contents of the garbage authentication
       packet is removed. This simplifies testing.
    
    The reason for having a separate garbage authentication packet was
    to materialize the separation of the key exchange phase and version
    negotiation phase even in the bytestream on the wire. However, this
    is not necessary, and arguably, these phases are still properly
    separated: Since the AEAD will ensure that AAD (=garbage) is checked
    before looking at the contents (=version), the peers won't interpret
    version data before having authenticated the garbage.
    75dc363d20
  34. real-or-random force-pushed on Sep 28, 2023
  35. real-or-random commented at 8:21 am on September 28, 2023: contributor

    One thing you may want to improve is making it clear that garbage-terminator is not a part of garbage when doing aad. This is something i had to double-check everywhere manually on my own :) A BIP could be explicit about this.

    Added “not including the garbage terminator” at an appropriate location in the text.

  36. in bip-0324.mediawiki:380 in 75dc363d20
    379+    aad = peer.sent_garbage
    380+    for decoy_content_len in decoy_content_lengths:
    381+        send(v2_enc_packet(peer, decoy_content_len * b'\x00', aad=aad))
    382+        aad = b''
    383+    # Send version packet.
    384+    send(v2_enc_packet(peer, TRANSPORT_VERSION, aad=aad))
    


    Sjors commented at 9:45 am on September 28, 2023:

    Nit:

    0aad = b''
    

    Maybe also rename to send_aad.

  37. in bip-0324.mediawiki:387 in 75dc363d20
    390-            v2_receive_packet(peer, aad=received_garbage, skip_decoy=False)
    391-            # Receive, decode, and ignore version packet, skipping decoys
    392-            v2_receive_packet(peer)
    393+            # Receive, decode, and ignore version packet.
    394+            # This includes skipping decoys and authenticating the received garbage.
    395+            v2_receive_packet(peer, aad=received_garbage)
    


    Sjors commented at 9:59 am on September 28, 2023:

    Mmm, the v2_receive_packet helper now includes assumptions that only make sense in the handshake.

    Maybe instead it can return a type and content? And also not do aad = b''.

    So then this code becomes something like:

    0# Receive, decode, and ignore version packet.
    1# This includes skipping decoys and authenticating the received garbage.
    2aad = received_garbage
    3while true:
    4  (type, content) = v2_receive_packet(peer, aad=aad)
    5  aad = b''
    6  break if type == VERSION
    7  throw if type != IGNORE
    

    real-or-random commented at 10:25 am on September 28, 2023:

    Mmm, the v2_receive_packet helper now includes assumptions that only make sense in the handshake.

    Does it? Can you spell them out explicitly?

    If you’re referring to “Only the first packet is expected to have non-empty AAD.”, then I think this will also be the case if we ever use AAD for anything else than garbage. Even in that case, you’d always want to skip decoy packets and it’s reasonable to include the AAD in the first packet. This assumes that the AAD will always be determined by the received (non-decoy) packets or the outer header (see my response in the Core PR: https://github.com/bitcoin/bitcoin/pull/28525#discussion_r1339889801).


    Sjors commented at 10:41 am on September 28, 2023:

    It depends on how the AAD is used, which you elaborated on here: https://github.com/bitcoin/bitcoin/pull/28525#discussion_r1339889801

    If the AAD is set at the level of traffic shaping then it might be different for decoy and real messages. If it’s only used at the application level then I agree it’s probably fine to set it in the first (decoy) message.

    If we believe the latter is much more likely, then I suppose a simple comment in the helper function would suffice:

    0# This includes skipping decoys and authenticating the received garbage.
    1# We assume that for future AAD uses, beyond the handshake, its contents
    2# is determined before a package is received and is checked in the first
    3# message, even if that's a decoy.
    

    sipa commented at 3:54 pm on September 28, 2023:

    Truth is, we have no idea how the AAD would be used in future extensions, or even whether it’ll ever get used for anything but garbage authentication.

    In light of that, it feels unnecessary to impose a particular “grand design” for future extensions or their interaction with AAD and explain it in the specification. We simply pick a particular design because it’s concise and can be rationalized, but I don’t think we need to specify future behavior - that feels like an analogue of premature optimization (premature specification?).

    “Set the expected AAD when processing garbage + terminator, verify the expected AAD whenever any packet is received, and reset the expected AAD to nothing after verifying a packet” is one concise and simple description, and I can imagine that it remains usable for hypothetical extensions that appropriate AAD for other purposes. And if not, those extensions can just change the specification.

  38. Sjors commented at 10:00 am on September 28, 2023: member
    utACK 75dc363d201072579289cc6d20d40ef7a67db291 modulo some example code suggestions
  39. sipa commented at 7:20 pm on September 28, 2023: member
    I consider this ready for merging. @luke-jr @kallewoof
  40. sipa referenced this in commit 5b6c9b0794 on Sep 28, 2023
  41. ajtowns commented at 6:23 am on September 29, 2023: contributor
    ACK 75dc363d201072579289cc6d20d40ef7a67db291 fwiw
  42. kallewoof merged this on Sep 29, 2023
  43. kallewoof closed this on Sep 29, 2023

  44. real-or-random referenced this in commit 5479e69049 on Sep 29, 2023
  45. achow101 referenced this in commit d18a8f6f69 on Sep 29, 2023
  46. Retropex referenced this in commit a79a593411 on Oct 4, 2023
  47. Frank-GER referenced this in commit 478612d643 on Oct 5, 2023
  48. in bip-0324.mediawiki:360 in 75dc363d20
    356@@ -358,10 +357,10 @@ def respond_v2_handshake(peer, garbage_len):
    357     use_v1_protocol()
    358 </pre>
    359 
    360-Upon receiving the encoded responder public key, the initiator derives the shared ECDH secret and instantiates the encrypted transport. It then sends the derived 16-byte <code>initiator_garbage_terminator</code> followed by an authenticated, encrypted packet with empty contents<ref name="send_empty_garbauth">'''Does the content of the garbage authentication packet need to be empty?''' The receiver ignores the content of the garbage authentication packet, so its content can be anything, and it can in principle be used as a shaping mechanism too. There is however no need for that, as immediately afterward the initiator can start using decoy packets as (a much more flexible) shaping mechanism instead.</ref> to authenticate the garbage, and its own version packet. It then receives the responder's garbage and garbage authentication packet (delimited by the garbage terminator), and checks if the garbage is authenticated correctly. The responder performs very similar steps but includes the earlier received prefix bytes in the public key. As mentioned before, the encrypted packets for the '''version negotiation phase''' can be piggybacked with the garbage authentication packet to minimize roundtrips.
    361+Upon receiving the encoded responder public key, the initiator derives the shared ECDH secret and instantiates the encrypted transport. It then sends the derived 16-byte <code>initiator_garbage_terminator</code>, optionally followed by an arbitrary number of decoy packets. Afterwards, it receives the responder's garbage (delimited by the garbage terminator). The responder performs very similar steps but includes the earlier received prefix bytes in the public key. Both the initiator and the responder set the AAD of the first encrypted packet they send after the garbage terminator (i.e., either an optional decoy packet or the version packet) to the garbage they have just sent, not including the garbage terminator.
    


    stratospher commented at 2:15 pm on October 6, 2023:
    since there’s no limit on the number of decoy packets that a peer can send (after sending the garbage terminator, before sending the version packet), wouldn’t we want to disconnect from the handshake at some point - so that the connection can be dropped and we use that slot for a more useful peer?

    stratospher commented at 2:16 pm on October 6, 2023:
    cc @sipa

    sipa commented at 2:20 pm on October 6, 2023:
    @stratospher We certainly do want to disconnect useless peers, but I don’t think the transport layer is the right place to address that. After all, peers can be equally useless by just behaving weirdly at the application level (e.g. never do anything except respond to pings). In Bitcoin Core, connection rotation and inbound eviction already deal with these cases (but can be improved, of course), and apply just fine here, because if a node only sends decoys, the application layer will just see a connection that never sends a version/verack packet.

    stratospher commented at 1:19 pm on October 7, 2023:
    thanks! makes sense.
  49. janus referenced this in commit 60b04c6d51 on Apr 1, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bips. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-31 23:10 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me