Update golang.org/x dependencies to their latest tagged versions.
Once this CL is submitted, and post-submit testing succeeds on all
first-class ports across all supported Go versions, this repository
will be tagged with its next minor version.
Change-Id: Ic7c0afcece0f3d2065c7a7e08f092c4344d90655
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/448755
Run-TryBot: Gopher Robot <gobot@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Gopher Robot <gobot@golang.org>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
Update golang.org/x dependencies to their latest tagged versions.
Once this CL is submitted, and post-submit testing succeeds on all
first-class ports across all supported Go versions, this repository
will be tagged with its next minor version.
Change-Id: If840eea1cadc749ce55efd88eb7d9fc38472839e
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/443996
Auto-Submit: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Gopher Robot <gobot@golang.org>
Block when creating the renewal timer, rather than doing it in a
goroutine. This fixes an issue where startRenew and stopRenew are called
very closely together, and due to lock ordering, stopRenew may be called
before startRenew, resulting in the appearance that the renewal timer
has been stopped before it has actually been created.
This is only an issue in tests, as that is the only place stopRenew is
actually used. In particular this issue manifests in TestGetCertiifcate
sub-tests, where a httptest server reuses a port across two of the
sub-tests. In this case, the renewal calls end up creating dirty state
for the subsequent test, which can cause confusing behavior (such as
attempting to register an account twice.)
Another solution to this problem would be introducing a bool, protected
by renewalMu, which indicates if renewal has been halted, and to check
it in startRenew to check if stopRenew has already been called, which
would allow us to continue calling startRenew in a goroutine and relying
on renewalMu locking for ordering. That said I don't see a particularly
strong reason to call startRenew concurrently, so this seems like the
simplest solution for now.
Fixesgolang/go#52494
Change-Id: I95420d3fd877572a0b9e408d2f8cd353f6a4e80e
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/433016
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Alongside the existing ECDH tests, add tests that use the new
crypto/ecdh package. The test vectors include a number of private
that use non-standard sizes, which we reject, but aren't flagged,
so we need to skip them.
Change-Id: Iaaef225b0149a86833095f51748d230385d43bfe
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/424274
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
In 2009, Google's open-source lawyers asked us to create the AUTHORS
file to define "The Go Authors", and the CONTRIBUTORS file was in
keeping with open source best practices of the time.
Re-reviewing our repos now in 2022, the open-source lawyers are
comfortable with source control history taking the place of the
AUTHORS file, and most open source projects no longer maintain
CONTRIBUTORS files.
To ease maintenance, remove AUTHORS and CONTRIBUTORS from all repos.
For golang/go#53961.
Change-Id: Ieb32933de4f234c77f0131490d4081b6c336820c
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/419094
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Don't assume the path passed into the DirCache methods is absolute, and
clean it before further operating on it. Put and Delete are not attacker
controlled, but clean them anyway.
Fixes#53082
Fixes CVE-2022-30636
Change-Id: I755f525a737da60ccba07ebce4d41cc8faebfcca
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/408694
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently discover is not called which results in a panic if just a key
is added to an ACME client and then deactivation is attempted.
This patch adds a discover call as well as missing unit tests for the
API.
Change-Id: I0719e5376eb2fccf62182e5f91e5b5eaa7bdd518
GitHub-Last-Rev: 501d7c6c1b75a3069dcad4254b4d4a0d2ccb02c8
GitHub-Pull-Request: golang/crypto#217
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/406734
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Add support for AccountKeyRollover. API only returns an error since acme.Error
will contain appropriate KID lookup information. Due to the requirements
of double JWS encoding jwsEncodeJSON is also modified to support a
missing Nonce header and raw string embedding in the payload.
Fixesgolang/go#42516
Change-Id: I959660a1a39b2c469b959accd48fda519daf4eb3
GitHub-Last-Rev: 8e8cc5b094743262939c145f56d3a3b57a057d64
GitHub-Pull-Request: golang/crypto#215
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/400274
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
In the test server, rather than spawning a goroutine to validate
challenges, block on the validation before responding to the client.
This prevents a test race, where testing.T.Logf is called after the
test is completed.
While this has a slight behavioral difference to some production
ACME server implementations (although is behavior allowed in the spec),
the change has little material impact on what we are testing, since
previously the validation would happen so quickly that it would be
indistinguishable from the new blocking behavior (i.e. we would not be
sending multiple requests during polling previously.)
Fixesgolang/go#52170
Change-Id: I75e3b2da69ddc2302be25a99f1b1151ed0f4af9b
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/405548
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
In accordance to RFC8308, send ext-info-c only during the first key
exchange. Some server implementations such as OpenSSH 7 will send an
extInfoMsg message each time when ext-info-c is received. This results
in a closed connection, as our client does not expect this message while
handling the mux.
See https://bugzilla.mindrot.org/show_bug.cgi?id=2929 regarding the
behaviour of OpenSSH if it sees ext-info-c in later key exchanges.
Fixesgolang/go#51808
Change-Id: Id94f1ef73cec6147136246b0b6048b57db92660d
GitHub-Last-Rev: fcfe5ed37306136219854031abc809e0dc9b3124
GitHub-Pull-Request: golang/crypto#208
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/394134
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
CL 220037 had implemented support for host authentication using
rsa-sha2-256/512, but not client public key authentication. OpenSSH
disabled the SHA-1 based ssh-rsa by default in version 8.8 (after
pre-announcing it in versions 8.2, 8.3, 8.4, 8.5, 8.6, and 8.7) although
some distributions re-enable it. GitHub will start rejecting ssh-rsa for
keys uploaded before November 2, 2021 on March 15, 2022.
https://github.blog/2021-09-01-improving-git-protocol-security-github/
The server side already worked, as long as the client selected one of
the SHA-2 algorithms, because the signature flowed freely to Verify.
There was however nothing verifying that the signature algorithm matched
the advertised one. The comment suggested the check was being performed,
but it got lost back in CL 86190043. Not a security issue because the
signature had to pass the callback's Verify method regardless, and both
values were checked to be acceptable.
Tested with OpenSSH 8.8 configured with "PubkeyAcceptedKeyTypes -ssh-rsa"
and no application-side changes.
The Signers returned by ssh/agent (when backed by an agent client)
didn't actually implement AlgorithmSigner but ParameterizedSigner, an
interface defined in an earlier version of CL 123955.
Updates golang/go#49269Fixesgolang/go#39885
For golang/go#49952
Change-Id: I13b41db8041f1112a70f106c55f077b904b12cb8
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392394
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
The server side implementation was not actually populating the
SSH_MSG_USERAUTH_INFO_REQUEST field with the
KeyboardInteractiveChallenge argument, although the client side was
deserializing it and passing it to the KeyboardInteractiveChallenge
callback.
Anyway, the first field of SSH_MSG_USERAUTH_INFO_REQUEST is "name", not
"user". Maybe the confusion was due to the first field of
SSH_MSG_USERAUTH_REQUEST being the user.
RFC 4256, Section 3.3, says this about it
One possibility is to use the name field (possibly
prefixed with the application's name) as the title of a dialog window
in which the prompt(s) are presented.
and examples include "CRYPTOCard Authentication", "Password
Authentication", and "Password Expired".
Co-authored-by: Kevin Wallace <kevin@pentabarf.net>
Change-Id: Ic6ec0dfea2122704603c44f42898a980689a15c9
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/372234
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
The server implementation looks at the HostKeys to advertise and
negotiate host key signature algorithms. A fundamental issue of the
Signer and AlgorithmSigner interfaces is that they don't expose the
supported signature algorithms, so really the server has to guess.
Currently, it would guess exclusively based on the PublicKey.Type,
regardless of whether the host key implemented AlgorithmSigner. This
means that a legacy Signer that only supports ssh-rsa still led the
server to negotiate rsa-sha2 algorithms. The server would then fail to
find a suitable host key to make the signature and crash.
This won't happen if only Signers from this package are used, but if a
custom Signer that doesn't support SignWithAlgorithm() but returns
"ssh-rsa" from PublicKey().Type() is used as a HostKey, the server is
vulnerable to DoS.
The only workable rules to determine what to advertise seems to be:
1. a pure Signer will always Sign with the PublicKey.Type
2. an AlgorithmSigner supports all algorithms associated with the
PublicKey.Type
Rule number two means that we can't add new supported algorithms in the
future, which is not great, but it's too late to fix that.
rsaSigner was breaking rule number one, and although it would have been
fine where it's used, I didn't want to break our own interface contract.
It's unclear why we had separate test key entries for rsa-sha2
algorithms, since we can use the ssh-rsa key for those. The only test
that used them, TestCertTypes, seemed broken: the init was actually
failing at making the corresponding signers rsaSigners, and indeed the
test for the SHA-256 signer expected and checked a SHA-512 signature.
Pending CVE
For golang/go#49952
Change-Id: Ie658eefcadd87906e63fc7faae8249376aa96c79
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392355
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
RFC 8332, Section 2 sets up two overlapping namespaces: public key
formats and public key algorithms.
* The formats are what we currently have KeyAlgo constants for, and they
appear in PublicKey.Type.
* The algorithms are the set of both KeyAlgo and SigAlgo constants, and
they appear in Signature.Format (amongst other places).
This is incoherent, because that means Signature.Format can be both a
KeyAlgo (like KeyAlgoECDSA256) or a SigAlgo (like SigAlgoRSASHA2256).
One solution would be to duplicate all the KeyAlgo constants into the
SigAlgo namespace, but that would be confusing because applications are
currently using KeyAlgos where they'd be supposed to use the new
SigAlgos (while we can't deprecate the KeyAlgos because they are still
necessary for the PublicKey.Type namespace).
Instead, drop the separate namespaces, and use KeyAlgos throughout.
There are simply some KeyAlgos that can't be a PublicKey.Type.
Take the opportunity to fix the stuttering SHA22565/SHA2512 names. It's
totally ok to call those hashes SHA-256 and SHA-512 without the family
infix.
For golang/go#49952
Change-Id: Ia1fce3912a7e60aa70a88f75ed311be331fd19d5
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392354
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
The previous code
if gex.g.Cmp(one) != 1 && gex.g.Cmp(pMinusOne) != -1 {
deobfuscates to the classic mistake
if g <= 1 && g >= p - 1 {
which is never true.
What the code actually intended to do is
if gex.g.Cmp(one) != 1 || gex.g.Cmp(pMinusOne) != -1 {
or more readably and consistently with the diffieHellman method
if gex.g.Cmp(one) <= 0 || gex.g.Cmp(pMinusOne) >= 0 {
Now, is this a security issue? The incorrect checks apply to g and k,
but not what we call Y and the spec calls f. RFC 4419 says:
Either side MUST NOT send or accept e or f values that are not in the
range [1, p-1]. If this condition is violated, the key exchange
fails. To prevent confinement attacks, they MUST accept the shared
secret K only if 1 < K < p - 1.
Note that RFC 8268, Section 4 updates the equivalent RFC 4253 statement
(although not the RFC 4419 one) about e and f, but we are already doing
the correct full check there.
DH Public Key values MUST be checked and both conditions:
1 < e < p-1
1 < f < p-1
MUST be true. Values not within these bounds MUST NOT be sent or
accepted by either side. If either one of these conditions is
violated, then the key exchange fails.
This simple check ensures that:
o The remote peer behaves properly.
o The local system is not forced into the two-element subgroup.
The check on K seems like a proxy for checking a number of ways to fix
the DH output (for example by manipulating g) to one of 0, 1, or -1.
This should not be meaningful to security for two reasons:
- all parameters end up in the "transcript" hash that will get signed
by the server's host key, and if the attacker controls the host
key's signature, they have the ability to MitM without resorting to
confinement attacks
- the client secret is ephemeral, so leaking bits of it by forcing it
into small sub-groups does not gain the attacker anything, as the
secret does not get reused
Indeed, this is the same explanation of why it's ok not to check that p
is indeed a (safe) prime, which even OpenSSH omits. Building an
equivalent attack by manipulating p instead of g is left as an exercise
to the reader.
For the future, this is a case study in why we should not add complexity
even when it looks easy enough to do. CL 174257 added the
diffie-hellman-group-exchange kex. That introduced a data race (arguably
a security issue), which was fixed in CL 222078. Then it was too slow,
which led to CL 252337 that removed the primalty check, which required a
full analysis of whether it's safe to skip it, and checking against
other implementations. Now we find there's a bug and we have to do
another security analysis that not even the RFC bothered to do in order
to decide if it's a security issue. My decision in
https://github.com/golang/go/issues/17230#issuecomment-489163656
does not look like the right one in hindsight.
While at it, clean up the code some
- drop useless bit size bounds logic in the server stub that get
ignored by the rest of the function
- make p and g local variables instead of method fields, since they
are not persistent state (this was originally a data race which was
fixed in CL 222078 by making Client not a pointer receiver)
Updates golang/go#17230
Change-Id: I4b1c68537109f627ccd75ec381dcfab57ce1768c
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392015
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
RFC 9142 made diffie-hellman-group14-sha256 from RFC 8268 a MUST, and
it's strictly better than diffie-hellman-group14-sha1, which we already
have, and trivial to add.
> The method of key exchange used for the name "diffie-hellman-
> group14-sha256" is the same as that for "diffie-hellman-group14-sha1"
> except that the SHA256 hash algorithm is used.
Ignore the bigger groups which have a meaningful performance cost, and
don't share the same interoperability benefit.
Adapted from CL 387994.
Fixesgolang/go#31731
Co-authored-by: Nicola Murino <nicola.murino@gmail.com>
Change-Id: Id4ce345a2065840f193986739ea890f105a1e929
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392014
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
If we are going to send the disconnect message and close the connection,
because we have exhausted the number of authentication attempts, don't
send the final authentication failure message. This behavior matches
that of OpenSSH.
Fixesgolang/go#51149
Change-Id: I43b2de2e854f789161cd7fd6c05876661adfb2c1
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/385236
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestRenewFromCache and TestRenewFromCacheAlreadyRenewed had several
races and API misuses:
1. They called t.Fatalf from a goroutine other than the one invoking
the Test function, which is explicitly disallowed (see
https://pkg.go.dev/testing#T).
2. The test did not stop the renewal timers prior to restoring
test-hook functions, and the process of stopping the renewal timers
itself did not wait for in-flight calls to complete. That could
cause data races if one of the renewals failed and triggered a
retry with a short-enough randomized backoff.
(One such race was observed in
https://build.golang.org/log/1a19e22ad826bedeb5a939c6130f368f9979208a.)
3. The testDidRenewLoop hooks accessed the Manager.renewal field
without locking the Mutex guarding that field.
4. TestGetCertificate_failedAttempt set a testDidRemoveState hook, but
didn't wait for the timers referring to that hook to complete
before restoring it, causing races with other timers. I tried
pulling on that thread a bit, but couldn't untangle the numerous
untracked goroutines in the package. Instead, I have made a smaller
and more local change to copy the value of testDidRemoveState into
a local variable in the timer's closure.
Given the number of untracked goroutines in this package, it is likely
that races and/or deadlocks remain. Notably, so far I have been unable
to spot the actual cause of golang/go#51080.
For golang/go#51080
Change-Id: I7797f6ac34ef3c272f16ca805251dac3aa7f0009
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/384594
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
At Let's Encrypt, we've found that most autocert users do not provide an email address, which makes it hard to get in touch when things go wrong with their client. Demonstrating how to provide an email will probably encourage more people to provide one.
Change-Id: I0aa1ecea7cfbe55ae155804194fb57a8e695c658
GitHub-Last-Rev: 902b5fe2682a6be5af91e886272fd680fd67a94e
GitHub-Pull-Request: golang/crypto#186
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/323313
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
cui fliter
Gopher Robot
Park Zhou
Alexander Yastrebov
Axel Wagner
Brad Fitzpatrick
Roland Shoemaker
edef
hopehook
Russ Cox
Aaqa Ishtyaq
Jason Baker
Tobias Klauser
Ben Burkert
Peter Verraedt
Михаил ПатинandFilippo Valsorda
Bryan C. Mills
Ian Lance Taylor
Jacob Hoffman-Andrews
HowJmay