Named returned values should only be used on public funcs and methods
when it contributes to the documentation.
Named return values should not be used if they're only saving the
programmer a few lines of code inside the body of the function,
especially if that means there's stutter in the documentation or it
was only there so the programmer could use a naked return
statement. (Naked returns should not be used except in very small
functions)
This change is a manual audit & cleanup of public func signatures.
Signatures were not changed if:
* the func was private (wouldn't be in public godoc)
* the documentation referenced it
* the named return value was an interesting name. (i.e. it wasn't
simply stutter, repeating the name of the type)
There should be no changes in behavior. (At least: none intended)
Change-Id: I3472ef49619678fe786e5e0994bdf2d9de76d109
Reviewed-on: https://go-review.googlesource.com/20024
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
This is minor cleanup that reduces test output noise.
Change-Id: Ib6db4daf8cb67b7784b2d5b222fa37c7f78a6a04
Reviewed-on: https://go-review.googlesource.com/19997
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is minor cleanup that makes the tests more readable.
Change-Id: I9f1f98f0f035096c284bdf3501e7520517a3e4d9
Reviewed-on: https://go-review.googlesource.com/19993
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a followup change to #13111 for filtering out IPv6 literals and
absolute FQDNs from being as the SNI values.
Updates #13111.
Fixes#14404.
Change-Id: I09ab8d2a9153d9a92147e57ca141f2e97ddcef6e
Reviewed-on: https://go-review.googlesource.com/19704
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
A comment existed referencing RC4 coming before AES because of it's
vulnerability to the Lucky 13 attack. This clarifies that the Lucky 13 attack
only effects AES-CBC, and not AES-GCM.
Fixes#14474
Change-Id: Idcb07b5e0cdb0f9257cf75abea60129ba495b5f5
Reviewed-on: https://go-review.googlesource.com/19845
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Conn.Close sends an encrypted "close notify" to signal secure EOF.
But writing that involves acquiring mutexes (handshake mutex + the
c.out mutex) and writing to the network. But if the reason we're
calling Conn.Close is because the network is already being
problematic, then Close might block, waiting for one of those mutexes.
Instead of blocking, and instead of introducing new API (at least for
now), distinguish between a normal Close (one that sends a secure EOF)
and a resource-releasing destructor-style Close based on whether there
are existing Write calls in-flight.
Because io.Writer and io.Closer aren't defined with respect to
concurrent usage, a Close with active Writes is already undefined, and
should only be used during teardown after failures (e.g. deadlines or
cancelations by HTTP users). A normal user will do a Write then
serially do a Close, and things are unchanged for that case.
This should fix the leaked goroutines and hung net/http.Transport
requests when there are network errors while making TLS requests.
Change-Id: If3f8c69d6fdcebf8c70227f41ad042ccc3f20ac9
Reviewed-on: https://go-review.googlesource.com/18572
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
During the TLS handshake, check the cipher suite the server selects is
one of those offered in the ClientHello. The code was checking it was
in the larger list that was sometimes whittled down for the ClientHello.
Fixes#13174
Change-Id: Iad8eebbcfa5027f30403b9700c43cfa949e135bb
Reviewed-on: https://go-review.googlesource.com/16698
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The user can inspect the record data to detect that the other side is
not using the TLS protocol.
This will be used by the net/http client (in a follow-on CL) to detect
when an HTTPS client is speaking to an HTTP server.
Updates #11111.
Change-Id: I872f78717aa8e8e98cebd8075436209a52039a73
Reviewed-on: https://go-review.googlesource.com/16078
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In keysFromMasterSecret(), don't copy from serverRandom into
seed[:len(clientRandom)]. Actually, switch from an array to a slice in
keysFromMasterSecret() and masterFromPreMasterSecret() so the length
need not be given; that's how it's done elsewhere in the file.
Fixes#13181
Change-Id: I92abaa892d1bba80c2d4f12776341cda7d538837
Reviewed-on: https://go-review.googlesource.com/16697
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
(This relands commit a4dcc692011bf1ceca9b1a363fd83f3e59e399ee.)
https://tools.ietf.org/html/rfc6066#section-3 states:
“Literal IPv4 and IPv6 addresses are not permitted in "HostName".”
However, if an IP literal was set as Config.ServerName (which could
happen as easily as calling Dial with an IP address) then the code would
send the IP literal as the SNI value.
This change filters out IP literals, as recognised by net.ParseIP, from
being sent as the SNI value.
Fixes#13111.
Change-Id: I6e544a78a01388f8fe98150589d073b917087f75
Reviewed-on: https://go-review.googlesource.com/16776
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
https://tools.ietf.org/html/rfc6066#section-3 states:
“Literal IPv4 and IPv6 addresses are not permitted in "HostName".”
However, if an IP literal was set as Config.ServerName (which could
happen as easily as calling Dial with an IP address) then the code would
send the IP literal as the SNI value.
This change filters out IP literals, as recognised by net.ParseIP, from
being sent as the SNI value.
Fixes#13111.
Change-Id: Ie9ec7acc767ae172b48c9c6dd8d84fa27b1cf0de
Reviewed-on: https://go-review.googlesource.com/16742
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
This change causes the types of skipped PEM blocks to be recorded when
no certificate or private-key data is found in a PEM input. This allows
for better error messages to be return in the case of common errors like
switching the certifiate and key inputs to X509KeyPair.
Fixes#11092
Change-Id: Ifc155a811cdcddd93b5787fe16a84c972011f2f7
Reviewed-on: https://go-review.googlesource.com/14054
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
https://tools.ietf.org/html/rfc7301#section-3.1 specifies that a
ProtocolName may not be empty. This change enforces this for ServerHello
messages—it's already enforced for ClientHello messages.
Change-Id: Ic5a5be6bebf07fba90a3cabd10b07ab7b4337f53
Reviewed-on: https://go-review.googlesource.com/12003
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In Go 1.5, Config.Certificates is no longer required if
Config.GetCertificate has been set. This change updated four comments to
reflect that.
Change-Id: Id72cc22fc79e931b2d645a7c3960c3241042762c
Reviewed-on: https://go-review.googlesource.com/13800
Reviewed-by: Adam Langley <agl@golang.org>
Go 1.5 allowed TLS connections where Config.Certificates was nil as long
as the GetCertificate callback was given. However, tls.Listen wasn't
updated accordingly until this change.
Change-Id: I5f67f323f63c988ff79642f3daf8a6b2a153e6b2
Reviewed-on: https://go-review.googlesource.com/13801
Reviewed-by: Adam Langley <agl@golang.org>
Could go in 1.5, although not critical.
See also #12107
Change-Id: I7f1608b58581d21df4db58f0db654fef79e33a90
Reviewed-on: https://go-review.googlesource.com/13481
Reviewed-by: Dave Cheney <dave@cheney.net>
Strengthening VerifyHostname exposed the fact that for resumed
connections, ConnectionState().VerifiedChains was not being saved
and restored during the ClientSessionCache operations.
Do that.
This change just saves the verified chains in the client's session
cache. It does not re-verify the certificates when resuming a
connection.
There are arguments both ways about this: we want fast, light-weight
resumption connections (thus suggesting that we shouldn't verify) but
it could also be a little surprising that, if the verification config
is changed, that would be ignored if the same session cache is used.
On the server side we do re-verify client-auth certificates, but the
situation is a little different there. The client session cache is an
object in memory that's reset each time the process restarts. But the
server's session cache is a conceptual object, held by the clients, so
can persist across server restarts. Thus the chance of a change in
verification config being surprisingly ignored is much higher in the
server case.
Fixes#12024.
Change-Id: I3081029623322ce3d9f4f3819659fdd9a381db16
Reviewed-on: https://go-review.googlesource.com/13164
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This change alters the certificate used in many tests so that it's no
longer self-signed. This allows some tests to exercise the standard
certificate verification paths in the future.
Change-Id: I9c3fcd6847eed8269ff3b86d9b6966406bf0642d
Reviewed-on: https://go-review.googlesource.com/13244
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
The one in misc/makerelease/makerelease.go is particularly bad and
probably warrants rotating our keys.
I didn't update old weekly notes, and reverted some changes involving
test code for now, since we're late in the Go 1.5 freeze. Otherwise,
the rest are all auto-generated changes, and all manually reviewed.
Change-Id: Ia2753576ab5d64826a167d259f48a2f50508792d
Reviewed-on: https://go-review.googlesource.com/12048
Reviewed-by: Rob Pike <r@golang.org>
The previous code had a brain fart: it took one of the length prefixes
as an element count, not a length. This didn't actually affect anything
because the loop stops as soon as it finds a hostname element, and the
hostname element is always the first and only element. (No other element
types have ever been defined.)
This change fixes the parsing in case SNI is ever changed in the future.
Fixes#10793.
Change-Id: Iafdf3381942bc22b1f33595315c53dc6cc2e9f0f
Reviewed-on: https://go-review.googlesource.com/11059
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
These were found by grepping the comments from the go code and feeding
the output to aspell.
Change-Id: Id734d6c8d1938ec3c36bd94a4dbbad577e3ad395
Reviewed-on: https://go-review.googlesource.com/10941
Reviewed-by: Aamir Khan <syst3m.w0rm@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously we enforced both that the extended key usages of a client
certificate chain allowed for client authentication, and that the
client-auth EKU was in the leaf certificate.
This change removes the latter requirement. It's still the case that the
chain must be compatible with the client-auth EKU (i.e. that a parent
certificate isn't limited to another usage, like S/MIME), but we'll now
accept a leaf certificate with no EKUs for client-auth.
While it would be nice if all client certificates were explicit in their
intended purpose, I no longer feel that this battle is worthwhile.
Fixes#11087.
Change-Id: I777e695101cbeba069b730163533e2977f4dc1fc
Reviewed-on: https://go-review.googlesource.com/10806
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
This is the second in a two-part change. See https://golang.org/cl/9415
for details of the overall change.
This change updates the supported signature algorithms to include
SHA-384 and updates all the testdata/ files accordingly. Even some of
the testdata/ files named “TLS1.0” and “TLS1.1” have been updated
because they have TLS 1.2 ClientHello's even though the server picks a
lower version.
Fixes#9757.
Change-Id: Ia76de2b548d3b39cd4aa3f71132b0da7c917debd
Reviewed-on: https://go-review.googlesource.com/9472
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Prior to TLS 1.2, the handshake had a pleasing property that one could
incrementally hash it and, from that, get the needed hashes for both
the CertificateVerify and Finished messages.
TLS 1.2 introduced negotiation for the signature and hash and it became
possible for the handshake hash to be, say, SHA-384, but for the
CertificateVerify to sign the handshake with SHA-1. The problem is that
one doesn't know in advance which hashes will be needed and thus the
handshake needs to be buffered.
Go ignored this, always kept a single handshake hash, and any signatures
over the handshake had to use that hash.
However, there are a set of servers that inspect the client's offered
signature hash functions and will abort the handshake if one of the
server's certificates is signed with a hash function outside of that
set. https://robertsspaceindustries.com/ is an example of such a server.
Clearly not a lot of thought happened when that server code was written,
but its out there and we have to deal with it.
This change decouples the handshake hash from the CertificateVerify
hash. This lays the groundwork for advertising support for SHA-384 but
doesn't actually make that change in the interests of reviewability.
Updating the advertised hash functions will cause changes in many of the
testdata/ files and some errors might get lost in the noise. This change
only needs to update four testdata/ files: one because a SHA-384-based
handshake is now being signed with SHA-256 and the others because the
TLS 1.2 CertificateRequest message now includes SHA-1.
This change also has the effect of adding support for
client-certificates in SSLv3 servers. However, SSLv3 is now disabled by
default so this should be moot.
It would be possible to avoid much of this change and just support
SHA-384 for the ServerKeyExchange as the SKX only signs over the nonces
and SKX params (a design mistake in TLS). However, that would leave Go
in the odd situation where it advertised support for SHA-384, but would
only use the handshake hash when signing client certificates. I fear
that'll just cause problems in the future.
Much of this code was written by davidben@ for the purposes of testing
BoringSSL.
Partly addresses #9757
Change-Id: I5137a472b6076812af387a5a69fc62c7373cd485
Reviewed-on: https://go-review.googlesource.com/9415
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This change causes the GetCertificate callback to be called if
Certificates is empty. Previously this configuration would result in an
error.
This allows people to have servers that depend entirely on dynamic
certificate selection, even when the client doesn't send SNI.
Fixes#9208.
Change-Id: I2f5a5551215958b88b154c64a114590300dfc461
Reviewed-on: https://go-review.googlesource.com/8792
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
The OCSP response is currently only exposed via a method on Conn,
which makes it inaccessible when using wrappers like net/http. The
ConnectionState structure is typically available even when using
wrappers and contains many of the other handshake details, so this
change exposes the stapled OCSP response in that structure.
Change-Id: If8dab49292566912c615d816321b4353e711f71f
Reviewed-on: https://go-review.googlesource.com/9361
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
This change adds a new method to tls.Config, SetSessionTicketKeys, that
changes the key used to encrypt session tickets while the server is
running. Additional keys may be provided that will be used to maintain
continuity while rotating keys. If a ticket encrypted with an old key is
provided by the client, the server will resume the session and provide
the client with a ticket encrypted using the new key.
Fixes#9994
Change-Id: Idbc16b10ff39616109a51ed39a6fa208faad5b4e
Reviewed-on: https://go-review.googlesource.com/9072
Reviewed-by: Jonathan Rudenberg <jonathan@titanous.com>
Reviewed-by: Adam Langley <agl@golang.org>
This change adds support for serving and receiving Signed Certificate
Timestamps as described in RFC 6962.
The server is now capable of serving SCTs listed in the Certificate
structure. The client now asks for SCTs and, if any are received,
they are exposed in the ConnectionState structure.
Fixes#10201
Change-Id: Ib3adae98cb4f173bc85cec04d2bdd3aa0fec70bb
Reviewed-on: https://go-review.googlesource.com/8988
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Jonathan Rudenberg <jonathan@titanous.com>
- Multiple GetCertificate tests shared the same name and were
overwriting each other, each test now has a unique name.
- expectAlert was not implemented in the data updater, the single
test that used it has been replaced with a ClientHello failure
test.
Fixes#10470
Change-Id: I500738f6302ffa863d7ee45d85fa8773155e0614
Reviewed-on: https://go-review.googlesource.com/8959
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
This is a follow on to 28f33b4a which removes one of the boolean flags
and adds a test for the key-driven cipher selection.
Change-Id: If2a400de807eb19110352912a9f467491cc8986c
Reviewed-on: https://go-review.googlesource.com/8428
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Jacob Haven <jacob@cloudflare.com>
This change replaces all direct ECDSA/RSA sign and decrypt operations
with calls through the crypto.Signer and crypto.Decrypter interfaces.
This is a follow-up to https://go-review.googlesource.com/#/c/3900/
which added crypto.Decrypter and implemented it for RSA.
Change-Id: Ie0f3928448b285f329efcd3a93ca3fd5e3b3e42d
Reviewed-on: https://go-review.googlesource.com/7804
Reviewed-by: Adam Langley <agl@golang.org>
Commit 604fa4d5 made TLS 1.0 the default minimum version. This commit
amends a comment to reflect that.
This is where the default is used in the absence of an explicit version
being set:
edadffa2f3/src/crypto/tls/common.go (L391-L393)
Change-Id: I8f1117ecdddc85bb1cc76a6834026505a380b793
Reviewed-on: https://go-review.googlesource.com/5525
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Just so that we notice in the future if another hash function is added
without updating this utility function, make it panic when passed an
unknown handshake hash function. (Which should never happen.)
Change-Id: I60a6fc01669441523d8c44e8fbe7ed435e7f04c8
Reviewed-on: https://go-review.googlesource.com/7646
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Joël Stemmer <stemmertech@gmail.com>
Commit f1d669aee994b28e1afcfe974680565932d25b70 added support for
AES_256_GCM_SHA384 cipher suites as specified in RFC5289. However, it
did not take the arbitrary hash function into account in the TLS client
handshake when using client certificates.
The hashForClientCertificate method always returned SHA256 as its
hashing function, even if it actually used a different one to calculate
its digest. Setting up the connection would eventually fail with the
error "tls: failed to sign handshake with client certificate:
crypto/rsa: input must be hashed message".
Included is an additional test for this specific situation that uses the
SHA384 hash.
Fixes#9808
Change-Id: Iccbf4ab225633471ef897907c208ad31f92855a3
Reviewed-on: https://go-review.googlesource.com/7040
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Some servers which misunderstood the point of the CertificateRequest
message send huge reply records. These records are large enough that
they were considered “insane” by the TLS code and rejected.
This change removes the sanity test for record lengths. Although the
maxCiphertext test still remains, just above, which (roughly) enforces
the 16KB protocol limit on record sizes:
https://tools.ietf.org/html/rfc5246#section-6.2.1Fixes#8928.
Change-Id: Idf89a2561b1947325b7ddc2613dc2da638d7d1c9
Reviewed-on: https://go-review.googlesource.com/5690
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Only documentation / comment changes. Update references to
point to golang.org permalinks or go.googlesource.com/go.
References in historical release notes under doc are left as is.
Change-Id: Icfc14e4998723e2c2d48f9877a91c5abef6794ea
Reviewed-on: https://go-review.googlesource.com/4060
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Generalizes PRF calculation for TLS 1.2 to support arbitrary hashes (SHA-384 instead of SHA-256).
Testdata were all updated to correspond with the new cipher suites in the handshake.
Change-Id: I3d9fc48c19d1043899e38255a53c80dc952ee08f
Reviewed-on: https://go-review.googlesource.com/3265
Reviewed-by: Adam Langley <agl@golang.org>
ECDSA is unsafe to use if an entropy source produces predictable
output for the ephemeral nonces. E.g., [Nguyen]. A simple
countermeasure is to hash the secret key, the message, and
entropy together to seed a CSPRNG, from which the ephemeral key
is derived.
Fixes#9452
--
This is a minimalist (in terms of patch size) solution, though
not the most parsimonious in its use of primitives:
- csprng_key = ChopMD-256(SHA2-512(priv.D||entropy||hash))
- reader = AES-256-CTR(k=csprng_key)
This, however, provides at most 128-bit collision-resistance,
so that Adv will have a term related to the number of messages
signed that is significantly worse than plain ECDSA. This does
not seem to be of any practical importance.
ChopMD-256(SHA2-512(x)) is used, rather than SHA2-256(x), for
two sets of reasons:
*Practical:* SHA2-512 has a larger state and 16 more rounds; it
is likely non-generically stronger than SHA2-256. And, AFAIK,
cryptanalysis backs this up. (E.g., [Biryukov] gives a
distinguisher on 47-round SHA2-256 with cost < 2^85.) This is
well below a reasonable security-strength target.
*Theoretical:* [Coron] and [Chang] show that Chop-MD(F(x)) is
indifferentiable from a random oracle for slightly beyond the
birthday barrier. It seems likely that this makes a generic
security proof that this construction remains UF-CMA is
possible in the indifferentiability framework.
--
Many thanks to Payman Mohassel for reviewing this construction;
any mistakes are mine, however. And, as he notes, reusing the
private key in this way means that the generic-group (non-RO)
proof of ECDSA's security given in [Brown] no longer directly
applies.
--
[Brown]: http://www.cacr.math.uwaterloo.ca/techreports/2000/corr2000-54.ps
"Brown. The exact security of ECDSA. 2000"
[Coron]: https://www.cs.nyu.edu/~puniya/papers/merkle.pdf
"Coron et al. Merkle-Damgard revisited. 2005"
[Chang]: https://www.iacr.org/archive/fse2008/50860436/50860436.pdf
"Chang and Nandi. Improved indifferentiability security analysis
of chopMD hash function. 2008"
[Biryukov]: http://www.iacr.org/archive/asiacrypt2011/70730269/70730269.pdf
"Biryukov et al. Second-order differential collisions for reduced
SHA-256. 2011"
[Nguyen]: ftp://ftp.di.ens.fr/pub/users/pnguyen/PubECDSA.ps
"Nguyen and Shparlinski. The insecurity of the elliptic curve
digital signature algorithm with partially known nonces. 2003"
New tests:
TestNonceSafety: Check that signatures are safe even with a
broken entropy source.
TestINDCCA: Check that signatures remain non-deterministic
with a functional entropy source.
Updated "golden" KATs in crypto/tls/testdata that use ECDSA suites.
Change-Id: I55337a2fbec2e42a36ce719bd2184793682d678a
Reviewed-on: https://go-review.googlesource.com/3340
Reviewed-by: Adam Langley <agl@golang.org>
There are two methods by which TLS clients signal the renegotiation
extension: either a special cipher suite value or a TLS extension.
It appears that I left debugging code in when I landed support for the
extension because there's a "+ 1" in the switch statement that shouldn't
be there.
The effect of this is very small, but it will break Firefox if
security.ssl.require_safe_negotiation is enabled in about:config.
(Although almost nobody does this.)
This change fixes the original bug and adds a test. Sadly the test is a
little complex because there's no OpenSSL s_client option that mirrors
that behaviour of require_safe_negotiation.
Change-Id: Ia6925c7d9bbc0713e7104228a57d2d61d537c07a
Reviewed-on: https://go-review.googlesource.com/1900
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
