This requirement is from RFC 4880 4.2.2.4.
Also simplify the partialLengthWriter loop. The old code worked but
was written in a confusing way, with a loop whose terminating condition
didn't make sense and was never true in practice.
Rewrite it to more clearly do a set of partial writes of decreasing size.
Fixesgolang/go#32474
Change-Id: Ia53ceb39a34f1d6f2ea7c60190d52948bb0db59b
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/181121
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
RFC 4800, Section 6 specifies that the CRC at the end of the
armor is optional, so do not fail to decode signatures missing
the CRC.
Credit: armor.go patch from engineer at Google
Change-Id: I39b04e0afaaafdf7aa86577fe4a35c50e7cf0b2b
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/215022
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
RFC 4880 uses the term "creation time" to refer to when keys and
signatures are created, and this is partially followed already. This
CL applies it to the rest of places that were using "currentTime"
instead.
Change-Id: I8a83604b53a240689327defd89e22ddbebe3b679
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/175446
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Aida Mynzhasova of SEC Consult Vulnerability Lab reported that the
clearsign package accepts some malformed messages, which can make it
possible for an attacker to trick a human user (but not a Go program)
into thinking unverified text is part of the message.
For example, if in the following message the vertical tab character is
printed as a new line, a human observer could believe that the
unverified header text is part of the signed message.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1\x0b\x0bThis text is part of the header.
Hello world!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iJwEAQECAAYFAk8kMuEACgkQO9o98PRieSpMsAQAhmY/vwmNpflrPgmfWsYhk5O8
[...]
MyTpno24AjIAGb+mH1U=
=hIJ6
-----END PGP SIGNATURE-----
The OpenPGP specs are delightfully vague about purpose and validation of
these headers. RFC 4880, Section 7 says
The cleartext signed message consists of:
- The cleartext header '-----BEGIN PGP SIGNED MESSAGE-----' on a
single line,
- One or more "Hash" Armor Headers,
- Exactly one empty line not included into the message digest,
[...]
but also
If MD5 is the only hash used, then an
implementation MAY omit this header for improved V2.x compatibility.
and
If more than one message digest is used in the signature, the "Hash"
armor header contains a comma-delimited list of used message digests.
which seems to suggest that there can be zero or more Hash headers, each
with one or more algorithms, and no other header types.
Anyway, it's entirely unclear what security purpose, if any, the Hash
header accomplishes. If the hash is too weak to be secure or
unsupported, the verification will fail. Otherwise, the user shouldn't
care. Given its dubious function, avoid breaking abstractions to check
that it matches the signature, and just document it as unverified.
As for valid characters, RFC 4880 is silent, except reluctantly
mentioning that the Comment header can be UTF-8, but I am going to
assume that all hash algorithms will have ASCII names, because come on.
Even more importantly, reject non-Hash SIGNED MESSAGE headers (as opposed
to the SIGNATURE headers), to prevent a "Thank you!" message turning into
-----BEGIN PGP SIGNED MESSAGE-----
Reminder: I need you to wire $100k to 12345566 as soon as possible.
Thank you!
-----BEGIN PGP SIGNATURE-----
[...]
While at it, also check for trailing characters after the signed message
delimiter, as they are invalid and can be similarly used to confuse humans.
The Decode API is also unfortunate in that it doesn't return an error,
so we can't tell the user what's wrong with the message, but that's what
we've got.
Change-Id: I8a72c4851075337443d7a27e0b49a6b6e39f5a41
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/453011
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/173778
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
audited using ineffassign tool from
github.com/gordonklaus/ineffassign
go generate does not generate any changes
Change-Id: Iabbec9ec1aae39081289d503d79fd7b4caadf17b
GitHub-Last-Rev: acd17cce410e9c68ce3c87b5546261be9153e3ea
GitHub-Pull-Request: golang/crypto#70
Reviewed-on: https://go-review.googlesource.com/c/155942
Reviewed-by: Filippo Valsorda <filippo@golang.org>
SHA384 is a natural hashing choice for P-384 ECDSA. The only thing
needed to make it usable, is adding it to the list of candidates.
Change-Id: I61f66f371774f95dfc1de30d10fab66f92c21b6b
Reviewed-on: https://go-review.googlesource.com/c/137956
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
keys_test.go was slowing down my editor because it was getting too
large. It helps to remove the keys of the file as they contain extremely
long lines and large strings.
Change-Id: I8d193179ddc32438b7233f0f9ca8c57c928a0436
Reviewed-on: https://go-review.googlesource.com/c/138997
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Test for RFC4880 5.2.3.3:
> An implementation that encounters multiple self-signatures on the
> same object may resolve the ambiguity in any way it sees fit, but it
> is RECOMMENDED that priority be given to the most recent self-
> signature.
Note: Some GPG implementation will reorder the packets for you when
exporting keys. This makes it complicated to generate a key for this
test. Should someone have to create a similar key again, look into
gpgsplit, gpg --dearmor, and gpg --enarmor. These keys exist in the
wild too.
Change-Id: I5d46054ebbc95407d644e4e462d777aab290794c
Reviewed-on: https://go-review.googlesource.com/138215
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Rather than using the first subkey binding signature encountered, use
the one with the most recent creation data, as per the recommendation from
RFC 4880:
> An implementation that encounters multiple self-signatures on the
> same object may resolve the ambiguity in any way it sees fit, but it
> is RECOMMENDED that priority be given to the most recent self-
> signature.
This allows subkeys to approach expiry then be re-signed with a new expiry.
This extends the recent commit 0e37d00 by @aviau and @FiloSottile.
Fixesgolang/go#26468
Change-Id: I7f12706727373259c188bfee4254306ef9d4e935
GitHub-Last-Rev: 0da814166411a3c3334312576d3f7f8f2bba4ff4
GitHub-Pull-Request: golang/crypto#57
Reviewed-on: https://go-review.googlesource.com/135357
Reviewed-by: Filippo Valsorda <filippo@golang.org>
In change id Id992676ef2363779a7028f4799180efb027fcf47, "current" was
moved into the UserID packet handling scope. This was the only thing
preventing us to move the UserID packet handling code inside its own
function.
This patch moves the UserID packet handling code inside a new addUserID
function. This is consistent with the other existing addSubKey method.
"current" is renamed to "identity" for improved readability.
Change-Id: I5d58eb35ab5fa9fc7d9d111fa186fec6f5e11e79
Reviewed-on: https://go-review.googlesource.com/118959
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Consider the following packet ordering scenario:
PUBKEY UID SELFSIG SUBKEY REV SELFSIG
In this scenario, addSubkey would only consume the REV signature after
the subkey, leaving SELFSIG to be read by ReadEntity, which in turn
would add the last SELFSIG to the UID's signatures, which is wrong to do
because this is a SUBKEY SELFSIG, not a UID signature.
Remove "current" from the ReadEntity scope, it should only be visible
to the UserId packet handling code.
Keep the warning about signature packets found before user id packets.
Without it, I would not have found this bug.
Modify addSubKey so that it consumes all signatures following the SUBKEY
packet, keeping eithier the first valid signature (like we did before)
or any valid revocation.
In a follow-up patch, we can improve this further by keeping the
most recent signature, as suggested by RFC4880:
> An implementation that encounters multiple self-signatures on the
> same object may resolve the ambiguity in any way it sees fit, but it
> is RECOMMENDED that priority be given to the most recent self-
> signature.
Fixesgolang/go#26449
Change-Id: Id992676ef2363779a7028f4799180efb027fcf47
Reviewed-on: https://go-review.googlesource.com/118957
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This is neither a '--clearsign' nor a '--detach-sign' which are already
supported. Verification of these signatures is already supported by
ReadMessage.
The code shares a lot with standard encrypt/sign, so mostly a
refactoring of 'Encrypt' to allow use of the code path without
actually doing a signing.
Change-Id: I5bb7487134ffcf1189ed74e28dbbbe1c01b356d1
GitHub-Last-Rev: 01162222600a4a2a230a444387b33883faf596ec
GitHub-Pull-Request: golang/crypto#50
Reviewed-on: https://go-review.googlesource.com/116017
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Signing was moved in NewEntity as it makes more sense there, but there
might be code that relies on SerializePrivate to make signatures with
parameters that were modified after NewEntity.
As it used to always sign in SerializePrivate, it shouldn't break
anything to sign in both places.
Fixesgolang/go#25463
Change-Id: Ia7f509daf31ac05fedc441225d554f333b288d70
Reviewed-on: https://go-review.googlesource.com/118015
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Yaron de Leeuw <jarondl@google.com>
Reviewed-by: Alexandre Viau <viau.alexandre@gmail.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
When failing, TestKeyExpiry would output the wrong expected key id. It
would output "Expected key 1ABB25A0" instead of "Expected key 96A672F5".
Avoid this mistake by declaring the variable only once and using it in
the error format.
Change-Id: I860d82bf2c7fa80558051cdb21a41d506e95c25f
Reviewed-on: https://go-review.googlesource.com/118958
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The existing code was wrongly assuming that UserID packets must be
immediately followed by a Signature packet. However, this is not true.
See RFC4880 11.1:
> Immediately following each User ID packet, there are zero or more
> Signature packets.
This change will ensure that Entities that are not immediately followed
by a Signature packet are read without raising a StructuralError.
Instead, UserID packets that are not immediately followed by a self
signature will be ignored.
Maximum backwards compatibility is retained because revoked UserIDs are
not added to the Entity's identities.
In a follow-up patch, we should probably add these UserIDs to the
Entity's identities too, but not without making sure that the revocation
is also available in the Entity's (or the Identity's) Revocations slice.
This would require adding support for a new Signature Type,
"Certification revocation signature", as defined in RFC 48880 5.2.1.
Fixesgolang/go#25850
Change-Id: Idde34b97429998f28e0c687171024e51ed959bf0
Reviewed-on: https://go-review.googlesource.com/118376
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously if you created a new Entity then ran `Serialize` _before_ running `SerializePrivate`, the resulting armored public key was corrupted, giving the error of `unexpected EOF`. This fix signs the public key with the private key upon creation of a NewEntity. Since SerializePrivate only is applicable to entities created with NewEntity per the docs we can also safely remove the signing portion from that function.
Fixes#25463
Change-Id: I58b808987ee173079f33bce3d6c3527f9233b2cd
GitHub-Last-Rev: 2c4b8e4d630a06d782816f8fbd3d59f01ece2565
GitHub-Pull-Request: golang/crypto#47
Reviewed-on: https://go-review.googlesource.com/114001
Reviewed-by: Filippo Valsorda <filippo@golang.org>
MPIs are (supposed to be) stripped of leading zeroes. Avoid passing
such short values into crypto/rsa, even if it currently happens to work.
Change-Id: I5a5f4813b8358e83fcc2deeda1272d2733814542
Reviewed-on: https://go-review.googlesource.com/100844
Reviewed-by: Adam Langley <agl@golang.org>
Rather than rounding the `type || X || Y` byte sequence to the next
8-bit boundary, packet.NewECDSAPublicKey() now rounds the X and Y
coordinates individually, then adds the bitlength 3 of type 4
(compressed). For NIST P-256, this leads to a bit length of 515,
rather than 520. GnuPG calculates 515 as well, and
https://tools.ietf.org/html/rfc6637#section-6 explicitly states that
"the exact size of the MPI payload is 515 bits for 'Curve P-256,'"
so the new formula is consistent.
Fixesgolang/go#23460
Change-Id: I64b340d1c761bfd795a1a64dc2f7a831c8b2ff32
Reviewed-on: https://go-review.googlesource.com/87995
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The openpgp package promotes bad defaults by not setting the
preferred cipher and hash of new entities created by
`openpgp.NewEntity`.
The preferred hash can be set by passing a `packet.Config`
with a `DefaultHash` set, but the same cannot be done for
the preferred cipher.
This change copies the DefaultCipher into the self-signature, similar to
DefaultHash.
Change-Id: I80e1289d67b7cd4079be8c1d5ba603a555dbe5c1
Reviewed-on: https://go-review.googlesource.com/66430
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Confusingly, `Identity.Name` should actually refer to `uid.Id` and not
`uid.Name`. This fixes the code in NewEntity, and creates a test case to
verify.
Change-Id: Id46de876fc7b0359faecfeb9b09a8e454710d485
Reviewed-on: https://go-review.googlesource.com/92595
Reviewed-by: Adam Langley <agl@golang.org>
None are "wrong" per se, but there are a lot of good suggestions and
in one case a docstring that was not present in godoc due to the
presence of an extra newline.
Changed "Id" in struct properties to "ID" in some non-exported
structs. Removed a trailing period from some error messages; I believe
the exact contents of error strings are not covered by the Go
compatibility promise.
Change-Id: I7c620582dc247396f72c52d38c909ccc0ec87b83
Reviewed-on: https://go-review.googlesource.com/80145
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing implementation checks to see if the session key size is a
multiple of the cipher blocksize. This fails for AES-192, which has a
keysize of 24 bytes and a 16 byte block size. Instead it should simply
check to ensure that the Session Key length is equal to the cipher
KeySize.
Fixesgolang/go#17060
Change-Id: I1dc78129f7fb2ca5ec71b650a2adcb3752dca885
Reviewed-on: https://go-review.googlesource.com/35848
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds support for crypto.Signer-based RSA and ECDSA private keys.
This enables using opaque signing keys as explained in the documentation
for crypto.Signer.
The support is in the form of a new NewSignerPrivateKey function which
makes a PrivateKey from a crypto.Signer.
Fixesgolang/go#15841.
Change-Id: Ice2ec2793a9f5409a5bfd4e5e49d919e14ede1e0
Reviewed-on: https://go-review.googlesource.com/23802
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Say the user wants to create a NewEntity, then Serialize the private and public
keys. Then call Encrypt. The Hash used first for Encrypt will be `RIPEMD160`,
which is not compiled in by default. So say they _specifically_ pass
crypto.SHA256 as the Default hash to NewEntity, this should set the
PreferredHash in the SelfSignature so they can be encrypted with that hash.
Related to golang/go#12153
Change-Id: I3d89f9d60f9abb29ec8365f81da38b8cd0594587
Reviewed-on: https://go-review.googlesource.com/23209
Reviewed-by: Adam Langley <agl@golang.org>
Signature#SignUserId was ignoring the error returned by userIdSignatureHash.
This error can happen, for example, when the Signature's Hash property is
uninitialized. Not returning the error makes it hard for the user to detect
the problem.
Change-Id: I0ae0033e77bfb1ea1c06b0769f949e48c713adc6
Reviewed-on: https://go-review.googlesource.com/27997
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Roland Shoemaker
Filippo Valsorda
cuishuang
Russ Cox
Ian Lance Taylor
Katie Hockman
Scott Ragan
Matthew Dempsky
Michael Cook
Axel Wagner
Igor Zhilianin
aviau
Paul M Furley
Thomas Bushnell, BSG
Wilfried Teiken
Ryan Canty
Mike Tsao
Lars Lehtonen
Damien Tournoud
Yaron de Leeuw
Kevin Burke
namusyaka
mdp
Joe Tsai
Samuele Pedroni
Alok Menghrajani
Jessica Frazelle
Reinaldo de Souza Jr
Martin Hamrle