mirror of
https://github.com/pgsty/minio.git
synced 2026-09-07 19:16:09 +03:00
fix: repair an undecodable SSE-C replica on retransmit
PutObjectHandler's precondition callback ran DecryptObjectInfo on the stored object before checkPreconditionsPUT, so an authenticated raw SSE-C replica overwrite was rejected when the stored version could not decrypt. A replica a pre-fix destination (issue #109) left as compress(ciphertext) or a re-encrypted body has an invalid decrypted length, so DecryptObjectInfo returned errObjectTampered and the retransmission that repairs it never ran -- the version stayed damaged through resync. #134's raw-replica exemption only covered the version/ETag duplicate check inside checkPreconditionsPUT, one step too late. Skip the stored object's decryption precondition only for a PURE raw SSE-C replica overwrite (a trusted SSE-C replica write with no public precondition), keyed on the incoming request's restored SSE-C metadata, the same predicate checkPreconditionsPUT uses. Such a write fully replaces the object, so requiring the damaged stored version to decrypt is both wrong and unnecessary. A conditional request keeps the check: DecryptObjectInfo also normalizes the stored sealed ETag to the client-visible one, and If-Match/If-None-Match must compare against that, not the sealed ETag -- skipping it for every replica inverted both conditions. Ordinary writes and non-SSE-C replicas are unchanged. Adds red/green regressions: a raw retransmit over a version staged as an undecodable body returns 500 XMinioObjectTampered before this change and 200 with full customer-key recovery after; and a conditional replica PUT (If-Match / If-None-Match) on the client-visible ETag is honoured rather than inverted. Fixes the single-PUT compression-damage recovery gap in Signed-off-by: Feng Ruohang <rh@vonng.com> #120.
This commit is contained in:
+15
-3
@@ -2227,9 +2227,21 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
|
||||
r.Header.Get(xhttp.IfMatch) != "" ||
|
||||
r.Header.Get(xhttp.IfNoneMatch) != "" {
|
||||
opts.CheckPrecondFn = func(oi ObjectInfo) bool {
|
||||
if _, err := DecryptObjectInfo(&oi, r); err != nil {
|
||||
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
|
||||
return true
|
||||
// A pure raw SSE-C replica overwrite (no public precondition) fully
|
||||
// replaces the stored object, so the destination must not first
|
||||
// require the stored object to decrypt: a replica an older destination
|
||||
// bug left as compress(ciphertext) or double-encrypted has an invalid
|
||||
// decrypted length, and requiring it here blocks the retransmission
|
||||
// that repairs it. The predicate is the incoming request's restored
|
||||
// SSE-C metadata, the same one checkPreconditionsPUT uses to exempt the
|
||||
// version/ETag duplicate. A conditional request (If-Match/If-None-Match)
|
||||
// still needs the decrypted, client-visible ETag, so it keeps the check.
|
||||
ssecReplica := isReplicaTrusted(ctx) && crypto.SSEC.IsEncrypted(opts.UserDefined)
|
||||
if !ssecReplica || r.Header.Get(xhttp.IfMatch) != "" || r.Header.Get(xhttp.IfNoneMatch) != "" {
|
||||
if _, err := DecryptObjectInfo(&oi, r); err != nil {
|
||||
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
|
||||
return true
|
||||
}
|
||||
}
|
||||
return checkPreconditionsPUT(ctx, w, r, oi, opts)
|
||||
}
|
||||
|
||||
@@ -551,6 +551,147 @@ func testAPISSECReplicaRetransmitOverExistingVersion(obj ObjectLayer, instanceTy
|
||||
assertRetransmittedVersion(t, obj, apiRouter, credentials, bucketName, object, srcInfo.VersionID, "retransmit=multipart", data, sseHeaders)
|
||||
})
|
||||
|
||||
t.Run("repairs-an-undecodable-existing-version", func(t *testing.T) {
|
||||
// A pre-fix destination (issue #109) could persist an SSE-C replica as
|
||||
// compress(ciphertext) or a re-encrypted body, leaving a stored length
|
||||
// that is not a valid encryption stream. Resync repairs such a version by
|
||||
// retransmitting the source's raw ciphertext, but the write must not first
|
||||
// require the stored, damaged object to decrypt. See issue #120.
|
||||
data := bytes.Repeat([]byte("SILO raw SSE-C recovery\n"), 400)
|
||||
object := "ssec-duplicate/undecodable"
|
||||
putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, sseHeaders)
|
||||
|
||||
gr, err := obj.GetObjectNInfo(t.Context(), bucketName, object, nil, http.Header{}, ObjectOptions{
|
||||
ReplicationRequest: true,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
srcInfo := gr.ObjInfo
|
||||
cipher, err := io.ReadAll(gr)
|
||||
gr.Close()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Stage the damage: overwrite the version with a body too short to be a
|
||||
// valid encryption stream, standing in for the compression/re-encryption
|
||||
// an old destination left behind. The staging write itself is a raw
|
||||
// replica over the still-valid version, so it stores verbatim.
|
||||
damaged := []byte("dmg!!")
|
||||
if _, derr := sio.DecryptedSize(uint64(len(damaged))); derr == nil {
|
||||
t.Fatalf("%s: fixture body of %d bytes is a valid stream length, not undecodable", instanceType, len(damaged))
|
||||
}
|
||||
stageHdrs := replicaHeaders(t, srcInfo)
|
||||
stageURL := getPutObjectURL("", bucketName, object) + "?versionId=" + srcInfo.VersionID
|
||||
stageReq, err := newTestSignedRequestV4(http.MethodPut, stageURL, int64(len(damaged)),
|
||||
bytes.NewReader(damaged), replicator.AccessKey, replicator.SecretKey, stageHdrs)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
stageRec := httptest.NewRecorder()
|
||||
apiRouter.ServeHTTP(stageRec, stageReq)
|
||||
if stageRec.Code != http.StatusOK {
|
||||
t.Fatalf("%s: could not stage the damaged replica: %d %s", instanceType, stageRec.Code, stageRec.Body.String())
|
||||
}
|
||||
// The staged version is genuinely undecodable at the object layer, which
|
||||
// is exactly what makes DecryptObjectInfo fail during the overwrite.
|
||||
staged, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{VersionID: srcInfo.VersionID})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, derr := staged.DecryptedSize(); derr == nil {
|
||||
t.Fatalf("%s: staged replica is decodable, cannot exercise the repair path", instanceType)
|
||||
}
|
||||
|
||||
// Retransmit the correct ciphertext over the same version. Before the
|
||||
// raw-replica precondition exemption this failed with XMinioObjectTampered
|
||||
// because the damaged object could not decrypt; it must now repair.
|
||||
srcInfo.UserTags = "retransmit=undecodable"
|
||||
hdrs := replicaHeaders(t, srcInfo)
|
||||
putURL := getPutObjectURL("", bucketName, object) + "?versionId=" + srcInfo.VersionID
|
||||
req, err := newTestSignedRequestV4(http.MethodPut, putURL, int64(len(cipher)),
|
||||
bytes.NewReader(cipher), replicator.AccessKey, replicator.SecretKey, hdrs)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
apiRouter.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("%s: retransmit over an undecodable version status %d, want 200: %s", instanceType, rec.Code, rec.Body.String())
|
||||
}
|
||||
assertRetransmittedVersion(t, obj, apiRouter, credentials, bucketName, object, srcInfo.VersionID, "retransmit=undecodable", data, sseHeaders)
|
||||
})
|
||||
|
||||
// setupHealthySSECVersion writes a normal SSE-C object and returns its
|
||||
// ObjectInfo (for building replica headers), its ciphertext, and the
|
||||
// client-visible ETag a keyed reader sees -- the decrypted ETag, which is
|
||||
// distinct from the stored sealed ETag.
|
||||
setupHealthySSECVersion := func(t *testing.T, object string, data []byte) (srcInfo ObjectInfo, cipher []byte, clientETag string) {
|
||||
t.Helper()
|
||||
putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, sseHeaders)
|
||||
gr, err := obj.GetObjectNInfo(t.Context(), bucketName, object, nil, http.Header{}, ObjectOptions{ReplicationRequest: true})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
srcInfo = gr.ObjInfo
|
||||
cipher, err = io.ReadAll(gr)
|
||||
gr.Close()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// The client-visible ETag is the decrypted one, which the handler derives
|
||||
// with the customer key through DecryptObjectInfo; compute it the same way.
|
||||
keyHeader := http.Header{}
|
||||
for k, v := range sseHeaders {
|
||||
keyHeader.Set(k, v)
|
||||
}
|
||||
clientETag = getDecryptedETag(keyHeader, srcInfo, false)
|
||||
if clientETag == "" || clientETag == srcInfo.ETag {
|
||||
t.Fatalf("%s: client ETag %q is not distinct from the sealed ETag %q", instanceType, clientETag, srcInfo.ETag)
|
||||
}
|
||||
return srcInfo, cipher, clientETag
|
||||
}
|
||||
|
||||
// A conditional replica PUT must compare the public precondition against the
|
||||
// client-visible ETag, not the stored sealed one. Skipping DecryptObjectInfo
|
||||
// for every raw SSE-C replica (not only a pure overwrite) left oi.ETag sealed
|
||||
// and inverted both conditions.
|
||||
t.Run("if-match-on-the-client-etag-proceeds", func(t *testing.T) {
|
||||
object := "ssec-duplicate/cond-if-match"
|
||||
srcInfo, cipher, clientETag := setupHealthySSECVersion(t, object, bytes.Repeat([]byte("cond-if-match-"), 64))
|
||||
hdrs := replicaHeaders(t, srcInfo)
|
||||
hdrs[xhttp.IfMatch] = "\"" + clientETag + "\""
|
||||
req, err := newTestSignedRequestV4(http.MethodPut,
|
||||
getPutObjectURL("", bucketName, object)+"?versionId="+srcInfo.VersionID,
|
||||
int64(len(cipher)), bytes.NewReader(cipher), replicator.AccessKey, replicator.SecretKey, hdrs)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
apiRouter.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("%s: If-Match on the client ETag status %d, want 200: %s", instanceType, rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("if-none-match-on-the-client-etag-fails", func(t *testing.T) {
|
||||
object := "ssec-duplicate/cond-if-none-match"
|
||||
srcInfo, cipher, clientETag := setupHealthySSECVersion(t, object, bytes.Repeat([]byte("cond-if-none-"), 64))
|
||||
hdrs := replicaHeaders(t, srcInfo)
|
||||
hdrs[xhttp.IfNoneMatch] = "\"" + clientETag + "\""
|
||||
req, err := newTestSignedRequestV4(http.MethodPut,
|
||||
getPutObjectURL("", bucketName, object)+"?versionId="+srcInfo.VersionID,
|
||||
int64(len(cipher)), bytes.NewReader(cipher), replicator.AccessKey, replicator.SecretKey, hdrs)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
apiRouter.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusPreconditionFailed {
|
||||
t.Fatalf("%s: If-None-Match on the client ETag status %d, want 412: %s", instanceType, rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// assertRetransmittedVersion checks that a retransmit landed on the addressed
|
||||
|
||||
Reference in New Issue
Block a user