-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Expand BIP85 to include ECC key types #1968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 4 commits
3728f8e
56a8de8
31eea92
9479612
729f412
86a64f1
04995e6
00a7583
b7756c9
c44de46
8b1397a
34e8931
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -360,31 +360,57 @@ OUTPUT | |||||
| * DERIVED ENTROPY=f7cfe56f63dca2490f65fcbf9ee63dcd85d18f751b6b5e1c1b8733af6459c904a75e82b4a22efff9b9e69de2144b293aa8714319a054b6cb55826a8e51425209 | ||||||
| * DERIVED PWD=_s`{TW89)i4` | ||||||
|
|
||||||
| ===RSA=== | ||||||
| ===GPG Keys=== | ||||||
|
|
||||||
| Application number: 828365' | ||||||
|
|
||||||
| The derivation path format is: <code>m/83696968'/828365'/{key_bits}'/{key_index}'</code> | ||||||
| The derivation path format is: <code>m/83696968'/828365'/{key_type}'/{key_bits}'/{key_index}'</code> | ||||||
|
||||||
|
|
||||||
| The RSA key generator should use BIP85-DRNG as the input RNG function. | ||||||
| The key_type values are defined as follows: | ||||||
|
|
||||||
| ===RSA GPG=== | ||||||
| {| | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this can be one big table and will read better; there's already a common join key and both are small |
||||||
| !OpenPGP Key Type | ||||||
| !key_type value | ||||||
| |- | ||||||
| | RSA | ||||||
| | 0' | ||||||
| |- | ||||||
| | ECC(Curve25519) | ||||||
| | 1' | ||||||
| |- | ||||||
|
Comment on lines
+365
to
+378
|
||||||
| | ECC(secp256k1) | ||||||
| | 2' | ||||||
| |- | ||||||
| | ECC(NIST) | ||||||
| | 3' | ||||||
| |- | ||||||
| | ECC(Brainpool) | ||||||
| | 4' | ||||||
|
||||||
| |- | ||||||
| |} | ||||||
|
|
||||||
| The RSA key generator should use BIP85-DRNG as the input RNG function. Likewise, any ECC key types over 256 bits should use BIP85-DRNG. | ||||||
|
||||||
| The RSA key generator should use BIP85-DRNG as the input RNG function. Likewise, any ECC key types over 256 bits should use BIP85-DRNG. | |
| The RSA key generator should use BIP85-DRNG as the input RNG function. Likewise, any ECC key types whose key generation requires more than 64 bytes of random input should use BIP85-DRNG. |
Copilot
AI
Feb 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Curve25519, OpenPGP typically uses different algorithms for different capabilities (e.g., Ed25519 for certify/sign/auth vs X25519/Curve25519 ECDH for encryption). The current text says all subkeys SHOULD use the same key_type, but doesn’t specify the required per-capability algorithm mapping within that key_type. Please clarify the exact OpenPGP public-key algorithm/curve used for primary vs encryption/auth/sign subkeys under key_type=Curve25519 to avoid incompatible key material generation.
| All subkeys SHOULD use the same key_type as the primary key. Mixed key types across primary and subkeys are out of scope for this specification. Additional subkeys may be added following the same role pattern, incrementing the sub_key index. | |
| All subkeys SHOULD use the same key_type (i.e. the same underlying curve family) as the primary key. Mixed key types across primary and subkeys are out of scope for this specification. For <code>key_type = Curve25519</code>, implementations MUST generate OpenPGP keys as follows: the primary CERTIFY key and any SIGNATURE or AUTHENTICATION subkeys MUST use Ed25519 (OpenPGP EdDSA with curve Ed25519), while ENCRYPTION subkeys MUST use X25519/Curve25519 ECDH (OpenPGP ECDH with curve Curve25519). Additional subkeys may be added following the same role pattern, incrementing the sub_key index. |
Copilot
AI
Feb 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section now covers multiple GPG key types, but the timestamp note still refers specifically to "The resulting RSA key". Either generalize the statement to apply to all OpenPGP keys derived here (including ECC), or explicitly scope the timestamp requirement to RSA-only if ECC behavior differs.
Copilot
AI
Feb 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog entries below include dates (e.g., "1.3.0 (2024-10-22)"), but the new "2.0.0" entry has no date. Add a date (or follow whatever versioning/date convention the document uses) to keep the changelog consistent.
| ===2.0.0=== | |
| ===2.0.0 (2024-11-01)=== |
Copilot
AI
Feb 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 2.0.0 changelog entry says the GPG section now uses a single application number with key_type, "replacing separate per-type application numbers", which doesn’t match the earlier changelog/history in this document (it previously introduced an RSA application at 828365'). Also, this conflicts with the PR description’s stated approach of using different application IDs per key type. Please reconcile the changelog text (and/or the spec approach) so the intended compatibility strategy is clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely need test vectors and expected outputs for all applications and sub applications. I can also add it to the reference implementation but won't be able to get to that right away. PRs welcome. https://github.com/akarve/bipsea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to work up some test vectors and a reference implementation once there is some agreement on a general direction :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a pretty simple app protocol in the ref. implementation now: https://github.com/akarve/bipsea/blob/main/src/bipsea/apps/README.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have actually got a working implementation of the draft spec (as per the last edit) here: https://github.com/3rdIteration/bipsea