From 1ea2b1e77c0e621aa17f049b60eed802e4e4e936 Mon Sep 17 00:00:00 2001 From: Martin Raiber Date: Wed, 9 Jun 2021 12:27:15 +0200 Subject: [PATCH] Remove etag checking --- clouddrive/IKvStoreBackend.h | 23 +++++ clouddrive/KvStoreBackendS3.cpp | 157 ++++++++------------------------ clouddrive/KvStoreFrontend.cpp | 22 ++--- 3 files changed, 68 insertions(+), 134 deletions(-) diff --git a/clouddrive/IKvStoreBackend.h b/clouddrive/IKvStoreBackend.h index e8923adf..e6e199c2 100644 --- a/clouddrive/IKvStoreBackend.h +++ b/clouddrive/IKvStoreBackend.h @@ -7,6 +7,29 @@ class IOnlineKvStore; +namespace +{ + std::string get_md5sum(const std::string& md5sum) + { + if (md5sum.size() == 16) + return md5sum; + else if (md5sum.size() > 16) + return md5sum.substr(0, 16); + else + return std::string(); + } + + std::string get_locinfo(const std::string& md5sum) + { + if (md5sum.size() == 16) + return std::string(); + else if (md5sum.size() > 16) + return md5sum.substr(16); + else + return md5sum; + } +} + class IKvStoreBackend : public IObject { public: diff --git a/clouddrive/KvStoreBackendS3.cpp b/clouddrive/KvStoreBackendS3.cpp index 5f0aa04a..144d3e45 100644 --- a/clouddrive/KvStoreBackendS3.cpp +++ b/clouddrive/KvStoreBackendS3.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -35,6 +36,7 @@ #include "../stringtools.h" #include "../urbackupcommon/os_functions.h" #include "../urbackupcommon/events.h" +#include "../md5.h" #include #include #include @@ -348,9 +350,14 @@ bool KvStoreBackendS3::get( const std::string& key, const std::string& path, con idx0 = getShardIdx(key, buckets.size()-1)+1; } + std::string expected_md5sum = get_md5sum(md5sum); + std::string version = get_locinfo(md5sum); + Aws::S3::Model::GetObjectRequest getObjectRequest; getObjectRequest.SetBucket(buckets[idx0].name.c_str()); getObjectRequest.SetKey(key.c_str()); + if(!version.empty()) + getObjectRequest.SetVersionId(version.c_str()); IFsFile* tmpfile = Server->openTemporaryFile(); if(tmpfile==NULL) @@ -414,65 +421,6 @@ bool KvStoreBackendS3::get( const std::string& key, const std::string& path, con } ++n_requests; } - - std::string etag = trim(getObjectOutcome.GetResult().GetETag().c_str()); - - if(etag.size()>2) - { - etag = strlower(etag.substr(1, etag.size()-2)); - - if(etag=="00000000000000000000000000000000-1") //minio returns this sometimes - { - if (!FileExists("/var/urbackup/ignore_etag_error")) - { - if (allow_error_event) - { - writestring("/var/urbackup/ignore_etag_error", convert(Server->getTimeSeconds())); - addSystemEvent("s3_backend", - "Minio has missing object metadata", - "Minio returned the etag (checksum) " + etag + " for object " + key + " but the appliance expected etag " + bytesToHex(md5sum) - + ". Your minio data store might be corrupted. The etag will be ignored, but the actual data might be" - " corrupted as well which will lead to further errors and might make your backup storage inaccessible.", LL_WARNING); - } - Server->Log("Minio returned etag " + etag + " for object " + key + ". Expected etag " + bytesToHex(md5sum), LL_WARNING); - } - etag = bytesToHex(md5sum); - } - else if (!IsHex(etag)) - { - if (!FileExists("/var/urbackup/ignore_etag_error")) - { - if (allow_error_event) - { - writestring("/var/urbackup/ignore_etag_error", convert(Server->getTimeSeconds())); - addSystemEvent("s3_backend", - "Missing object metadata", - "S3 storage returned the etag (checksum) " + etag + " for object " + key + " but the appliance expected etag " + bytesToHex(md5sum) - + " (md5sum of object data). The appliance will ignore this etag (and other etags like it).", LL_WARNING); - } - Server->Log("S3 returned etag " + etag + " for object " + key + ". Expected etag " + bytesToHex(md5sum), LL_WARNING); - } - etag = bytesToHex(md5sum); - } - } - - if(!md5sum.empty() - && etag!=bytesToHex(md5sum)) - { - if (allow_error_event) - { - addSystemEvent("s3_backend", - "Object checksum wrong after download", - "Online md5sum (" + etag + " -- orig etag '" + getObjectOutcome.GetResult().GetETag().c_str() + "') is not equal to locally stored md5sum (" + bytesToHex(md5sum) + ") after retrieving "+key, LL_ERROR); - } - Server->Log("Online md5sum ("+etag+" -- orig etag '"+ getObjectOutcome.GetResult().GetETag().c_str()+"') is not equal to locally stored md5sum ("+bytesToHex(md5sum)+") after retrieving "+key, LL_ERROR); - return false; - } - else if (md5sum.empty() - && key!="cd_magic_file") - { - Server->Log("Locally stored md5sum of object "+key+" empty. Using online md5sum from now ("+etag+")", LL_WARNING); - } int mode = MODE_RW; #ifdef _WIN32 @@ -532,6 +480,7 @@ bool KvStoreBackendS3::get( const std::string& key, const std::string& path, con std::vector buffer; buffer.resize(32768); + MD5 md_check; tmpfile->Seek(0); while(true) { @@ -541,7 +490,7 @@ bool KvStoreBackendS3::get( const std::string& key, const std::string& path, con break; } - if(decrypt_and_decompress.get()!=NULL + if(decrypt_and_decompress.get()!=nullptr && !decrypt_and_decompress->put(buffer.data(), read)) { if(allow_error_event) @@ -553,8 +502,9 @@ bool KvStoreBackendS3::get( const std::string& key, const std::string& path, con Server->Log("Error decrypting and decompressing", LL_ERROR); return false; } - else if(decrypt_and_decompress.get()==NULL) + else if(decrypt_and_decompress.get()==nullptr) { + md_check.update(reinterpret_cast(buffer.data()), read); if(res->Write(buffer.data(), read)!=read) { std::string syserr = os_last_error_str(); @@ -584,47 +534,33 @@ bool KvStoreBackendS3::get( const std::string& key, const std::string& path, con return false; } - std::string final_md5sum_hex = decrypt_and_decompress->md5sum(); + ret_md5sum = hexToBytes(decrypt_and_decompress->md5sum()); + } + else + { + md_check.finalize(); + ret_md5sum.assign(reinterpret_cast(md_check.raw_digest_int()), 16); + } - if (!md5sum.empty() - && bytesToHex(md5sum) != final_md5sum_hex) + if (!expected_md5sum.empty() + && bytesToHex(expected_md5sum) != ret_md5sum) + { + Server->Log("Calculated md5sum of downloaded object differs from expected md5sum for object " + key + + ". Calculated=" + ret_md5sum + " Expected=" + bytesToHex(expected_md5sum), LL_ERROR); + if (allow_error_event) { - Server->Log("Calculated md5sum of downloaded object differs from expected md5sum for object "+key+". Calculated=" + final_md5sum_hex+ " Expected="+ bytesToHex(md5sum), LL_ERROR); - if (allow_error_event) - { - addSystemEvent("s3_backend", - "Calculated md5sum differs from expected", - "Calculated md5sum of downloaded object differs from expected md5sum for object " + key + ". Calculated=" + final_md5sum_hex + " Expected=" + bytesToHex(md5sum), LL_ERROR); - } - return false; - } - - if (!etag.empty() - && etag != final_md5sum_hex) - { - Server->Log("Calculated md5sum of downloaded object differs from etag for object " + key + ". Calculated=" + final_md5sum_hex + " Etag=" + etag, LL_ERROR); - if (allow_error_event) - { - addSystemEvent("s3_backend", - "Calculated md5sum differs from etag", - "Calculated md5sum of downloaded object differs from etag for object " + key + ". Calculated=" + final_md5sum_hex + " Etag=" + etag, LL_ERROR); - } - return false; + addSystemEvent("s3_backend", + "Calculated md5sum differs from expected", + "Calculated md5sum of downloaded object differs from expected md5sum for object " + key + + ". Calculated=" + ret_md5sum + " Expected=" + bytesToHex(expected_md5sum), LL_ERROR); } + return false; } if(ret_file==nullptr) { ret_file = res_src.release(); } - if (!md5sum.empty()) - { - ret_md5sum = md5sum; - } - else - { - ret_md5sum = hexToBytes(etag); - } return true; } else @@ -696,26 +632,26 @@ bool KvStoreBackendS3::list( IListCallback* callback ) } for (const Aws::S3::Model::Object& object: listObjectsOutcome.GetResult().GetContents()) - { + { std::string etag = trim(object.GetETag().c_str()); if(etag.size()>2) { etag = etag.substr(1, etag.size()-2); if(etag=="00000000000000000000000000000000-1") //minio returns this sometimes - { + { Server->Log("Server (minio) returned etag "+etag+" while listing object "+object.GetKey().c_str() + ". Ignoring etag.", LL_ERROR); etag.clear(); - } + } else if(!IsHex(etag)) - { + { Server->Log("Server returned invalid etag "+etag+" (not hex) while listing object "+object.GetKey().c_str() + ". Ignoring etag.", LL_ERROR); etag.clear(); - } + } } marker = object.GetKey(); - + if(!callback->onlineItem(object.GetKey().c_str(), hexToBytes(etag), object.GetSize(), object.GetLastModified().Millis())) { releaseS3Client(idx, s3_client); @@ -879,6 +815,8 @@ bool KvStoreBackendS3::put( const std::string& key, IFsFile* src, const std::str putObjectRequest.SetBucket(buckets[idx].name.c_str()); putObjectRequest.SetKey(key.c_str()); putObjectRequest.SetBody(upload_file); + putObjectRequest.SetContentMD5(base64_encode(reinterpret_cast(local_md5.data()), + static_cast(local_md5.size())).c_str()); if (storage_class != Aws::S3::Model::StorageClass::NOT_SET) { putObjectRequest.SetStorageClass(storage_class); @@ -908,27 +846,8 @@ bool KvStoreBackendS3::put( const std::string& key, IFsFile* src, const std::str } ++n_requests; } - - std::string etag = trim(putObjectOutcome.GetResult().GetETag().c_str()); - if(etag.size()>2) - { - etag = etag.substr(1, etag.size()-2); - } - - if(strlower(etag)!=bytesToHex(local_md5)) - { - Server->Log("Local md5sum is not equal to md5sum from s3 during upload (key "+key+"). Expected "+bytesToHex(local_md5)+" got "+strlower(etag), LL_WARNING); - if(allow_error_event) - { - addSystemEvent("s3_backend", - "Checksum error after upload", - "Local md5sum is not equal to md5sum from s3 during upload while uploading object "+key+". Expected "+bytesToHex(local_md5)+" got "+strlower(etag), LL_ERROR); - } - return false; - } - - md5sum = hexToBytes(etag); + md5sum = local_md5; compressed_size = local_size; if (del_with_location_info()) diff --git a/clouddrive/KvStoreFrontend.cpp b/clouddrive/KvStoreFrontend.cpp index 26949a1f..d119d6d9 100644 --- a/clouddrive/KvStoreFrontend.cpp +++ b/clouddrive/KvStoreFrontend.cpp @@ -680,7 +680,7 @@ IFsFile* KvStoreFrontend::get(int64 cd_id, const std::string& key, int64 transid bool backend_not_found = not_found; if (backend_mirror == nullptr || !backend_mirror->get(bkey, path, - cd_object.md5sum.size()>16 ? cd_object.md5sum.substr(0, 16) : cd_object.md5sum, + get_md5sum(cd_object.md5sum), flags, allow_error_event, ret, ret_md5sum, get_status)) { not_found = (get_status & IKvStoreBackend::GetStatusNotFound)>0; @@ -827,7 +827,7 @@ bool KvStoreFrontend::put(int64 cd_id, const std::string& key, int64 transid, dao.getObjectInTransid(transid, tkey) : dao.getObjectInTransidCd(cd_id, transid, tkey); if (obj.exists - && obj.md5sum.size()>16) + && !get_locinfo(obj.md5sum).empty()) { size_t idx1 = 0; size_t idx2 = 0; @@ -855,7 +855,7 @@ bool KvStoreFrontend::put(int64 cd_id, const std::string& key, int64 transid, } if (idx2>0) return false; - *locinfo = obj.md5sum.substr(16); + *locinfo = get_locinfo(obj.md5sum); ++idx2; return true; },false); @@ -2852,9 +2852,7 @@ bool KvStoreFrontend::BackgroundWorker::removeOldObjects(KvStoreDao& dao, object_collector.add( deletable_object.trans_id, deletable_object.tkey, - deletable_object.md5sum.size() > 16 ? - deletable_object.md5sum.substr(16) : - std::string(), + get_locinfo(deletable_object.md5sum), mirrored); has_object = true; @@ -2987,9 +2985,7 @@ bool KvStoreFrontend::BackgroundWorker::removeOldObjects(KvStoreDao& dao, obj.md5sum.clear(); #endif assert(backend->check_deleted(frontend->prefixKey(frontend->encodeKey(cd_id, obj.tkey, obj.trans_id)), - obj.md5sum.size() > 16 ? - obj.md5sum.substr(16) : - std::string())); + get_locinfo(obj.md5sum))); } #endif @@ -3139,9 +3135,7 @@ bool KvStoreFrontend::BackgroundWorker::removeTransaction(KvStoreDao& dao, int64 object_collector.add(-1, trans_object.tkey, - trans_object.md5sum.size() > 16 ? - trans_object.md5sum.substr(16) : - std::string(), + get_locinfo(trans_object.md5sum), mirrored); has_object = true; @@ -3294,9 +3288,7 @@ bool KvStoreFrontend::BackgroundWorker::removeTransaction(KvStoreDao& dao, int64 obj.md5sum.clear(); #endif assert(backend->check_deleted(frontend->prefixKey(frontend->encodeKey(cd_id, obj.tkey, trans_id)), - obj.md5sum.size() > 16 ? - obj.md5sum.substr(16) : - std::string())); + get_locinfo(obj.md5sum))); } for (std::string& key : add_backend_keys) {