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>
AnyOverlap and InexactOverlap implement checks for the aliasing
requirements defined by the crypto/cipher interfaces. Apply them to all
implementations as the actual requirement could be architecture-dependent
and user code should not rely on undefined behavior.
Updates golang/go#21624
Change-Id: I465de02fb3fec4e0c6f1fdee1ef6ae7ed5abff10
Reviewed-on: https://go-review.googlesource.com/112236
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
To comply with the specification the value of the extension should be a ASN.1
OCTET STRING rather than a raw SHA 256 hash. This change uses asn1.Marshal to
wrap the hash before putting it in the extension.
Change-Id: I4ebe88a00238c6f928555d605e4b5dd98aad8128
Reviewed-on: https://go-review.googlesource.com/118696
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A var isn't needed and a const is what the upstream (Go 1.10+) version
it's copying is.
Change-Id: I335270be3b3d09ac3c22cf6fb889a74ac39b8f1d
Reviewed-on: https://go-review.googlesource.com/116379
Reviewed-by: Filippo Valsorda <filippo@golang.org>
GetCertificate has all the information it needs to know if a client
supports ECDSA in ClientHelloInfo. Deprecate and ignore ForceRSA, and
just obtain a RSA certificate on the fly when a client that doesn't
support ECDSA connects.
This changes the cache key format to have a "+rsa" suffix for RSA
certificates. The default (ForceRSA = false) cache key is unchanged,
so most DirCache instances will still be valid. Caches created with
ForceRSA set will be silently ignored and certificates reissued.
The cache keys for HTTP tokens and the account key are changed to be
guaranteed not to overlap with valid domain names as well.
Note that ECDSA support detection is more strict in following RFC 5246
than crypto/tls, which ignores signature_algorithms.
Fixesgolang/go#22066
Change-Id: I70227747b563d6849cb693f83a950d57040b3f39
Reviewed-on: https://go-review.googlesource.com/114501
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
There's been some confusion about failed request retries.
Rightfully so: some requests are retried, others are not.
This change attempts to clarify the issue and unify backoff
usage in all Client's methods by introducing a new exported
optional field RetryBackoff and adding retry logic where missing.
Also, updates golang/go#22457.
Change-Id: Ied434edf998d52925a48b6b3b2407d45a6e9d2ee
Reviewed-on: https://go-review.googlesource.com/109615
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Most implementations, including this one, consider the private key to be
the concatenation of the initial 32-byte seed and the public key.
However the RFC 8032 formulation considers the "private key" to just be
the seed, which, in turn, means the upcoming draft-ietf-curdle-pkix
specification for embedding Ed25519 into PKCS#8 only stores the seed.
Exporting ed25519.PrivateKey to the seed is easy: key[:32]. Importing
the seed to ed25519.PrivateKey is not currently possible because the
logic is tied up in ed25519.GenerateKey. Split out
ed25519.NewKeyFromSeed for this, as well as an ed25519.PrivateKey.Seed
accessor to keep the abstraction consistent.
Change-Id: I4068eaf2073009dff3d84224aa145b56b59a5854
Reviewed-on: https://go-review.googlesource.com/115297
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes a bug introduced in golang.org/cl/100078 where incorrect
ACME client was used, causing nil pointer dereference.
The change also improves related tests,
removing code paths diverging in testing.
Fixesgolang/go#25581
Change-Id: I8c5531fcc5814a5a64f14911c0ad86c476a76d2f
Reviewed-on: https://go-review.googlesource.com/114817
Reviewed-by: Filippo Valsorda <filippo@golang.org>
We now keep track of pending authorization requests during verify() and
defer the asynchronous revocation of the ones that failed.
This should help avoid letsencrypt's "too many currently pending
authorizations" error.
Fixesgolang/go#23426
Change-Id: Ibffb10f59733962d45e43b67fc42a2ec7c5faf51
Reviewed-on: https://go-review.googlesource.com/100078
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Leo Antunes <costela@gmail.com>
Reviewed-by: Filippo Valsorda <filippo@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>
Forgotten file from CL 112635.
The integration tests didn't catch this because the integration tests
never run (sshd isn't in $PATH usually), but even when $PATH is
modified, it seems to have been failing for ages on both Debian jessie
and stretch. I'll fix the tests and make them run in TryBots in later
changes.
Change-Id: I85fd3b6109a73990bc353a61da5f527b9a698501
Reviewed-on: https://go-review.googlesource.com/113056
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The extra goroutines were distracting while debugging something else,
especially as I wasn't using that feature. This also saves a bit of
memory.
Change-Id: Ia6489e64bbd3d5a6ff699a25018676d8ff8bd2e4
Reviewed-on: https://go-review.googlesource.com/112635
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
The tag matches the platforms defined in util*.go
where the most tested logic is defined.
Change-Id: I90f67d988c795738c3effbc8554a933a7cb355d2
Reviewed-on: https://go-review.googlesource.com/112555
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Changes function name from "testUnalignedWrites" to "TestUnalignedWrites"
which makes "go test" actually run this test.
Change-Id: Ie46f11ee1fd5c8780d5704d2fabe6ddb229262f9
Reviewed-on: https://go-review.googlesource.com/111282
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When c = ∞, z = 0 and 0 does not have a modular inverse, triggering
undefined behavior (recently changed to returning nil) in ModInverse.
Unclear how this used to work anyway. Looks like ModInverse was
leaving the receiver untouched, making zInv = 0 when pool = nil.
Fixesgolang/go#25199
Change-Id: Ib39abf59f0e71cf43cdb5836142ebdd3b206fb3f
Reviewed-on: https://go-review.googlesource.com/110695
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
CL 110355 switched out the adhoc cpu feature detection for x/sys/cpu, in
doing so the AVX2 check was broken. The assembly code uses MULX which is
part of BMI2.
Updates golang/go#24843
Change-Id: I4719b8ff3211eb1c823099512e593e540d6f3be8
GitHub-Last-Rev: 70542b53cd09fcae26c3579cd84bcee39a4b8a57
GitHub-Pull-Request: golang/crypto#44
Reviewed-on: https://go-review.googlesource.com/110796
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Doesn't matter because contentKey and lengthKey are the same length,
but we should really be looping over lengthKey here.
Change-Id: Ic7af3c553eb4382239bec1c030e77c7f5baa0662
Reviewed-on: https://go-review.googlesource.com/109735
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Some OCSP responders contain more than one certificate, which can be
used to "help the OCSP client verify the responders signature" (RFC
6960 section 4.2.1). This client doesn't do verification of the chain
to the root, but it's not an error for a responder to send more than
one.
Fixesgolang/go#21527
Change-Id: Ie23cfcb347a4f7cdfb1a0cbad2aa03a1242553af
Reviewed-on: https://go-review.googlesource.com/57510
Reviewed-by: Adam Langley <agl@golang.org>
Unlock compares the length of the passphrase with the given one before
calling subtle.ConstantTimeCompare. This is redundant, since
ConstantTimeCompare already perform a lengths check before doing
anything. Remove the check from Unlock.
Updates golang/go#25173
Change-Id: Ib5fec3a94392bddf2996f5c6bf5a414529e86f2f
Reviewed-on: https://go-review.googlesource.com/110068
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
SIMD implementations of ChaCha20 (such as CL 35842) interleave block
computations in order to achieve high performance. This means that
they produce more than 64 bytes of output at a time. Unfortunately
when encrypting small amounts of data (such as Poly1305 keys) the
current interface to ChaCha20 forces the additional encrypted blocks
of output to be discarded and recomputed later since it does not
maintain any state. This additional overhead slows down the encryption
of small amounts of data when using such optimized code.
This CL makes the generic ChaCha20 implementation stateful, caching
key, nonce and counter values and buffering any unused key stream bytes.
ChaCha20 now also implements the high level cipher.Stream interface
which makes the API more consistent with other stream ciphers in the
standard library's crypto package. This will make it easier to add high
performance SIMD implementations in the future.
In addition to modifying the API I have also added some optimizations
to improve the performance of the generic implementation. Note that
the performance will improve further on amd64 with Go 1.11 due to
CL 95475 (binary.LittleEndian.PutUint32 optimization). These benchmarks
are based on Go 1.10.1.
name old speed new speed delta
ChaCha20/32 174MB/s ± 2% 174MB/s ± 1% ~ (p=0.796 n=10+10)
ChaCha20/63 309MB/s ± 1% 337MB/s ± 2% +9.32% (p=0.000 n=10+9)
ChaCha20/64 299MB/s ± 2% 350MB/s ± 1% +17.12% (p=0.000 n=9+8)
ChaCha20/256 297MB/s ± 2% 390MB/s ± 1% +31.40% (p=0.000 n=10+10)
ChaCha20/1024 300MB/s ± 0% 400MB/s ± 3% +33.38% (p=0.000 n=7+10)
ChaCha20/1350 290MB/s ± 1% 386MB/s ± 2% +33.10% (p=0.000 n=9+10)
ChaCha20/65536 301MB/s ± 1% 416MB/s ± 2% +38.25% (p=0.000 n=9+10)
ChaCha20-Poly1305 (AEAD optimizations manually disabled):
name old speed new speed delta
Chacha20Poly1305Open_64 122MB/s ± 7% 131MB/s ± 2% +7.23% (p=0.000 n=18+18)
Chacha20Poly1305Seal_64 125MB/s ± 4% 137MB/s ± 2% +9.88% (p=0.000 n=20+19)
Chacha20Poly1305Open_1350 244MB/s ± 4% 305MB/s ± 3% +25.04% (p=0.000 n=20+19)
Chacha20Poly1305Seal_1350 242MB/s ± 3% 309MB/s ± 2% +27.56% (p=0.000 n=20+19)
Chacha20Poly1305Open_8K 260MB/s ± 7% 338MB/s ± 3% +29.96% (p=0.000 n=20+19)
Chacha20Poly1305Seal_8K 262MB/s ± 5% 335MB/s ± 4% +27.80% (p=0.000 n=20+19)
No change in allocations for either set of benchmarks.
Change-Id: I28ca7947904e9d79debe2d5aac6623526fe5e595
Reviewed-on: https://go-review.googlesource.com/104856
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change adds two functions to cryptobyte to encode and decode
int64 with tags supplied by the user.
This change also modifies a documentation comment which was outdated.
Fixesgolang/go#24973
Change-Id: I2e3ca475891ba62df902f33085719f94e87a27cc
GitHub-Last-Rev: cd0300d9bfbd4e38f22503a046ea18f9ddd37676
GitHub-Pull-Request: golang/crypto#42
Reviewed-on: https://go-review.googlesource.com/108456
Reviewed-by: Adam Langley <agl@golang.org>
CL 106235 removed runtime.support_avx on which blake2b depends to
determine AVX support. Switch to use the corresponding feature flags
from internal/cpu instead. While at it, also use AVX2 and SSE4 (for Go
1.7 and later) from there.
Change-Id: I4f7a27fcfa0c5d0d7a50444c7aa32519b97e90dd
Reviewed-on: https://go-review.googlesource.com/106336
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change fixes a minor misspelling in the documentation for ReadBytes.
Change-Id: Ia1cfedfce11844c4d901b9aeed52065f3749dfa2
GitHub-Last-Rev: d1560b97807871c356d1b704d4879b1cf94a3fd8
GitHub-Pull-Request: golang/crypto#40
Reviewed-on: https://go-review.googlesource.com/106116
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This fixes the following vulnerability scenario:
* Victim logs into SAFE-HOST on SAFE-IP-ADDRESS regularly.
* Victim is cajoled into connecting to attacker controlled
ATTACK-HOST, on ATTACK-IP-ADDRESS. ATTACK-HOST uses a different host
key type (e.g. Ed25519 vs RSA). The new key is added at the end of
known_hosts.
* Attacker makes DNS system return ATTACK-IP-ADDRESS for SAFE-HOST.
* Victim logs into SAFE-HOST, but is not warned because the host key
matches ATTACK-IP-ADDRESS.
For this attack to work, the key type has to be different, because
knownhosts gives precedence to the first key found for each type. Add
a test that asserts this behavior.
The new semantics simplify the code, but callers that modify
.ssh/known_host interactviely must now take an extra step to remain
OpenSSH compatible: on successful login, the IP address must be
checked without hostname, and if it is not known, added separately to
the known_hosts file, so future logins that use an IP address only
will be protected too.
Thanks to Daniel Parks <security@demonhorse.net> for finding this
vulnerability.
Change-Id: I62b1b60ceb02e2f583a4657213feac1a8885dd42
Reviewed-on: https://go-review.googlesource.com/104939
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The extra Close call runs in a separate goroutine, and races with the
deferred Close call, leading to test flakes if the extra call fails
Change-Id: Ia1a2fb3932906da7cd0a8854b55f82785bb33716
Reviewed-on: https://go-review.googlesource.com/104335
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The marshal method allows the hash's internal state to be serialized and
unmarshaled at a later time, without having the re-write the entire stream
of data that was already written to the hash.
Fixesgolang/go#24548
Change-Id: I82358c34181fc815f85d5d1509fb2fe0e62e40bd
Reviewed-on: https://go-review.googlesource.com/103241
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
This change adds the equivalents of crypto_sign, crypto_sign_open and
crypto_sign_keypair in TweetNaCl and libsodium using the Ed25519 system.
The original NaCl codebase does not contain functions with identical
semantics but its documentation stated the intent of using Ed25519 in
future releases.
Fixesgolang/go#24350
Change-Id: I4c3c86b4875f2f718ad9299c2274b4ad9e11fbeb
Reviewed-on: https://go-review.googlesource.com/100578
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
aviau
Filippo Valsorda
Roland Shoemaker
Maciej Dębski
Brad Fitzpatrick
Alex Vaghin
David Benjamin
Tobias Klauser
Leo Antunes
Ryan Canty
Michael Munday
bill_ofarrell
Hana (Hyang-Ah) Kim
Leon Klingele
Filip Haglund
Kris Kwiatkowski
Tom Thorogood
Joe Shaw
Andreas Auernhammer
Alberto Donizetti
Agniva De Sarker
Andrey Mirtchovski
Han-Wen Nienhuys
ValarDragon
Mark Rushakoff
Florian Uekermann