Skip to content

Commit

Permalink
Document OpenPGP specification mistake
Browse files Browse the repository at this point in the history
GnuPG has the correct behavior, it's the spec that was incorrect.
  • Loading branch information
skeeto committed Aug 12, 2019
1 parent a2e283a commit 5f6d6a8
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 33 deletions.
8 changes: 0 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,6 @@ passphrase2pgp will reuse your derivation passphrase as the protection
passphrase. However, keep in mind that the S2K algorithm is *much*
weaker than the algorithm used to derive the asymmetric key, Argon2id.

Important note: [**GnuPG's S2K was implemented incorrectly and is
incompatible with OpenPGP**][t4676]. Since this defect went practically
unnoticed for literally two decades, GnuPG's algorithm has become the de
facto standard. As a result, passphrase2pgp uses GnuPG's S2K algorithm,
not the OpenPGP standard, since compatibility with GnuPG is more useful.

[t4676]: https://dev.gnupg.org/T4676

### Examples

Generate a private key and send it to GnuPG (with no protection passphrase):
Expand Down
38 changes: 13 additions & 25 deletions openpgp/signkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ const (

// Encoded S2K octet count.
s2kCount = 0xff // maximum strength

// Use GnuPG conventions instead of OpenPGP where they differ.
gnupgCompat = true
)

var (
Expand Down Expand Up @@ -224,29 +221,20 @@ func decodeS2K(c byte) int {
// Compute a symmetric protection key via S2K.
func s2k(passphrase, salt []byte, count int) []byte {
h := sha256.New()
if gnupgCompat {
// Due to an old bug, GnuPG's S2K subtly differs from OpenPGP
// making them incompatible. This branch implements the GnuPG
// verison.
// https://dev.gnupg.org/T4676
full := make([]byte, 8+len(passphrase))
copy(full[0:], salt)
copy(full[8:], passphrase)
iterations := count / len(full)
for i := 0; i < iterations; i++ {
h.Write(full)
}
tail := count - iterations*len(full)
h.Write(full[:tail])
} else {
// OpenPGP, incompatible with GnuPG
inlen := 8 + len(passphrase)
iterations := (count + inlen - 1) / inlen
for i := 0; i < iterations; i++ {
h.Write(salt)
h.Write(passphrase)
}
// Note: This implements S2K as it is actually used in practice by
// both GnuPG and PGP. The OpenPGP standard (3.7.1.3) is subtly
// incorrect in its description, and that algorithm is not used by
// actual implementations.
// https://dev.gnupg.org/T4676
full := make([]byte, 8+len(passphrase))
copy(full[0:], salt)
copy(full[8:], passphrase)
iterations := count / len(full)
for i := 0; i < iterations; i++ {
h.Write(full)
}
tail := count - iterations*len(full)
h.Write(full[:tail])
return h.Sum(nil)
}

Expand Down

0 comments on commit 5f6d6a8

Please sign in to comment.