Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

[インデックス 18938] ファイルの概要

このコミットでは、Go言語の標準ライブラリであるcrypto/tlsパッケージと、そのテストコードが変更されています。具体的には以下のファイルが影響を受けています。

  • src/pkg/crypto/tls/conn.go: TLSコネクションの読み取り処理を司る主要なファイル。
  • src/pkg/crypto/tls/tls_test.go: crypto/tlsパッケージのテストコード。

コミット

commit f61f18d694028e5dd466dde11aa1c84bb3a434ed
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Tue Mar 25 10:58:35 2014 -0700

    crypto/tls: make Conn.Read return (n, io.EOF) when EOF is next in buffer
    
    Update #3514
    
    An io.Reader is permitted to return either (n, nil)
    or (n, io.EOF) on EOF or other error.
    
    The tls package previously always returned (n, nil) for a read
    of size n if n bytes were available, not surfacing errors at
    the same time.
    
    Amazon's HTTPS frontends like to hang up on clients without
    sending the appropriate HTTP headers. (In their defense,
    they're allowed to hang up any time, but generally a server
    hangs up after a bit of inactivity, not immediately.) In any
    case, the Go HTTP client tries to re-use connections by
    looking at whether the response headers say to keep the
    connection open, and because the connection looks okay, under
    heavy load it's possible we'll reuse it immediately, writing
    the next request, just as the Transport's always-reading
    goroutine returns from tls.Conn.Read and sees (0, io.EOF).
    
    But because Amazon does send an AlertCloseNotify record before
    it hangs up on us, and the tls package does its own internal
    buffering (up to 1024 bytes) of pending data, we have the
    AlertCloseNotify in an unread buffer when our Conn.Read (to
    the HTTP Transport code) reads its final bit of data in the
    HTTP response body.
    
    This change makes that final Read return (n, io.EOF) when
    an AlertCloseNotify record is buffered right after, if we'd
    otherwise return (n, nil).
    
    A dependent change in the HTTP code then notes whether a
    client connection has seen an io.EOF and uses that as an
    additional signal to not reuse a HTTPS connection. With both
    changes, the majority of Amazon request failures go
    away. Without either one, 10-20 goroutines hitting the S3 API
    leads to such an error rate that empirically up to 5 retries
    are needed to complete an API call.
    
    LGTM=agl, rsc
    R=agl, rsc
    CC=golang-codereviews
    https://golang.org/cl/76400046

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/f61f18d694028e5dd466dde11aa1c84bb3a434ed

元コミット内容

crypto/tls: make Conn.Read return (n, io.EOF) when EOF is next in buffer

Update #3514

An io.Reader is permitted to return either (n, nil)
or (n, io.EOF) on EOF or other error.

The tls package previously always returned (n, nil) for a read
of size n if n bytes were available, not surfacing errors at
the same time.

Amazon's HTTPS frontends like to hang up on clients without
sending the appropriate HTTP headers. (In their defense,
they're allowed to hang up any time, but generally a server
hangs up after a bit of inactivity, not immediately.) In any
case, the Go HTTP client tries to re-use connections by
looking at whether the response headers say to keep the
connection open, and because the connection looks okay, under
heavy load it's possible we'll reuse it immediately, writing
the next request, just as the Transport's always-reading
goroutine returns from tls.Conn.Read and sees (0, io.EOF).

But because Amazon does send an AlertCloseNotify record before
it hangs up on us, and the tls package does its own internal
buffering (up to 1024 bytes) of pending data, we have the
AlertCloseNotify in an unread buffer when our Conn.Read (to
the HTTP Transport code) reads its final bit of data in the
HTTP response body.

This change makes that final Read return (n, io.EOF) when
an AlertCloseNotify record is buffered right after, if we'd
otherwise return (n, nil).

A dependent change in the HTTP code then notes whether a
client connection has seen an io.EOF and uses that as an
additional signal to not reuse a HTTPS connection. With both
changes, the majority of Amazon request failures go
away. Without either one, 10-20 goroutines hitting the S3 API
leads to such an error rate that empirically up to 5 retries
are needed to complete an API call.

LGTM=agl, rsc
R=agl, rsc
CC=golang-codereviews
https://golang.org/cl/76400046

変更の背景

このコミットは、Go言語のHTTPクライアントがAmazonのHTTPSフロントエンド（特にS3 APIなど）と通信する際に発生していた、高いエラーレートの問題を解決するために導入されました。

問題の根源は以下の点にありました。

  1. AmazonのHTTPSフロントエンドの挙動: Amazonのサーバーは、HTTPヘッダーを適切に送信せずにクライアントとの接続を切断することがありました。これは、サーバーがいつでも接続を切断できるという点では許容される挙動ですが、通常は一定期間の非アクティブ状態の後に切断されるのが一般的です。
  2. Go HTTPクライアントの接続再利用ロジック: GoのHTTPクライアントは、パフォーマンス向上のために接続の再利用（Keep-Alive）を試みます。レスポンスヘッダーが接続を維持するように示している場合、接続が正常に見えるため、高負荷時にはその接続をすぐに再利用して次のリクエストを書き込む可能性がありました。
  3. TLS AlertCloseNotifyの扱い: Amazonのサーバーは、接続を切断する前にTLSのAlertCloseNotifyレコードを送信していました。crypto/tlsパッケージは内部でデータをバッファリング（最大1024バイト）するため、HTTPレスポンスボディの最後のデータを読み取る際に、このAlertCloseNotifyが未読のバッファに残っている状態になることがありました。
  4. tls.Conn.Readの既存の挙動: 以前のtls.Conn.Readの実装では、nバイトのデータが利用可能であれば、エラーを同時に通知することなく(n, nil)を返していました。このため、バッファにAlertCloseNotifyが存在していても、HTTPトランスポート層はそれを即座にEOFとして認識できませんでした。

結果として、HTTPクライアントは接続がまだ有効であると誤認し、切断された接続に次のリクエストを送信しようとして、エラー（例えば、0, io.EOF）を受け取ることになります。このタイムラグと誤認識が、特にS3 APIを叩くような高並行なシナリオで、10-20のゴルーチンが動作すると、API呼び出しを完了するために最大5回のリトライが必要になるほどの高いエラーレートを引き起こしていました。

このコミットは、tls.Conn.ReadがAlertCloseNotifyがバッファに存在する場合に(n, io.EOF)を返すように変更することで、この問題を解決しようとしました。これにより、HTTPクライアントは接続が閉じられたことをより早く検知し、再利用を避けることができるようになります。

前提知識の解説

このコミットの変更を理解するためには、以下の技術的な概念を理解しておく必要があります。

1. Goのio.ReaderインターフェースとEOFの扱い

Go言語のio.Readerインターフェースは、バイトストリームからの読み取り操作を抽象化する基本的なインターフェースです。その唯一のメソッドはRead(p []byte) (n int, err error)です。

  • n: 読み取られたバイト数。len(p)より小さい場合があります。
  • err: 読み取り中に発生したエラー。

io.Readerの重要な契約の一つは、Readメソッドが非ゼロのバイト数 (n > 0) と同時にio.EOFエラーを返すことができるという点です。これは、バッファに残っていた最後のデータを読み取り終え、同時にストリームの終端に達したことを意味します。この場合、呼び出し元はまずnバイトのデータを処理し、その後にio.EOFをエラーとして扱う必要があります。その後のRead呼び出しでは、0, io.EOFが返されることが期待されます。

以前のtls.Conn.Readは、データが利用可能であればエラーを同時に返さず(n, nil)を返していたため、このio.EOFの同時通知の機会を逃していました。

2. TLS AlertCloseNotifyレコード

TLS (Transport Layer Security) プロトコルでは、セキュアな接続の正常な終了を通知するために「アラートプロトコル」を使用します。そのアラートの一つがAlertCloseNotify（またはclose_notify）です。

  • 目的: close_notifyアラートは、送信側が現在のTLS接続でこれ以上アプリケーションデータを送信しないことを受信側に通知するために使用されます。これにより、両側がセキュアなセッションの終了を秩序正しく認識し、接続を正常にシャットダウンできます。
  • トランケーション攻撃の防止: close_notifyの主な目的の一つは、「トランケーション攻撃」を防ぐことです。これは、攻撃者がTLS層に知られることなく、基盤となるTCP接続を途中で切断（例: TCP FINパケットの注入）する攻撃です。close_notifyがない場合、一方のピアはセッションがまだアクティブであると信じている間に、もう一方のピアはすでに終了している可能性があり、セキュリティ上の脆弱性につながる可能性があります。
  • 挙動: close_notifyを受け取ったピアは、自身のclose_notifyを返信し、その後接続を閉じることがTLS仕様で期待されています。

このコミットの文脈では、Amazonのサーバーが接続を切断する前にこのAlertCloseNotifyを送信していたことが重要です。

3. HTTP接続の再利用 (Keep-Alive)

HTTP/1.1では、複数のHTTPリクエスト/レスポンスを単一のTCP接続上で送受信できる「持続的接続（Persistent Connections）」または「Keep-Alive」というメカニズムが導入されました。

  • 目的: 新しいHTTPリクエストごとにTCP接続を確立・切断するオーバーヘッド（TCPハンドシェイク、スロースタートなど）を削減し、パフォーマンスを向上させます。
  • Goのnet/httpクライアント: Goのnet/httpパッケージのクライアントは、デフォルトで接続の再利用を積極的に行います。レスポンスボディが完全に読み取られ、閉じられた場合、その基盤となるTCP接続は接続プールに戻され、同じホストへの次のリクエストで再利用されます。
  • 問題点: サーバーが予期せず接続を切断した場合、クライアントがその切断を即座に認識できないと、再利用された接続に次のリクエストを送信してしまい、エラーが発生します。

4. TLS内部バッファリング

crypto/tlsパッケージは、TLSレコードの処理のために内部でデータをバッファリングします。これは、ネットワークから読み取ったバイトをTLSレコードとして解析し、アプリケーションデータとアラートなどの制御データを分離するために必要です。このバッファリングにより、AlertCloseNotifyのような制御レコードが、アプリケーションデータがすべて読み取られた後も、TLS層の内部バッファに残存する可能性があります。

技術的詳細

このコミットの技術的な核心は、crypto/tlsパッケージのConn.Readメソッドの挙動を変更し、io.Readerの契約をより厳密に遵守させることで、HTTPクライアントがTLS接続の終了をより迅速かつ正確に検知できるようにした点にあります。

変更前は、tls.Conn.Readは、たとえ内部バッファにAlertCloseNotifyレコードが既に存在し、それが接続の論理的な終端を示していたとしても、アプリケーションデータが読み取れる限り(n, nil)を返していました。これにより、HTTPトランスポート層は、接続が閉じられたことを示すio.EOFをすぐに受け取ることができず、その接続を再利用可能であると誤って判断してしまう可能性がありました。

このコミットでは、Conn.Readメソッドに以下のロジックが追加されました。

  1. 条件チェック: Conn.Readがデータを読み取った後（n != 0）、エラーがなく（err == nil）、かつアプリケーションデータがこれ以上ない状態（c.input == nil）であるかを確認します。
  2. バッファ内のAlertCloseNotifyの確認: 上記の条件が満たされ、かつTLS層の生入力バッファ（c.rawInput）にデータが残っており、その最初のバイトがrecordTypeAlert（アラートレコード）を示している場合、AlertCloseNotifyがバッファに存在している可能性が高いと判断します。
  3. AlertCloseNotifyの読み取りとEOFの生成: この状況で、c.readRecord(recordTypeApplicationData)を呼び出すことで、バッファ内のAlertCloseNotifyレコードを処理しようとします。AlertCloseNotifyが読み取られると、readRecordはio.EOFを返します。このio.EOFが、Conn.Readの最終的な戻り値のerrとして設定されます。

これにより、Conn.Readは、アプリケーションデータをすべて読み取った直後に、もしAlertCloseNotifyがバッファに控えているならば、(n, io.EOF)という形式で戻り値を返します。これはio.Readerの契約に合致しており、HTTPクライアントはnバイトのデータを処理した後、すぐにio.EOFを検知して、その接続が閉じられたことを認識し、再利用を避けることができます。

コミットメッセージにもあるように、この変更はHTTPクライアント側の依存する変更と組み合わされることで、Amazon S3 APIとの通信におけるエラーレートを大幅に削減しました。HTTPクライアントは、io.EOFを受け取った接続を再利用しないように判断するようになります。

コアとなるコードの変更箇所

src/pkg/crypto/tls/conn.go

--- a/src/pkg/crypto/tls/conn.go
+++ b/src/pkg/crypto/tls/conn.go
@@ -451,6 +451,8 @@ func (b *block) readFromUntil(r io.Reader, n int) error {
 		m, err := r.Read(b.data[len(b.data):cap(b.data)])
 		b.data = b.data[0 : len(b.data)+m]
 		if len(b.data) >= n {
+			// TODO(bradfitz,agl): slightly suspicious
+			// that we're throwing away r.Read's err here.
 			break
 		}
 		if err != nil {
@@ -906,6 +908,25 @@ func (c *Conn) Read(b []byte) (n int, err error) {
 			c.input = nil
 		}
 
+		// If a close-notify alert is waiting, read it so that
+		// we can return (n, EOF) instead of (n, nil), to signal
+		// to the HTTP response reading goroutine that the
+		// connection is now closed. This eliminates a race
+		// where the HTTP response reading goroutine would
+		// otherwise not observe the EOF until its next read,
+		// by which time a client goroutine might have already
+		// tried to reuse the HTTP connection for a new
+		// request.
+		// See https://codereview.appspot.com/76400046
+		// and http://golang.org/issue/3514
+		if ri := c.rawInput; ri != nil &&
+			n != 0 && err == nil &&
+			c.input == nil && len(ri.data) > 0 && recordType(ri.data[0]) == recordTypeAlert {
+			if recErr := c.readRecord(recordTypeApplicationData); recErr != nil {
+				err = recErr // will be io.EOF on closeNotify
+			}
+		}
+
 		if n != 0 || err != nil {
 			return n, err
 		}

src/pkg/crypto/tls/tls_test.go

--- a/src/pkg/crypto/tls/tls_test.go
+++ b/src/pkg/crypto/tls/tls_test.go
@@ -5,6 +5,7 @@
 package tls
 
 import (
+	"io"
 	"net"
 	"strings"
 	"testing"
@@ -109,18 +110,22 @@ func TestX509MixedKeyPair(t *testing.T) {
 	}\n}\n \n-func TestDialTimeout(t *testing.T) {\n-\tif testing.Short() {\n-\t\tt.Skip(\"skipping in short mode\")\n-\t}\n-\n-\tlistener, err := net.Listen(\"tcp\", \"127.0.0.1:0\")\n+func newLocalListener(t *testing.T) net.Listener {\n+\tln, err := net.Listen(\"tcp\", \"127.0.0.1:0\")\n \tif err != nil {\n-\t\tlistener, err = net.Listen(\"tcp6\", \"[::1]:0\")\n+\t\tln, err = net.Listen(\"tcp6\", \"[::1]:0\")\n \t}\n \tif err != nil {\n \t\tt.Fatal(err)\n \t}\n+\treturn ln\n+}\n+\n+func TestDialTimeout(t *testing.T) {\n+\tif testing.Short() {\n+\t\tt.Skip(\"skipping in short mode\")\n+\t}\n+\tlistener := newLocalListener(t)\n \n \taddr := listener.Addr().String()\n \tdefer listener.Close()\n@@ -142,6 +147,7 @@ func TestDialTimeout(t *testing.T) {\n \t\tTimeout: 10 * time.Millisecond,\n \t}\n \n+\tvar err error\n \tif _, err = DialWithDialer(dialer, \"tcp\", addr, nil); err == nil {\n \t\tt.Fatal(\"DialWithTimeout completed successfully\")\n \t}\n@@ -150,3 +156,59 @@ func TestDialTimeout(t *testing.T) {\n \t\tt.Errorf(\"resulting error not a timeout: %s\", err)\n \t}\n }\n+\n+// tests that Conn.Read returns (non-zero, io.EOF) instead of\n+// (non-zero, nil) when a Close (alertCloseNotify) is sitting right\n+// behind the application data in the buffer.\n+func TestConnReadNonzeroAndEOF(t *testing.T) {\n+\tln := newLocalListener(t)\n+\tdefer ln.Close()\n+\n+\tsrvCh := make(chan *Conn, 1)\n+\tgo func() {\n+\t\tsconn, err := ln.Accept()\n+\t\tif err != nil {\n+\t\t\tt.Error(err)\n+\t\t\tsrvCh <- nil\n+\t\t\treturn\n+\t\t}\n+\t\tserverConfig := *testConfig\n+\t\tsrv := Server(sconn, &serverConfig)\n+\t\tif err := srv.Handshake(); err != nil {\n+\t\t\tt.Error(\"handshake: %v\", err)\n+\t\t\tsrvCh <- nil\n+\t\t\treturn\n+\t\t}\n+\t\tsrvCh <- srv\n+\t}()\n+\n+\tclientConfig := *testConfig\n+\tconn, err := Dial(\"tcp\", ln.Addr().String(), &clientConfig)\n+\tif err != nil {\n+\t\tt.Fatal(err)\n+\t}\n+\tdefer conn.Close()\n+\n+\tsrv := <-srvCh\n+\tif srv == nil {\n+\t\treturn\n+\t}\n+\n+\tbuf := make([]byte, 6)\n+\n+\tsrv.Write([]byte(\"foobar\"))\n+\tn, err := conn.Read(buf)\n+\tif n != 6 || err != nil || string(buf) != \"foobar\" {\n+\t\tt.Fatalf(\"Read = %d, %v, data %q; want 6, nil, foobar\", n, err, buf)\n+\t}\n+\n+\tsrv.Write([]byte(\"abcdef\"))\n+\tsrv.Close()\n+\tn, err = conn.Read(buf)\n+\tif n != 6 || string(buf) != \"abcdef\" {\n+\t\tt.Fatalf(\"Read = %d, buf= %q; want 6, abcdef\", n, buf)\n+\t}\n+\tif err != io.EOF {\n+\t\tt.Errorf(\"Second Read error = %v; want io.EOF\", err)\n+\t}\n+}\n```

## コアとなるコードの解説

### `src/pkg/crypto/tls/conn.go`の変更

追加されたコードブロックは、`Conn.Read`メソッドの終盤に位置し、アプリケーションデータの読み取りが完了した後に実行されます。

```go
		// If a close-notify alert is waiting, read it so that
		// we can return (n, EOF) instead of (n, nil), to signal
		// to the HTTP response reading goroutine that the
		// connection is now closed. This eliminates a race
		// where the HTTP response reading goroutine would
		// otherwise not observe the EOF until its next read,
		// by which time a client goroutine might have already
		// tried to reuse the HTTP connection for a new
		// request.
		// See https://codereview.appspot.com/76400046
		// and http://golang.org/issue/3514
		if ri := c.rawInput; ri != nil &&
			n != 0 && err == nil &&
			c.input == nil && len(ri.data) > 0 && recordType(ri.data[0]) == recordTypeAlert {
			if recErr := c.readRecord(recordTypeApplicationData); recErr != nil {
				err = recErr // will be io.EOF on closeNotify
			}
		}

このコードブロックの条件式は、以下のすべてが真である場合に実行されます。

  • ri := c.rawInput; ri != nil: 生の入力バッファが存在すること。
  • n != 0: 直前の読み取りで何らかのデータが読み取られたこと。
  • err == nil: 直前の読み取りでエラーが発生しなかったこと。
  • c.input == nil: アプリケーションデータがこれ以上ないこと（つまり、アプリケーションデータバッファが空になったこと）。
  • len(ri.data) > 0: 生入力バッファにデータが残っていること。
  • recordType(ri.data[0]) == recordTypeAlert: 生入力バッファの最初のバイトがTLSアラートレコードのタイプを示していること。

これらの条件が満たされるということは、アプリケーションデータはすべて読み取られたが、その直後にAlertCloseNotifyのようなTLSアラートレコードが内部バッファに残っている状態であることを意味します。

この場合、c.readRecord(recordTypeApplicationData)が呼び出されます。通常、この関数はアプリケーションデータを読み取るために使用されますが、ここではバッファに残っているアラートレコードを処理するために間接的に利用されます。AlertCloseNotifyが読み取られると、readRecordはio.EOFを返します。このio.EOFがerr変数に代入され、結果としてConn.Readは(n, io.EOF)を返すことになります。

これにより、HTTPクライアントは、アプリケーションデータをすべて受け取った直後に、接続が正常に終了したことを示すio.EOFを検知できるようになり、接続の誤った再利用を防ぐことができます。

src/pkg/crypto/tls/tls_test.goの変更

新しいテストケースTestConnReadNonzeroAndEOFが追加されました。

// tests that Conn.Read returns (non-zero, io.EOF) instead of
// (non-zero, nil) when a Close (alertCloseNotify) is sitting right
// behind the application data in the buffer.
func TestConnReadNonzeroAndEOF(t *testing.T) {
    // ... (リスナー、サーバー、クライアントのセットアップ) ...

    // 最初のデータ "foobar" を送信し、クライアントが正常に読み取ることを確認
    srv.Write([]byte("foobar"))
    n, err := conn.Read(buf)
    if n != 6 || err != nil || string(buf) != "foobar" {
        t.Fatalf("Read = %d, %v, data %q; want 6, nil, foobar", n, err, buf)
    }

    // 次のデータ "abcdef" を送信し、すぐにサーバーをクローズ（AlertCloseNotifyを送信）
    srv.Write([]byte("abcdef"))
    srv.Close() // これによりAlertCloseNotifyが送信される

    // クライアントが残りのデータと同時にio.EOFを受け取ることを確認
    n, err = conn.Read(buf)
    if n != 6 || string(buf) != "abcdef" {
        t.Fatalf("Read = %d, buf= %q; want 6, abcdef", n, buf)
    }
    if err != io.EOF {
        t.Errorf("Second Read error = %v; want io.EOF", err)
    }
}

このテストは、まさにこのコミットが解決しようとしているシナリオを再現しています。

  1. サーバーが最初のデータ（"foobar"）を送信し、クライアントがこれを正常に読み取ります。この時点ではエラーは発生しません。
  2. 次に、サーバーが2番目のデータ（"abcdef"）を送信し、直後にsrv.Close()を呼び出して接続を閉じます。このClose()呼び出しにより、TLS層はAlertCloseNotifyレコードを送信します。
  3. クライアントが2回目のconn.Read(buf)を呼び出すと、内部バッファには"abcdef"とそれに続くAlertCloseNotifyが存在します。
  4. このコミットの変更により、Conn.Readは"abcdef"（n=6）を読み取ると同時に、バッファ内のAlertCloseNotifyを検知し、io.EOFを返します。
  5. テストは、nが6であり、かつerrがio.EOFであることを検証し、新しい挙動が正しく実装されていることを確認します。

このテストの追加は、変更が意図した通りに機能し、特定の条件下でio.EOFが適切に伝播されることを保証する上で非常に重要です。

関連リンク

  • Go Code Review: https://golang.org/cl/76400046
  • 関連するGo Issue (コミットメッセージに記載): http://golang.org/issue/3514 (ただし、このIssueは古いものであり、現在のGoリポジトリのIssue番号とは異なる可能性があります。コミットメッセージ自体が問題の詳細を説明しています。)

参考にした情報源リンク