Skip to content

Conversation

jonatack
Copy link
Member

@jonatack jonatack commented Jan 7, 2024

A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type. Encryption type is a property of the session, not the destination. Sessions may support multiple encryption types.

As Bitcoin Core is not currently setting the encryption type when creating I2P sessions, it uses the older default, ElGamal (type 0).

This pull updates our I2P session creation to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This allows to connect to I2P peers of either type, and the newer, faster ECIES-X25519 will be preferred.

See also:

Thank you and credit to zzzi2p for reporting and to vort for the patch.

Closes #29197.

A Bitcoin Core node may only connect to a peer destination via I2P if both sides
have sessions with the same encryption type.  The encryption type is a property
of the session, not the destination.  Sessions may support multiple encryption
types.

As Bitcoin Core is not currently setting the I2P encryption type when creating
sessions, it is using the older default, ElGamal (type 0).

This pull updates Bitcoin Core to use both ECIES-X25519 and ElGamal (types 4 and
0, respectively).  This allows to connect to I2P peers with either type, and the
newer, faster ECIES-X25519 will be preferred.

See also the recently updated section "Signature and Encryption Types" in
https://geti2p.net/en/docs/api/samv3

Thanks and credit to zzzi2p (https://github.com/zzzi2p) for reporting.

Closes bitcoin#29197.
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 7, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK zzzi2p, kristapsk, brunoerg, recursive-rat4, shaavan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the P2P label Jan 7, 2024
@zzzi2p
Copy link

zzzi2p commented Jan 8, 2024

ACK 9d72891

Copy link
Contributor

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cr utACK 9d72891

@jonatack
Copy link
Member Author

jonatack commented Jan 8, 2024

544A5198-B738-49C5-A08E-0630603AE372_4_5005_c

Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crACK 9d72891

@recursive-rat4
Copy link

ACK 9d72891

I2P 2.3.0 shows both encryption keys.

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crACK 9d72891

Expanded Encryption Support:
Enables I2P to connect using both ECIES-X25519 and ElGamal encryption types (4 and 0, respectively), broadening secure connectivity options beyond the default ElGamal.

Optimal Encryption Choice:
Code logic appropriately prioritizes the faster ECIES-X25519 encryption, aligning with best practices for security and performance.

Clear Rationale:
Commit message provides a concise and transparent explanation for the code update, enhancing overall clarity.

Overall, this PR enhances encryption flexibility, prioritizing speed, and maintains clear documentation.

@jonatack jonatack mentioned this pull request Jan 9, 2024
@fanquake fanquake merged commit 5a121bc into bitcoin:master Jan 9, 2024
@jonatack jonatack deleted the 2024-01-i2p-use-both-encryption-types branch January 9, 2024 17:21
glozow pushed a commit to glozow/bitcoin that referenced this pull request Jan 10, 2024
A Bitcoin Core node may only connect to a peer destination via I2P if both sides
have sessions with the same encryption type.  The encryption type is a property
of the session, not the destination.  Sessions may support multiple encryption
types.

As Bitcoin Core is not currently setting the I2P encryption type when creating
sessions, it is using the older default, ElGamal (type 0).

This pull updates Bitcoin Core to use both ECIES-X25519 and ElGamal (types 4 and
0, respectively).  This allows to connect to I2P peers with either type, and the
newer, faster ECIES-X25519 will be preferred.

See also the recently updated section "Signature and Encryption Types" in
https://geti2p.net/en/docs/api/samv3

Thanks and credit to zzzi2p (https://github.com/zzzi2p) for reporting.

Closes bitcoin#29197.

Github-Pull: bitcoin#29200
Rebased-From: 9d72891
@glozow glozow mentioned this pull request Jan 10, 2024
@glozow
Copy link
Member

glozow commented Jan 10, 2024

Backported in #29209

@jonatack jonatack mentioned this pull request Jan 16, 2024
glozow pushed a commit to glozow/bitcoin that referenced this pull request Jan 18, 2024
A Bitcoin Core node may only connect to a peer destination via I2P if both sides
have sessions with the same encryption type.  The encryption type is a property
of the session, not the destination.  Sessions may support multiple encryption
types.

As Bitcoin Core is not currently setting the I2P encryption type when creating
sessions, it is using the older default, ElGamal (type 0).

This pull updates Bitcoin Core to use both ECIES-X25519 and ElGamal (types 4 and
0, respectively).  This allows to connect to I2P peers with either type, and the
newer, faster ECIES-X25519 will be preferred.

See also the recently updated section "Signature and Encryption Types" in
https://geti2p.net/en/docs/api/samv3

Thanks and credit to zzzi2p (https://github.com/zzzi2p) for reporting.

Closes bitcoin#29197.

Github-Pull: bitcoin#29200
Rebased-From: 9d72891
glozow pushed a commit to glozow/bitcoin that referenced this pull request Jan 19, 2024
A Bitcoin Core node may only connect to a peer destination via I2P if both sides
have sessions with the same encryption type.  The encryption type is a property
of the session, not the destination.  Sessions may support multiple encryption
types.

As Bitcoin Core is not currently setting the I2P encryption type when creating
sessions, it is using the older default, ElGamal (type 0).

This pull updates Bitcoin Core to use both ECIES-X25519 and ElGamal (types 4 and
0, respectively).  This allows to connect to I2P peers with either type, and the
newer, faster ECIES-X25519 will be preferred.

See also the recently updated section "Signature and Encryption Types" in
https://geti2p.net/en/docs/api/samv3

Thanks and credit to zzzi2p (https://github.com/zzzi2p) for reporting.

Closes bitcoin#29197.

Github-Pull: bitcoin#29200
Rebased-From: 9d72891
fanquake added a commit that referenced this pull request Feb 16, 2024
11f3a7e Use hardened runtime on macOS release builds. (Mark Friedenbach)
ac1b9a5 [test] import descriptor wallet with reorged parent + IsFromMe child in mempool (glozow)
ecb8ebc [test] rescan legacy wallet with reorged parent + IsFromMe child in mempool (Gloria Zhao)
438ac29 snapshots: don't core dump when running -checkblockindex after `loadtxoutset` (Mark Friedenbach)
7ec3455 [log] mempool loading (glozow)
fe0f8fe net: create I2P sessions with both ECIES-X25519 and ElGamal encryption (Jon Atack)
fc62271 [refactor] Add helper for iterating through mempool entries (stickies-v)

Pull request description:

  Backports for 26.x. Includes:
  - 453b481 from #28391
    - #29179
  - #29200
  - #29227
  - #28791
  - #29127

ACKs for top commit:
  stickies-v:
    ACK 11f3a7e

Tree-SHA512: 20ef871ec768f2328056d83f958e595b36ae9b8baa8a6e8b0e1f02c3df4b16965a8e05dcb4323afbcc9ecc4bdde10931232512022c39ee7e12baf9795bf25bf1
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 24, 2024
… and ElGamal encryption

9d72891 net: create I2P sessions with both ECIES-X25519 and ElGamal encryption (Jon Atack)

Pull request description:

  A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type.  Encryption type is a property of the session, not the destination.  Sessions may support multiple encryption types.

  As Bitcoin Core is not currently setting the encryption type when creating I2P sessions, it uses the older default, ElGamal (type 0).

  This pull updates our I2P session creation to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This allows to connect to I2P peers of either type, and the newer, faster ECIES-X25519 will be preferred.

  See also:

  - discussion around qbittorrent/qBittorrent#19625 (comment)
  - recently updated "Signature and Encryption Types" in https://geti2p.net/en/docs/api/samv3

  Thank you and credit to zzzi2p for reporting and to vort for the patch.

  Closes bitcoin#29197.

ACKs for top commit:
  zzzi2p:
    ACK 9d72891
  recursive-rat4:
    ACK 9d72891
  kristapsk:
    cr utACK 9d72891
  brunoerg:
    crACK 9d72891
  shaavan:
    crACK 9d72891

Tree-SHA512: 0912fc01af9706914a7854f7479b9d82fc86c9530466cad8674e30f7eb4894d90d514efbc1aee8b7ea690faa6ff4a23b62cf5de8737cffdbc463300082c9b917
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 24, 2024
… and ElGamal encryption

9d72891 net: create I2P sessions with both ECIES-X25519 and ElGamal encryption (Jon Atack)

Pull request description:

  A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type.  Encryption type is a property of the session, not the destination.  Sessions may support multiple encryption types.

  As Bitcoin Core is not currently setting the encryption type when creating I2P sessions, it uses the older default, ElGamal (type 0).

  This pull updates our I2P session creation to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This allows to connect to I2P peers of either type, and the newer, faster ECIES-X25519 will be preferred.

  See also:

  - discussion around qbittorrent/qBittorrent#19625 (comment)
  - recently updated "Signature and Encryption Types" in https://geti2p.net/en/docs/api/samv3

  Thank you and credit to zzzi2p for reporting and to vort for the patch.

  Closes bitcoin#29197.

ACKs for top commit:
  zzzi2p:
    ACK 9d72891
  recursive-rat4:
    ACK 9d72891
  kristapsk:
    cr utACK 9d72891
  brunoerg:
    crACK 9d72891
  shaavan:
    crACK 9d72891

Tree-SHA512: 0912fc01af9706914a7854f7479b9d82fc86c9530466cad8674e30f7eb4894d90d514efbc1aee8b7ea690faa6ff4a23b62cf5de8737cffdbc463300082c9b917
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 24, 2024
… and ElGamal encryption

9d72891 net: create I2P sessions with both ECIES-X25519 and ElGamal encryption (Jon Atack)

Pull request description:

  A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type.  Encryption type is a property of the session, not the destination.  Sessions may support multiple encryption types.

  As Bitcoin Core is not currently setting the encryption type when creating I2P sessions, it uses the older default, ElGamal (type 0).

  This pull updates our I2P session creation to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This allows to connect to I2P peers of either type, and the newer, faster ECIES-X25519 will be preferred.

  See also:

  - discussion around qbittorrent/qBittorrent#19625 (comment)
  - recently updated "Signature and Encryption Types" in https://geti2p.net/en/docs/api/samv3

  Thank you and credit to zzzi2p for reporting and to vort for the patch.

  Closes bitcoin#29197.

ACKs for top commit:
  zzzi2p:
    ACK 9d72891
  recursive-rat4:
    ACK 9d72891
  kristapsk:
    cr utACK 9d72891
  brunoerg:
    crACK 9d72891
  shaavan:
    crACK 9d72891

Tree-SHA512: 0912fc01af9706914a7854f7479b9d82fc86c9530466cad8674e30f7eb4894d90d514efbc1aee8b7ea690faa6ff4a23b62cf5de8737cffdbc463300082c9b917
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 24, 2024
8bf1d06 Merge bitcoin#29308: doc: update `BroadcastTransaction` comment (glozow)
2a77808 Merge bitcoin-core/gui#789: Avoid non-self-contained Windows header (Hennadii Stepanov)
da371b8 Merge bitcoin#28870: depends: Include `config.guess` and `config.sub` into `meta_depends` (fanquake)
2e41562 Merge bitcoin#29219: fuzz: Improve fuzzing stability for ellswift_roundtrip harness (fanquake)
b091329 Merge bitcoin#29211: fuzz: fix `connman` initialization (Ava Chow)
df42d41 Merge bitcoin#29200: net: create I2P sessions using both ECIES-X25519 and ElGamal encryption (fanquake)
4cdd1a8 Merge bitcoin#29172: fuzz: set `nMaxOutboundLimit` in connman target (fanquake)
97012ea Merge bitcoin#28962: doc: Rework guix docs after 1.4 release (fanquake)
c70ff5d Merge bitcoin#28844: contrib: drop GCC MAX_VERSION to 4.3.0 in symbol-check (fanquake)
e6f19e7 Merge bitcoin#29068: test: Actually fail when a python unit test fails (fanquake)
75e0334 Merge bitcoin#28989: test: Fix test by checking the actual exception instance (Andrew Chow)
8cd85d3 Merge bitcoin#28852: script, assumeutxo: Enhance validations in utxo_snapshot.sh (Ryan Ofsky)
fd2e88d Merge bitcoin#26077: guix: switch from `guix environment` to `guix shell` (fanquake)
02741a7 Merge bitcoin#28913: coins: make sure PoolAllocator uses the correct alignment (fanquake)
dfd53da Merge bitcoin#28902: doc: Simplify guix install doc, after 1.4 release (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  Batch of trivial backports

  ## What was done?
  See commits

  ## How Has This Been Tested?
  built locally; large combined merge passed tests locally

  ## Breaking Changes
  Should be none

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 8bf1d06
  knst:
    utACK 8bf1d06

Tree-SHA512: 506273e5a188f9ca74edf656e3cd338992192e6e97f68c89fc43e34be20fb7f211b48e4dfa8693727839a7920da8284509413c722f55774a428939c296dad517
@bitcoin bitcoin locked and limited conversation to collaborators Jan 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I2P: Change encryption type
9 participants