rgw: fix mfa setting, verify on object deletion

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>

diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
index f319c5c113ab1fd854218f1a3043488455916dd2..7f1769fd0e60a7996399a73994a36b5bd25bb0dc 100644 (file)
--- a/src/rgw/rgw_common.cc
+++ b/src/rgw/rgw_common.cc
@@ -84,6 +84,7 @@ rgw_http_errors rgw_http_s3_errors({
     { ERR_USER_SUSPENDED, {403, "UserSuspended" }},
     { ERR_REQUEST_TIME_SKEWED, {403, "RequestTimeTooSkewed" }},
     { ERR_QUOTA_EXCEEDED, {403, "QuotaExceeded" }},
+    { ERR_MFA_REQUIRED, {403, "AccessDenied" }},
     { ENOENT, {404, "NoSuchKey" }},
     { ERR_NO_SUCH_BUCKET, {404, "NoSuchBucket" }},
     { ERR_NO_SUCH_WEBSITE_CONFIGURATION, {404, "NoSuchWebsiteConfiguration" }},

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 64789a1eb86a1e9abb3ffba33e805d0dcd637207..8cd71a276a51b284e3ea1cafc1789057ff65ce5a 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -200,6 +200,7 @@ using ceph::crypto::MD5;
 #define ERR_NO_SUCH_LC           2041
 #define ERR_NO_SUCH_USER         2042
 #define ERR_NO_SUCH_SUBUSER      2043
+#define ERR_MFA_REQUIRED         2044
 #define ERR_USER_SUSPENDED       2100
 #define ERR_INTERNAL_ERROR       2200
 #define ERR_NOT_IMPLEMENTED      2201
@@ -1352,8 +1353,8 @@ struct RGWBucketInfo
 
   bool versioned() const { return (flags & BUCKET_VERSIONED) != 0; }
   int versioning_status() { return flags & (BUCKET_VERSIONED | BUCKET_VERSIONS_SUSPENDED | BUCKET_MFA_ENABLED); }
-  bool versioning_enabled() { return versioning_status() == BUCKET_VERSIONED; }
-  bool mfa_enabled() { return versioning_status() == BUCKET_MFA_ENABLED; }
+  bool versioning_enabled() { return (versioning_status() & (BUCKET_VERSIONED | BUCKET_VERSIONS_SUSPENDED)) == BUCKET_VERSIONED; }
+  bool mfa_enabled() { return (versioning_status() & BUCKET_MFA_ENABLED) != 0; }
   bool datasync_flag_enabled() const { return (flags & BUCKET_DATASYNC_DISABLED) == 0; }
 
   bool has_swift_versioning() const {

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 89319c3f798cea65f06795b6f147ac2966ea9b80..8193d77b14e323cffcfaf2a9d0808f64bed97472 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -2171,6 +2171,12 @@ void RGWSetBucketVersioning::execute()
   if (op_ret < 0)
     return;
 
+  if (mfa_set_status &&
+      !s->mfa_verified) {
+    op_ret = -ERR_MFA_REQUIRED;
+    return;
+  }
+
   if (!store->is_meta_master()) {
     op_ret = forward_request_to_master(s, NULL, store, in_data, nullptr);
     if (op_ret < 0) {
@@ -2179,20 +2185,35 @@ void RGWSetBucketVersioning::execute()
     }
   }
 
+  bool modified = mfa_set_status;
+
   op_ret = retry_raced_bucket_write(store, s, [this] {
+      if (mfa_set_status) {
+        if (mfa_status) {
+          s->bucket_info.flags |= BUCKET_MFA_ENABLED;
+        } else {
+          s->bucket_info.flags &= ~BUCKET_MFA_ENABLED;
+        }
+      }
+
       if (versioning_status == VersioningEnabled) {
        s->bucket_info.flags |= BUCKET_VERSIONED;
        s->bucket_info.flags &= ~BUCKET_VERSIONS_SUSPENDED;
+        modified = true;
       } else if (versioning_status == VersioningSuspended) {
        s->bucket_info.flags |= (BUCKET_VERSIONED | BUCKET_VERSIONS_SUSPENDED);
+        modified = true;
       } else {
        return op_ret;
       }
-      op_ret = store->put_bucket_instance_info(s->bucket_info, false, real_time(),
-                                              &s->bucket_attrs);
-      return op_ret;
+      return store->put_bucket_instance_info(s->bucket_info, false, real_time(),
+                                             &s->bucket_attrs);
     });
 
+  if (!modified) {
+    return;
+  }
+
   if (op_ret < 0) {
     ldout(s->cct, 0) << "NOTICE: put_bucket_info on bucket=" << s->bucket.name
                     << " returned err=" << op_ret << dendl;
@@ -4327,6 +4348,13 @@ int RGWDeleteObj::verify_permission()
     return -EACCES;
   }
 
+  if (s->bucket_info.mfa_enabled() &&
+      !s->object.instance.empty() &&
+      !s->mfa_verified) {
+    ldout(s->cct, 5) << "NOTICE: object delete request with a versioned object, mfa auth not provided" << dendl;
+    return -ERR_MFA_REQUIRED;
+  }
+
   return 0;
 }
 
@@ -5939,6 +5967,21 @@ void RGWDeleteMultiObj::execute()
   if (multi_delete->is_quiet())
     quiet = true;
 
+  if (s->bucket_info.mfa_enabled()) {
+    bool has_versioned = false;
+    for (auto i : multi_delete->objects) {
+      if (!i.instance.empty()) {
+        has_versioned = true;
+        break;
+      }
+    }
+    if (has_versioned && !s->mfa_verified) {
+      ldout(s->cct, 5) << "NOTICE: multi-object delete request with a versioned object, mfa auth not provided" << dendl;
+      op_ret = -ERR_MFA_REQUIRED;
+      goto error;
+    }
+  }
+
   begin_response();
   if (multi_delete->objects.empty()) {
     goto done;

diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h
index 4c85af9966c18c4edfae96abe5945b3e22a78d8d..952fe36d1643b4db49a05c3d9f046b9873710731 100644 (file)
--- a/src/rgw/rgw_op.h
+++ b/src/rgw/rgw_op.h
@@ -776,6 +776,7 @@ enum BucketVersionStatus {
 class RGWSetBucketVersioning : public RGWOp {
 protected:
   int versioning_status;
+  bool mfa_set_status{false};
   bool mfa_status{false};
   bufferlist in_data;
 public:

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index f7070939126c06ed27754007edb0ae298a0c9c6c..32cdb81d8b5bfcae4579abdcbdd82a98cfe4c574 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -885,30 +885,33 @@ void RGWGetBucketVersioning_ObjStore_S3::send_response()
 
 struct ver_config_status {
   int status{VersioningSuspended};
-  int mfa_status{0};
+
+  enum MFAStatus {
+    MFA_UNKNOWN,
+    MFA_DISABLED,
+    MFA_ENABLED,
+  } mfa_status{MFA_UNKNOWN};
+  int retcode{0};
 
   void decode_xml(XMLObj *obj) {
-dout(0) << __FILE__ << ":" << __LINE__ << dendl;
     string status_str;
     string mfa_str;
     RGWXMLDecoder::decode_xml("Status", status_str, obj);
-dout(0) << __FILE__ << ":" << __LINE__ << dendl;
     if (status_str == "Enabled") {
       status = VersioningEnabled;
     } else if (status_str != "Suspended") {
       status = VersioningStatusInvalid;
     }
 
-dout(0) << __FILE__ << ":" << __LINE__ << " status_str=" << status_str << dendl;
 
-    RGWXMLDecoder::decode_xml("MfaDelete", mfa_str, obj);
-dout(0) << __FILE__ << ":" << __LINE__ << " mfa_str=" << mfa_str << dendl;
-    if (mfa_str == "Enabled") {
-      mfa_status = 1;
-    } else if (mfa_str == "Disabled") {
-      mfa_status = 0;
-    } else {
-      mfa_status = -EINVAL;
+    if (RGWXMLDecoder::decode_xml("MfaDelete", mfa_str, obj)) {
+      if (mfa_str == "Enabled") {
+        mfa_status = MFA_ENABLED;
+      } else if (mfa_str == "Disabled") {
+        mfa_status = MFA_DISABLED;
+      } else {
+        retcode = -EINVAL;
+      }
     }
   }
 };
@@ -937,14 +940,17 @@ int RGWSetBucketVersioning_ObjStore_S3::get_params()
   }
 
   if (!parser.parse(data, len, 1)) {
-    ldout(s->cct, 10) << "failed to parse data: " << data << dendl;
+    ldout(s->cct, 10) << "NOTICE: failed to parse data: " << data << dendl;
     r = -EINVAL;
     return r;
   }
 
   ver_config_status status_conf;
 
-  RGWXMLDecoder::decode_xml("VersioningConfiguration", status_conf, &parser);
+  if (!RGWXMLDecoder::decode_xml("VersioningConfiguration", status_conf, &parser)) {
+    ldout(s->cct, 10) << "NOTICE: bad versioning config input" << dendl;
+    return -EINVAL;
+  }
 
   if (!store->is_meta_master()) {
     /* only need to keep this data around if we're not meta master */
@@ -956,10 +962,21 @@ int RGWSetBucketVersioning_ObjStore_S3::get_params()
     r = -EINVAL;
   }
 
-  if (status_conf.mfa_status >= 0) {
-    mfa_status = (bool)status_conf.mfa_status;
-  } else {
-    r = -EINVAL;
+  if (status_conf.mfa_status != ver_config_status::MFA_UNKNOWN) {
+    mfa_set_status = true;
+    switch (status_conf.mfa_status) {
+      case ver_config_status::MFA_DISABLED:
+        mfa_status = false;
+        break;
+      case ver_config_status::MFA_ENABLED:
+        mfa_status = true;
+        break;
+      default:
+        ldout(s->cct, 0) << "ERROR: RGWSetBucketVersioning_ObjStore_S3::get_params(): unexpected switch case mfa_status=" << status_conf.mfa_status << dendl;
+        r = -EIO;
+    }
+  } else if (status_conf.retcode < 0) {
+    r = status_conf.retcode;
   }
   return r;
 }