DNM support client crypto multipart upload by yangwanyuan · Pull Request #157 · aliyun/aliyun-oss-python-sdk

yangwanyuan · 2019-01-23T14:04:14Z

Support client crypto multipart upload, but now use x-oss-meta-* as crypto header. Will replace with x-oss-client-side-crypto-* after OSS server code is ready

coveralls · 2019-01-23T14:23:36Z

Coverage decreased (-35.3%) to 61.99% when pulling 0c6782b on yangwanyuan:ywy-multipart-crypto-dev-20190123 into df1e8fa on aliyun:master.

examples/object_crypto.py

oss2/api.py

oss2/resumable.py

oss2/api.py

hangzws · 2019-02-12T08:27:01Z

tests/test_object.py

+        get_result = self.rsa_crypto_bucket.get_object(key, byte_range=(32, 103))
+        self.assertEqual(get_result.read(), content[32:103+1])
+
+        self.assertRaises(oss2.exceptions.ClientError, self.rsa_crypto_bucket.get_object, key, byte_range=(31, None))


这个返回的异常需要再具体下，返回clienterror无法知道错误是什么

在api.py中是会返回具体message的。这里只捕获ClientError异常是因为参考了其他测试用例里的使用方法，保持代码风格一致。

tests/test_object.py

oss2/headers.py

oss2/api.py

coderall · 2019-02-13T02:22:38Z

oss2/api.py

-        if 'range' in headers:
-            raise ClientError('Crypto bucket do not support range get')
+        if byte_range and (not utils.is_multiple_sizeof_encrypt_block(byte_range[0])):
+            raise ClientError('Crypto bucket get range start must align to encrypt block')


这里把出错的byte_range打出来，方便debug

恩，这个版本先把byte_range打印出来，debug的时候可以看的比较清楚，另外range只支持对齐要在文档的说明。如果后续用户有不对齐的需求，后续版本可能还要考虑是否可以兼容？

oss2/api.py

coderall · 2019-02-13T09:30:46Z

oss2/crypto.py


+    def check_plain_key_valid(self, plain_key, plain_key_hmac):
+        if str(hash(plain_key)) != plain_key_hmac:
+            raise ClientError("The decrypted key is inconsistent, make sure use right RSA key pair")


这里检查的应该是私钥的正确性，在错误提示中可以更明确点指出

tests/test_multipart.py

hangzws · 2019-02-15T07:09:07Z

oss2/api.py

+        :return: :class:`ListPartsResult <oss2.models.ListPartsResult>`
+        """
+        logger.info("Start list parts securely, upload_id = {0}".format(upload_id))
+        try:


缺少uploadPartCopy接口？

coderall · 2019-04-11T06:37:27Z

examples/custom_crypto.py


        if 'content-md5' in headers:
-            headers['x-oss-meta-unencrypted-content-md5'] = headers['content-md5']
+            headers[OSS_CLIENT_SIDE_ENCRYPTION_UNENCRYPTED_CONTENT_MD5] = headers['content-md5']


之前的逻辑是用户如果传入MD5，那么后端回校验，现在如果传入未加密数据的MD5，后端的逻辑怎么处理？

后端对此不做任何处理，这里把MD5去掉的原因就是防止后端发现加密数据和明文数据的MD5不一样而报错，可以考虑重新计算加密数据的MD5填入。

coderall · 2019-04-11T06:54:28Z

examples/object_crypto.py

+#    parts.append(oss2.models.PartInfo(i+1, result.etag, size = part_size, part_crc = result.crc))
+#
+#res = bucket.list_parts(multi_key, upload_id)
+#crypto_multipart_context_new = res.crypto_multipart_context


这里的示例能不能加上提示，提醒用户在中断后，获取上一次的context后，再次校验下以符合预期

好的，可以加上一些文字提示。由于现在对part的合法性判断全部移到服务端做了，所以用户是否校验符合预期已经不重要了，因为就算不符合，下一次上传时服务端也会报错。

coderall · 2019-04-11T07:28:22Z

oss2/crypto.py

            raise e
        except:
-            return None
+            return None


这里为什么不把异常抛出去

这里我没有做特殊更改，保留了原始开发者的代码处理逻辑。

这个地方有异常肯定要抛出来啊，原始开发的逻辑是不对的。

oss2/models.py

coderall · 2019-04-16T06:29:02Z

oss2/api.py

+
+        crypto_key = self.crypto_provider.decrypt_from_str(OSS_CLIENT_SIDE_ENCRYPTION_KEY, res.crypto_key)
+        crypto_start = int(self.crypto_provider.decrypt_from_str(OSS_CLIENT_SIDE_ENCRYPTION_START, res.crypto_start))
+        context = CryptoMultipartContext(crypto_key, crypto_start, res.client_encryption_data_size, res.client_encryption_part_size)


结合语雀文档上的说明，在list接口中现在没有返回 key-hmac ？是否考虑加上，并添加校验逻辑？

hangzws · 2019-04-16T09:45:41Z

oss2/models.py

+            count_start = self.__crypto_provider.decrypt_oss_meta_data(resp.headers, OSS_CLIENT_SIDE_ENCRYPTION_START)
+
+            # if content range , adjust the decrypt adapter
+            count_offset = 0;


这一行多了一个分号

hangzws · 2019-04-16T10:01:04Z

oss2/api.py

+        headers = self.crypto_provider.build_header(headers, context)
+
+        resp = self.bucket.init_multipart_upload(key, headers)
+        resp.crypto_multipart_context = context;


这个地方有多余的分号

hangzws · 2019-04-16T10:13:08Z

oss2/api.py


            return result

+    def init_multipart_upload(self, key, data_size, part_size = None, headers=None):


这个地方data_size的值如果跟真实文件不一致会导致什么问题？complete的时候回校验这个data_size

hangzws · 2019-04-16T11:50:14Z

oss2/api.py

+        logger.info("Start upload part by crypto bucket, upload_id = {0}, part_number = {1}".format(upload_id, part_number))
+
+        headers = http.CaseInsensitiveDict(headers)
+        headers[FLAG_CLIENT_SIDE_ENCRYPTION_MULTIPART_FILE] = "true"


这个header的命名为啥跟其他的命名风格不一致？

为啥需要客户端传入FLAG_CLIENT_SIDE_ENCRYPTION_MULTIPART_FILE，有没有其他的解决方案？

hangzws · 2019-04-16T12:07:28Z

oss2/api.py

+
+        return res
+
+    def list_parts(self, key, upload_id,


uploadPartCopy接口的设计呢？

hangzws · 2019-04-17T03:52:27Z

oss2/api.py

        resp = self.__do_object('GET', key, headers=headers, params=params)
        logger.debug("Get object done, req_id: {0}, status_code: {1}".format(resp.request_id, resp.status))

+        if models._hget(resp.headers, OSS_CLIENT_SIDE_ENCRYPTION_KEY):


这个地方在get_object这里做了限制，其他操作如symlink或者uploadpartcopy可以做类似的限制么？

yangwanyuan added 8 commits January 23, 2019 21:29

add decrypt get range data feature in CryptoBucket

4b07044

add unittest for get range

7c90852

add tests of rsa and kms crypto range get

2ac86a6

add CryptoBucket get range example

ce4199c

add crypto Bucket multipart upload feature

c7a0551

add CryptoBucket multipart upload example

5e9b27e

add utils unittest

8b56d0d

add CryptoBucket multepart upload tests

651d73c

huiguangjun requested review from ailan-gl, coderall and hangzws and removed request for ailan-gl January 24, 2019 05:51

coderall reviewed Jan 24, 2019

View reviewed changes

examples/object_crypto.py Show resolved Hide resolved

coderall reviewed Jan 24, 2019

View reviewed changes

examples/object_crypto.py Show resolved Hide resolved

coderall reviewed Jan 24, 2019

View reviewed changes

oss2/api.py Outdated Show resolved Hide resolved

coderall reviewed Jan 24, 2019

View reviewed changes

oss2/api.py Outdated Show resolved Hide resolved

coderall reviewed Jan 24, 2019

View reviewed changes

oss2/api.py Show resolved Hide resolved

coderall reviewed Jan 24, 2019

View reviewed changes

oss2/resumable.py Show resolved Hide resolved

coderall reviewed Feb 1, 2019

View reviewed changes

oss2/api.py Outdated Show resolved Hide resolved

coderall reviewed Feb 1, 2019

View reviewed changes

oss2/api.py Outdated Show resolved Hide resolved

yangwanyuan added 3 commits February 2, 2019 22:23

add some crypto bucket tests

87b6f4b

fix kms_crypto_bucket example bug

0df71c2

add upload_id to ClientError msg

9a22e8c

hangzws reviewed Feb 12, 2019

View reviewed changes

coderall reviewed Feb 13, 2019

View reviewed changes

oss2/api.py Outdated Show resolved Hide resolved

coderall reviewed Feb 13, 2019

View reviewed changes

oss2/api.py Outdated Show resolved Hide resolved

coderall reviewed Feb 13, 2019

View reviewed changes

oss2/api.py Outdated Show resolved Hide resolved

coderall reviewed Feb 13, 2019

View reviewed changes

tests/test_multipart.py Outdated Show resolved Hide resolved

hangzws reviewed Feb 15, 2019

View reviewed changes

refine multipart client encryption code

0c6782b

coderall reviewed Apr 11, 2019

View reviewed changes

oss2/models.py Show resolved Hide resolved

coderall reviewed Apr 16, 2019

View reviewed changes

hangzws reviewed Apr 17, 2019

View reviewed changes


		return result

		def init_multipart_upload(self, key, data_size, part_size = None, headers=None):

Conversation

yangwanyuan commented Jan 23, 2019

Uh oh!

coveralls commented Jan 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hangzws Apr 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Jan 23, 2019 •

edited

Loading

hangzws Apr 17, 2019 •

edited

Loading