From: Matthew Heler Date: Fri, 5 Jun 2026 15:48:32 +0000 (-0500) Subject: rgw: fix AES-256-GCM key/IV reuse on multipart part re-upload X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=57e0250f5738dcea3adf4193d82f00b90c52c136;p=ceph.git rgw: fix AES-256-GCM key/IV reuse on multipart part re-upload Re-uploading the same part number in a GCM multipart upload encrypted the new data under the same key and IV as the first upload, since the IV is part_number||chunk_index and the part key came from the part number alone. GCM requires a unique IV per key; reusing one to encrypt different data weakens its confidentiality and integrity guarantees. Generate a random 16-byte salt on each UploadPart and fold it into the part key, HMAC(ObjectKey, BE32(part) || salt), so every upload gets a fresh key. The salt rides RGWUploadPartInfo, and complete stores the selected part's salt in RGW_ATTR_CRYPT_PART_NUMS, which now holds (part, salt) pairs. GET reads it back to re-derive the key, and an empty salt reproduces the old derivation so unsalted parts still decrypt. Signed-off-by: Matthew N. Heler --- diff --git a/src/rgw/driver/rados/rgw_putobj_processor.cc b/src/rgw/driver/rados/rgw_putobj_processor.cc index ee194eb7533..3292e1c4e97 100644 --- a/src/rgw/driver/rados/rgw_putobj_processor.cc +++ b/src/rgw/driver/rados/rgw_putobj_processor.cc @@ -563,6 +563,13 @@ int MultipartObjectProcessor::complete( obj_op.meta.if_match = if_match; obj_op.meta.if_nomatch = if_nomatch; + // Move the transient GCM salt onto the part info and drop it from attrs (never on the head). + std::string part_salt; + if (auto i = attrs.find(RGW_ATTR_CRYPT_PART_SALT); i != attrs.end()) { + part_salt = i->second.to_str(); + attrs.erase(i); + } + r = obj_op.write_meta(actual_size, accounted_size, attrs, rctx, writer.get_trace(), flags & rgw::sal::FLAG_LOG_OP); if (r < 0) @@ -586,6 +593,7 @@ int MultipartObjectProcessor::complete( info.accounted_size = accounted_size; info.modified = real_clock::now(); info.manifest = manifest; + info.crypt_salt = std::move(part_salt); bool compressed; r = rgw_compression_info_from_attrset(attrs, compressed, info.cs_info); diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc index 4e78e18c2b7..5907e907534 100644 --- a/src/rgw/driver/rados/rgw_rados.cc +++ b/src/rgw/driver/rados/rgw_rados.cc @@ -7969,16 +7969,14 @@ int RGWRados::Object::Read::prepare(optional_yield y, const DoutPrefixProvider * } for (auto& iter : src_attrset) { - /** - * Skip object-level encryption attributes when reading individual parts. - * These attrs describe the complete multipart object, not this part: - * - ORIGINAL_SIZE: would cause Content-Length mismatch - * - PARTS: contains sizes of all parts, not applicable to single part - * - PART_NUMS: maps part indices to S3 part numbers for the full object + /* + * Skip object-level crypt attrs (ORIGINAL_SIZE, PARTS) that describe the + * whole object and would break a single-part read. PART_NUMS passes + * through; it carries the per-part (num, salt) needed to derive this + * part's key. */ if (iter.first == RGW_ATTR_CRYPT_ORIGINAL_SIZE || - iter.first == RGW_ATTR_CRYPT_PARTS || - iter.first == RGW_ATTR_CRYPT_PART_NUMS) { + iter.first == RGW_ATTR_CRYPT_PARTS) { ldpp_dout(dpp, 4) << "skip crypt attr for part read: " << iter.first << dendl; continue; } diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 3825d25aced..a4a3987d6c2 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -4290,6 +4290,8 @@ int RadosMultipartUpload::complete(const DoutPrefixProvider *dpp, std::string crypt_mode = mode_iter->second.to_str(); is_aead = is_aead_mode(crypt_mode); } + // AEAD: (S3 part number, GCM salt) per selected part, in manifest-segment order. + std::vector> part_keys; do { ret = list_parts(dpp, cct, max_parts, marker, &marker, &truncated, y); @@ -4409,6 +4411,7 @@ int RadosMultipartUpload::complete(const DoutPrefixProvider *dpp, // Track plaintext size for AEAD encryption if (is_aead) { + part_keys.push_back({obj_part.num, obj_part.crypt_salt}); if (part_compressed) { // For compressed parts, use the uncompressed size directly plaintext_ofs += obj_part.accounted_size; @@ -4460,17 +4463,13 @@ int RadosMultipartUpload::complete(const DoutPrefixProvider *dpp, bl.append(std::to_string(plaintext_ofs)); attrs[RGW_ATTR_CRYPT_ORIGINAL_SIZE] = std::move(bl); - // Store actual S3 part numbers for correct IV/key derivation during decrypt - std::vector part_nums; - part_nums.reserve(part_etags.size()); - for (const auto& part : part_etags) { - part_nums.push_back(static_cast(part.first)); - } - bufferlist part_nums_bl; + // Store (S3 part number, GCM salt) pairs (collected in selection order + // above) for correct key derivation during decrypt. + bufferlist part_keys_bl; using ceph::encode; - encode(part_nums, part_nums_bl); - attrs[RGW_ATTR_CRYPT_PART_NUMS] = std::move(part_nums_bl); - ldpp_dout(dpp, 20) << "Stored CRYPT_PART_NUMS with " << part_nums.size() + encode(part_keys, part_keys_bl); + attrs[RGW_ATTR_CRYPT_PART_NUMS] = std::move(part_keys_bl); + ldpp_dout(dpp, 20) << "Stored CRYPT_PART_NUMS with " << part_keys.size() << " parts" << dendl; } diff --git a/src/rgw/driver/rados/rgw_sal_rados.h b/src/rgw/driver/rados/rgw_sal_rados.h index 22b0d1d7e5a..ff0eae39894 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -866,6 +866,7 @@ public: virtual const ACLOwner& get_owner() const override { return owner; } virtual ceph::real_time& get_mtime() override { return mtime; } virtual std::unique_ptr get_meta_obj() override; + virtual bool supports_crypt_part_salts() const override { return true; } virtual int init(const DoutPrefixProvider* dpp, optional_yield y, ACLOwner& owner, rgw_placement_rule& dest_placement, rgw::sal::Attrs& attrs) override; virtual int list_parts(const DoutPrefixProvider* dpp, CephContext* cct, int num_parts, int marker, diff --git a/src/rgw/rgw_basic_types.h b/src/rgw/rgw_basic_types.h index 9a50ff3aba0..99ea71db34d 100644 --- a/src/rgw/rgw_basic_types.h +++ b/src/rgw/rgw_basic_types.h @@ -278,6 +278,7 @@ struct RGWUploadPartInfo { RGWObjManifest manifest; RGWCompressionInfo cs_info; std::optional cksum; + std::string crypt_salt; // per-UploadPart GCM salt (non-secret HMAC input) // Previous part obj prefixes. Recorded here for later cleanup. std::set past_prefixes; @@ -285,7 +286,7 @@ struct RGWUploadPartInfo { RGWUploadPartInfo() : num(0), size(0) {} void encode(bufferlist& bl) const { - ENCODE_START(6, 2, bl); + ENCODE_START(7, 2, bl); encode(num, bl); encode(size, bl); encode(etag, bl); @@ -295,10 +296,11 @@ struct RGWUploadPartInfo { encode(accounted_size, bl); encode(past_prefixes, bl); encode(cksum, bl); + encode(crypt_salt, bl); ENCODE_FINISH(bl); } void decode(bufferlist::const_iterator& bl) { - DECODE_START_LEGACY_COMPAT_LEN(6, 2, 2, bl); + DECODE_START_LEGACY_COMPAT_LEN(7, 2, 2, bl); decode(num, bl); decode(size, bl); decode(etag, bl); @@ -317,6 +319,9 @@ struct RGWUploadPartInfo { if (struct_v >= 6) { decode(cksum, bl); } + if (struct_v >= 7) { + decode(crypt_salt, bl); + } DECODE_FINISH(bl); } void dump(Formatter *f) const; diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 14843124c66..bd7851b1786 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -192,6 +192,7 @@ using ceph::crypto::MD5; #define RGW_ATTR_CRYPT_DATAKEY RGW_ATTR_CRYPT_PREFIX "datakey" #define RGW_ATTR_CRYPT_PARTS RGW_ATTR_CRYPT_PREFIX "part-lengths" #define RGW_ATTR_CRYPT_PART_NUMS RGW_ATTR_CRYPT_PREFIX "part-numbers" +#define RGW_ATTR_CRYPT_PART_SALT RGW_ATTR_CRYPT_PREFIX "part-salt" #define RGW_ATTR_CRYPT_SALT RGW_ATTR_CRYPT_PREFIX "salt" #define RGW_ATTR_CRYPT_ORIGINAL_SIZE RGW_ATTR_CRYPT_PREFIX "original-size" #define RGW_ATTR_CRYPT_PREFETCH_ALIGN RGW_ATTR_CRYPT_PREFIX "prefetch-align" diff --git a/src/rgw/rgw_crypt.cc b/src/rgw/rgw_crypt.cc index e572779af3a..37eb0f02a6b 100644 --- a/src/rgw/rgw_crypt.cc +++ b/src/rgw/rgw_crypt.cc @@ -674,6 +674,7 @@ private: uint8_t salt[AES_256_GCM_SALT_SIZE]; bool salt_initialized = false; uint32_t part_number_ = 0; // For multipart: ensures unique IVs across parts + bool part_salt_applied_ = false; std::once_flag gcm_accel_init_once; CryptoAccelRef gcm_accel; @@ -731,27 +732,22 @@ public: return salt_initialized; } - /** - * Set part number for multipart IV derivation and key derivation (SSE-C). - * Must be called before encrypt/decrypt for multipart uploads. - * - * For SSE-C mode (has_base_key=true): also re-derives the part-specific key - * from base_key, enabling correct decryption when switching between parts - * during multipart GET operations. + /* + * For a multipart part, re-derive the part key from base_key with the salt. + * has_base_key holds for all GCM modes. */ - void set_part_number(uint32_t part_number) override { + void set_part_number(uint32_t part_number, + std::string_view part_salt = {}) override { this->part_number_ = part_number; + this->part_salt_applied_ = !part_salt.empty(); - // For SSE-C mode, also derive the correct part key if (has_base_key && part_number > 0) { - // Restore base key, then derive part key memcpy(this->key, this->base_key, AES_256_KEYSIZE); - derive_part_key(part_number); + derive_part_key(part_number, part_salt); } else if (has_base_key && part_number == 0) { // Part 0 means single-part or init - use base key directly memcpy(this->key, this->base_key, AES_256_KEYSIZE); } - // For non-SSE-C modes (has_base_key=false), only IV derivation uses part_number } /** @@ -845,16 +841,12 @@ public: return true; } - /** - * Derive part-specific key for multipart uploads. - * This prevents part reordering/swapping attacks. - * - * Formula: PartKey = HMAC-SHA256(ObjectKey, part_number) - * - * @param part_number Part number (1-based, as per S3 multipart API) - * @return true on success + /* + * PartKey = HMAC-SHA256(ObjectKey, BE32(part_number) || part_salt). + * Binds the key to the part number, and with a non-empty salt to the upload + * so re-uploading a part can't reuse (key, IV). */ - bool derive_part_key(uint32_t part_number) { + bool derive_part_key(uint32_t part_number, std::string_view part_salt = {}) { // Encode part number as big-endian 4 bytes uint8_t part_bytes[4]; part_bytes[0] = (part_number >> 24) & 0xFF; @@ -867,6 +859,9 @@ public: try { ceph::crypto::HMACSHA256 hmac(this->key, AES_256_KEYSIZE); hmac.Update(part_bytes, 4); + if (!part_salt.empty()) { + hmac.Update(reinterpret_cast(part_salt.data()), part_salt.size()); + } hmac.Final(derived); } catch (const ceph::crypto::DigestException& e) { ldpp_dout(dpp, 0) << "ERROR: derive_part_key: HMAC failed: " << e.what() << dendl; @@ -1067,6 +1062,18 @@ public: { output.clear(); + // write-path nonce-uniqueness guards: fresh per-part salt + chunk-aligned offset + if (part_number_ > 0 && !part_salt_applied_) { + ldpp_dout(dpp, 0) << "GCM: multipart part " << part_number_ + << " missing per-part salt; refusing to encrypt" << dendl; + return false; + } + if (stream_offset % static_cast(CHUNK_SIZE) != 0) { + ldpp_dout(dpp, 0) << "GCM: stream_offset " << stream_offset + << " not chunk-aligned (" << CHUNK_SIZE << ")" << dendl; + return false; + } + // Calculate output size: each CHUNK_SIZE plaintext becomes CHUNK_SIZE + GCM_TAG_SIZE size_t num_full_chunks = size / CHUNK_SIZE; size_t remainder = size % CHUNK_SIZE; @@ -1397,7 +1404,7 @@ RGWGetObj_BlockDecrypt::RGWGetObj_BlockDecrypt(const DoutPrefixProvider *dpp, RGWGetObj_Filter* next, std::unique_ptr crypt, std::vector parts_len, - std::vector part_nums, + std::vector> part_keys, off_t encrypted_total_size, bool has_compression, optional_yield y) @@ -1415,38 +1422,38 @@ RGWGetObj_BlockDecrypt::RGWGetObj_BlockDecrypt(const DoutPrefixProvider *dpp, cache(), y(y), parts_len(std::move(parts_len)), - part_nums(std::move(part_nums)), + part_keys(std::move(part_keys)), current_part_num(0) { block_size = this->crypt->get_block_size(); encrypted_block_size = this->crypt->get_encrypted_block_size(); /** - * Sanity check: when BOTH part_nums and parts_len are populated, they must + * Sanity check: when BOTH part_keys and parts_len are populated, they must * match in size. A mismatch indicates data corruption or a bug. * * When parts_len is empty (e.g., GET ?partNumber=N where CRYPT_PARTS is * intentionally skipped and the part object has no manifest), we can only - * trust a single fallback part number. + * trust a single fallback part. */ - if (!this->part_nums.empty() && !this->parts_len.empty() && - this->part_nums.size() != this->parts_len.size()) { - ldpp_dout(dpp, 0) << "ERROR: part_nums.size()=" << this->part_nums.size() + if (!this->part_keys.empty() && !this->parts_len.empty() && + this->part_keys.size() != this->parts_len.size()) { + ldpp_dout(dpp, 0) << "ERROR: part_keys.size()=" << this->part_keys.size() << " != parts_len.size()=" << this->parts_len.size() << " - possible data corruption" << dendl; - this->part_nums.clear(); + this->part_keys.clear(); } - if (this->parts_len.empty() && this->part_nums.size() > 1) { - ldpp_dout(dpp, 0) << "ERROR: part_nums.size()=" << this->part_nums.size() + if (this->parts_len.empty() && this->part_keys.size() > 1) { + ldpp_dout(dpp, 0) << "ERROR: part_keys.size()=" << this->part_keys.size() << " but parts_len is empty - cannot map part boundaries" << dendl; - this->part_nums.clear(); + this->part_keys.clear(); } // Initialize with first part's key if multipart - if (!this->part_nums.empty()) { - current_part_num = this->part_nums[0]; - this->crypt->set_part_number(current_part_num); + if (!this->part_keys.empty()) { + current_part_num = this->part_keys[0].first; + this->crypt->set_part_number(current_part_num, this->part_keys[0].second); } } @@ -1551,15 +1558,17 @@ int RGWGetObj_BlockDecrypt::process(bufferlist& in, size_t part_ofs, size_t size int RGWGetObj_BlockDecrypt::process_part_boundaries(size_t& plain_part_ofs_out) { size_t enc_part_ofs = enc_ofs; size_t plain_part_ofs = ofs; - const bool is_multipart = !part_nums.empty(); + const bool is_multipart = !part_keys.empty(); uint32_t part_idx = 0; int res = 0; for (size_t part : parts_len) { - // Get actual S3 part number from attribute (not calculated!) + // Get actual S3 part number + salt from the attribute (not calculated!) uint32_t this_part_num = 0; - if (is_multipart && part_idx < part_nums.size()) { - this_part_num = part_nums[part_idx]; + std::string_view this_salt; + if (is_multipart && part_idx < part_keys.size()) { + this_part_num = part_keys[part_idx].first; + this_salt = part_keys[part_idx].second; } if (enc_part_ofs >= part) { @@ -1571,7 +1580,7 @@ int RGWGetObj_BlockDecrypt::process_part_boundaries(size_t& plain_part_ofs_out) // Ensure cipher has correct part number if (is_multipart && current_part_num != this_part_num) { current_part_num = this_part_num; - crypt->set_part_number(current_part_num); + crypt->set_part_number(current_part_num, this_salt); } // Data crosses part boundary - process up to boundary @@ -1584,12 +1593,14 @@ int RGWGetObj_BlockDecrypt::process_part_boundaries(size_t& plain_part_ofs_out) // Move to next part part_idx++; uint32_t next_part_num = 0; - if (is_multipart && part_idx < part_nums.size()) { - next_part_num = part_nums[part_idx]; + std::string_view next_salt; + if (is_multipart && part_idx < part_keys.size()) { + next_part_num = part_keys[part_idx].first; + next_salt = part_keys[part_idx].second; } if (is_multipart && part_idx < parts_len.size() && current_part_num != next_part_num) { current_part_num = next_part_num; - crypt->set_part_number(current_part_num); + crypt->set_part_number(current_part_num, next_salt); } enc_part_ofs = 0; @@ -1598,7 +1609,7 @@ int RGWGetObj_BlockDecrypt::process_part_boundaries(size_t& plain_part_ofs_out) // Ensure cipher has correct part number if (is_multipart && current_part_num != this_part_num) { current_part_num = this_part_num; - crypt->set_part_number(current_part_num); + crypt->set_part_number(current_part_num, this_salt); } break; } diff --git a/src/rgw/rgw_crypt.h b/src/rgw/rgw_crypt.h index 386ad462ea0..7d54b0abf00 100644 --- a/src/rgw/rgw_crypt.h +++ b/src/rgw/rgw_crypt.h @@ -101,15 +101,18 @@ public: /** * Set the part number for multipart object decryption. - * AEAD modes use this for per-part IV derivation. + * AEAD modes use this for per-part IV derivation, and for GCM also fold the + * optional per-UploadPart salt into part-key derivation. * Default is no-op; CBC derives IVs from block offsets instead. */ - virtual void set_part_number(uint32_t part_number) {} + virtual void set_part_number(uint32_t part_number, + std::string_view part_salt = {}) {} }; static const size_t AES_256_KEYSIZE = 256 / 8; static const size_t AES_256_GCM_IV_SIZE = 96 / 8; // 12 bytes, GCM standard static const size_t AES_256_GCM_SALT_SIZE = 32; // 256-bit random salt for HMAC-based key derivation +static constexpr size_t AES_256_GCM_PART_SALT_SIZE = 16; // per-UploadPart entropy folded into part-key derivation /** * AEAD chunk size constants used for size calculations across RGW. @@ -266,7 +269,7 @@ class RGWGetObj_BlockDecrypt : public RGWGetObj_Filter { size_t encrypted_block_size; /**< snapshot of \ref BlockCrypt.get_encrypted_block_size() (includes auth tag for GCM) */ optional_yield y; std::vector parts_len; /**< size of parts of multipart object, parsed from manifest */ - std::vector part_nums; /**< actual S3 part numbers for multipart (e.g., [1,3,5]) */ + std::vector> part_keys; /**< per part: (S3 part number, GCM salt) */ uint32_t current_part_num = 0; /**< current part number (1-based, 0 means single-part object) */ int process(bufferlist& cipher, size_t part_ofs, size_t size); @@ -289,7 +292,7 @@ public: RGWGetObj_Filter* next, std::unique_ptr crypt, std::vector parts_len, - std::vector part_nums, + std::vector> part_keys, off_t encrypted_total_size, bool has_compression, optional_yield y); diff --git a/src/rgw/rgw_multi.cc b/src/rgw/rgw_multi.cc index 9abda447ea4..d7155dd6259 100644 --- a/src/rgw/rgw_multi.cc +++ b/src/rgw/rgw_multi.cc @@ -100,5 +100,6 @@ void RGWUploadPartInfo::dump(Formatter *f) const utime_t ut(modified); encode_json("modified", ut, f); encode_json("past_prefixes", past_prefixes, f); + encode_json("crypt_salt", crypt_salt, f); } diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 4a00c1112fe..bfcb0ea6df1 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -10370,27 +10370,43 @@ int get_decrypt_filter( // correctly decrypt across part boundaries std::vector parts_len; - // Read actual S3 part numbers from attribute (set by CompleteMultipartUpload) - std::vector part_nums; + // Read (S3 part number, GCM salt) pairs from the attribute (set by Complete). + std::vector> part_keys; if (auto it = attrs.find(RGW_ATTR_CRYPT_PART_NUMS); it != attrs.end()) { try { auto p = it->second.cbegin(); using ceph::decode; - decode(part_nums, p); + decode(part_keys, p); } catch (const buffer::error&) { ldpp_dout(s, 1) << "failed to decode RGW_ATTR_CRYPT_PART_NUMS" << dendl; - // Continue with empty part_nums - will fail for multipart, ok for single-part + // Continue with empty part_keys - will fail for multipart, ok for single-part } } - /** - * Fallback for GET ?partNumber=N (single part read). - * When reading an individual part, the CRYPT_PART_NUMS attribute is skipped - * (see rgw_rados.cc skip list), so we use the requested part_num to ensure - * correct key derivation and IV generation. + /* + * GET ?partNumber=N receives the full pair vector; reduce it to the requested + * part so the salt/key derivation is correct and the vector matches the + * single-part manifest. + */ + if (part_num > 0 && !part_keys.empty()) { + size_t idx = part_keys.size(); + for (size_t k = 0; k < part_keys.size(); k++) { + if (part_keys[k].first == static_cast(part_num)) { idx = k; break; } + } + if (idx == part_keys.size()) { + return -ERR_INVALID_PART; + } + auto sel = std::move(part_keys[idx]); + part_keys.clear(); + part_keys.push_back(std::move(sel)); + } + + /* + * No CRYPT_PART_NUMS attr but a part was requested: derive from part_num with + * an empty salt. */ - if (part_nums.empty() && part_num > 0) { - part_nums.push_back(part_num); + if (part_keys.empty() && part_num > 0) { + part_keys.push_back({static_cast(part_num), {}}); } // for replicated objects, the original part lengths are preserved in an xattr @@ -10442,7 +10458,7 @@ int get_decrypt_filter( const bool has_compression = attrs.count(RGW_ATTR_COMPRESSION); *filter = std::make_unique( s, s->cct, cb, std::move(block_crypt), - std::move(parts_len), std::move(part_nums), encrypted_total_size, + std::move(parts_len), std::move(part_keys), encrypted_total_size, has_compression, s->yield); return 0; } diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index e0d1fc47e79..7dc6320c447 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -2958,6 +2958,10 @@ int RGWPutObj_ObjStore_S3::get_params(optional_yield y) ldpp_dout(s, 10) << "bad part number: " << multipart_part_str << ": " << err << dendl; return -EINVAL; } + if (multipart_part_num < 1 || multipart_part_num > 10000) { + ldpp_dout(s, 10) << "part number out of range: " << multipart_part_num << dendl; + return -EINVAL; + } } else if (!multipart_upload_id.empty()) { ldpp_dout(s, 10) << "part number with no multipart upload id" << dendl; return -EINVAL; @@ -3117,8 +3121,27 @@ int RGWPutObj_ObjStore_S3::get_encrypt_filter( res = rgw_s3_prepare_decrypt(s, s->yield, obj->get_attrs(), &block_crypt, &crypt_http_responses, copy_source, multipart_part_num); - if (res == 0 && block_crypt != nullptr) + if (res == 0 && block_crypt != nullptr) { + /* + * AEAD UploadPart: fold fresh per-UploadPart entropy into the part key so + * re-uploading the same part can't reuse (key, IV). Refuse the upload if + * the backend can't persist per-part salts rather than silently falling + * back to a deterministic part key. + */ + if (is_aead_mode(get_str_attribute(obj->get_attrs(), RGW_ATTR_CRYPT_MODE))) { + if (!upload->supports_crypt_part_salts()) { + ldpp_dout(this, 0) << "ERROR: AEAD multipart upload requires a supported backend" << dendl; + return -ERR_NOT_IMPLEMENTED; + } + std::string part_salt(AES_256_GCM_PART_SALT_SIZE, '\0'); + s->cct->random()->get_bytes(part_salt.data(), part_salt.size()); + block_crypt->set_part_number(multipart_part_num, part_salt); + // string_view selects the raw-bytes set_attr overload, not the local + // length-prefixing one; the writer reads it back via to_str(). + set_attr(this->attrs, RGW_ATTR_CRYPT_PART_SALT, std::string_view(part_salt)); + } filter->reset(new RGWPutObj_BlockEncrypt(s, s->cct, cb, std::move(block_crypt), s->yield)); + } } /* it is ok, to not have encryption at all */ } diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 92e084ee34a..74d4b100a30 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -1543,6 +1543,9 @@ public: /** Get the Object that represents this upload */ virtual std::unique_ptr get_meta_obj() = 0; + /** True if this store persists per-part GCM salts; gates AEAD UploadPart salt emission. */ + virtual bool supports_crypt_part_salts() const { return false; } + /** Initialize this upload */ virtual int init(const DoutPrefixProvider* dpp, optional_yield y, ACLOwner& owner, rgw_placement_rule& dest_placement, rgw::sal::Attrs& attrs) = 0; /** List all the parts of this upload, filling the parts cache */ diff --git a/src/rgw/rgw_sal_filter.h b/src/rgw/rgw_sal_filter.h index c50b4cdeead..ecb55a98ee5 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -966,6 +966,8 @@ public: virtual std::unique_ptr get_meta_obj() override; + virtual bool supports_crypt_part_salts() const override { return next->supports_crypt_part_salts(); } + virtual int init(const DoutPrefixProvider* dpp, optional_yield y, ACLOwner& owner, rgw_placement_rule& dest_placement, rgw::sal::Attrs& attrs) override; virtual int list_parts(const DoutPrefixProvider* dpp, CephContext* cct, int num_parts, int marker, diff --git a/src/test/rgw/test_rgw_crypto.cc b/src/test/rgw/test_rgw_crypto.cc index f7cecb00b30..1b09963cc07 100644 --- a/src/test/rgw/test_rgw_crypto.cc +++ b/src/test/rgw/test_rgw_crypto.cc @@ -1202,16 +1202,19 @@ TEST(TestRGWCrypto, verify_AES_256_GCM_key_derivation) // Test 3: Different part numbers produce different derived keys { + const std::string part_salt(AES_256_GCM_PART_SALT_SIZE, 'p'); auto aes1(AES_256_GCM_create(&no_dpp, g_ceph_context, &user_key[0], 32)); std::string salt = AES_256_GCM_get_salt(aes1.get()); ASSERT_TRUE(AES_256_GCM_derive_object_key(aes1.get(), user_key, 32, "bucket", "object", 1)); + aes1->set_part_number(1, part_salt); auto aes2(AES_256_GCM_create(&no_dpp, g_ceph_context, &user_key[0], 32, reinterpret_cast(salt.c_str()), salt.size())); ASSERT_TRUE(AES_256_GCM_derive_object_key(aes2.get(), user_key, 32, "bucket", "object", 2)); + aes2->set_part_number(2, part_salt); bufferlist input; input.append(plaintext); @@ -1245,6 +1248,44 @@ TEST(TestRGWCrypto, verify_AES_256_GCM_key_derivation) } } +TEST(TestRGWCrypto, verify_AES_256_GCM_part_salt_key_isolation) +{ + NoDoutPrefix no_dpp(g_ceph_context, ceph_subsys_rgw); + uint8_t user_key[32]; + for (size_t i = 0; i < sizeof(user_key); i++) user_key[i] = i; + + const std::string osalt(AES_256_GCM_SALT_SIZE, 'o'); + const std::string a(AES_256_GCM_PART_SALT_SIZE, 'a'); + const std::string b(AES_256_GCM_PART_SALT_SIZE, 'b'); + const uint8_t* os = reinterpret_cast(osalt.data()); + bufferlist in; + in.append("same part number, different salt"); + + auto mk = [&](const std::string& s) { + auto e = AES_256_GCM_create(&no_dpp, g_ceph_context, &user_key[0], 32, os, osalt.size()); + EXPECT_NE(e.get(), nullptr); + EXPECT_TRUE(AES_256_GCM_derive_object_key(e.get(), user_key, 32, "bucket", "object", 1)); + e->set_part_number(1, s); + return e; + }; + + // Same object key, same part number, different salt -> different ciphertext. + // This is false on the pre-fix binary (same part number -> same key+IV). + bufferlist ca, cb; + ASSERT_TRUE(mk(a)->encrypt(in, 0, in.length(), ca, 0, null_yield)); + ASSERT_TRUE(mk(b)->encrypt(in, 0, in.length(), cb, 0, null_yield)); + ASSERT_NE(std::string_view(ca.c_str(), ca.length()), + std::string_view(cb.c_str(), cb.length())); + + // Correct salt round-trips; wrong salt fails the GCM tag. + bufferlist pa; + ASSERT_TRUE(mk(a)->decrypt(ca, 0, ca.length(), pa, 0, null_yield)); + ASSERT_EQ(std::string_view(in.c_str(), in.length()), + std::string_view(pa.c_str(), pa.length())); + bufferlist bad; + ASSERT_FALSE(mk(b)->decrypt(ca, 0, ca.length(), bad, 0, null_yield)); +} + TEST(TestRGWCrypto, verify_AES_256_GCM_chunk_reorder_detection) { // Verify that swapping chunk positions is detected via AAD