You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by interma <gi...@git.apache.org> on 2017/07/25 09:30:47 UTC

[GitHub] incubator-hawq pull request #1269: HAWQ-1506. Fix multi-append bug of write ...

GitHub user interma opened a pull request:

    https://github.com/apache/incubator-hawq/pull/1269

    HAWQ-1506. Fix multi-append bug of write a encryption zone

    This bug is happened on: 
    1, append a existed file. 
    2, call multi `hdfsWrite()`
    
    Test case please see `TEST(TestCInterfaceTDE, TestAppendMultiTimes)`.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/interma/interma-hawq hawq-1506

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-hawq/pull/1269.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1269
    
----
commit 0947e052fe4f6685a178ee2fd1cd78616ec740f9
Author: interma <in...@outlook.com>
Date:   2017-07-25T08:50:25Z

    HAWQ-1506. Fix multi-append bug of write a encryption zone

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1269: HAWQ-1506. Support multi-append a file wi...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1269#discussion_r129487114
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -25,154 +25,188 @@
     
     using namespace Hdfs::Internal;
     
    -namespace Hdfs {
    -
    -/**
    - * Construct a CryptoCodec instance.
    - * @param encryptionInfo the encryption info of file.
    - * @param kcp a KmsClientProvider instance to get key from kms server.
    - * @param bufSize crypto buffer size.
    - */
    -CryptoCodec::CryptoCodec(FileEncryptionInfo *encryptionInfo, shared_ptr<KmsClientProvider> kcp, int32_t bufSize) : encryptionInfo(encryptionInfo), kcp(kcp), bufSize(bufSize)
    -{
    -
    -    /* Init global status. */
    -    ERR_load_crypto_strings();
    -    OpenSSL_add_all_algorithms();
    -    OPENSSL_config(NULL);
    -
    -    /* Create cipher context. */
    -    encryptCtx = EVP_CIPHER_CTX_new();
    -    cipher = NULL;
    -
    -}
    -
    -/**
    - * Destroy a CryptoCodec instance.
    - */
    -CryptoCodec::~CryptoCodec()
    -{
    -    if (encryptCtx)
    -        EVP_CIPHER_CTX_free(encryptCtx);
    -}
    -
    -/**
    - * Get decrypted key from kms.
    - */
    -std::string CryptoCodec::getDecryptedKeyFromKms()
    -{
    -    ptree map = kcp->decryptEncryptedKey(*encryptionInfo);
    -    std::string key;
    -    try {
    -        key = map.get < std::string > ("material");
    -    } catch (...) {
    -        THROW(HdfsIOException, "CryptoCodec : Can not get key from kms.");
    -    }
    -
    -    int rem = key.length() % 4;
    -    if (rem) {
    -        rem = 4 - rem;
    -        while (rem != 0) {
    -            key = key + "=";
    -            rem--;
    -        }
    -    }
    -
    -    std::replace(key.begin(), key.end(), '-', '+');
    -    std::replace(key.begin(), key.end(), '_', '/');
    -
    -    LOG(INFO, "CryptoCodec : getDecryptedKeyFromKms material is :%s", key.c_str());
    -
    -    key = KmsClientProvider::base64Decode(key);
    -    return key;
    -
    -	
    -}
    -
    -/**
    - * Common encode/decode buffer method.
    - * @param buffer the buffer to be encode/decode.
    - * @param size the size of buffer.
    - * @param enc true is for encode, false is for decode.
    - * @return return the encode/decode buffer.
    - */
    -std::string CryptoCodec::endecInternal(const char * buffer, int64_t size, bool enc)
    -{
    -    std::string key = encryptionInfo->getKey();
    -    std::string iv = encryptionInfo->getIv();
    -    LOG(INFO,
    -            "CryptoCodec : endecInternal info. key:%s, iv:%s, buffer:%s, size:%d, is_encode:%d.",
    -            key.c_str(), iv.c_str(), buffer, size, enc);
    -	
    -    /* Get decrypted key from KMS */
    -    key = getDecryptedKeyFromKms();
    -
    -    /* Select cipher method based on the key length. */
    -    if (key.length() == KEY_LENGTH_256) {
    -        cipher = EVP_aes_256_ctr();
    -    } else if (key.length() == KEY_LENGTH_128) {
    -        cipher = EVP_aes_128_ctr();
    -    } else {
    -        THROW(InvalidParameter, "CryptoCodec : Invalid key length.");
    -    }
    -
    -    /* Init cipher context with cipher method, encrypted key and IV from KMS. */
    -    int encode = enc ? 1 : 0;
    -    if (!EVP_CipherInit_ex(encryptCtx, cipher, NULL,
    -            (const unsigned char *) key.c_str(),
    -            (const unsigned char *) iv.c_str(), encode)) {
    -        LOG(WARNING, "EVP_CipherInit_ex failed");
    -    }
    -    LOG(DEBUG3, "EVP_CipherInit_ex successfully");
    -    EVP_CIPHER_CTX_set_padding(encryptCtx, 0);
    -
    -    /* Encode/decode buffer within cipher context. */
    -    std::string result;
    -    result.resize(size);
    -    int offset = 0;
    -    int remaining = size;
    -    int len = 0;
    -    /* If the encode/decode buffer size larger than crypto buffer size, encode/decode buffer one by one. */
    -    while (remaining > bufSize) {
    -        if (!EVP_CipherUpdate(encryptCtx, (unsigned char *) &result[offset],
    -                &len, (const unsigned char *) buffer + offset, bufSize)) {
    -            std::string err = ERR_lib_error_string(ERR_get_error());
    -            THROW(HdfsIOException, "CryptoCodec : Cannot encrypt AES data %s",
    -                    err.c_str());
    -        }
    -        offset += len;
    -        remaining -= len;
    -        LOG(DEBUG3,
    -                "CryptoCodec : EVP_CipherUpdate successfully, result:%s, len:%d",
    -                result.c_str(), len);
    -    }
    -    if (remaining) {
    -        if (!EVP_CipherUpdate(encryptCtx, (unsigned char *) &result[offset],
    -                &len, (const unsigned char *) buffer + offset, remaining)) {
    -            std::string err = ERR_lib_error_string(ERR_get_error());
    -            THROW(HdfsIOException, "CryptoCodec : Cannot encrypt AES data %s",
    -                    err.c_str());
    -        }
    -    }
    -
    -    return result;
    -}
     
    -/**
    - * Encode buffer.
    - */
    -std::string CryptoCodec::encode(const char * buffer, int64_t size)
    -{
    -    return endecInternal(buffer, size, true);
    -}
    +namespace Hdfs {
     
    -/**
    - * Decode buffer.
    - */	
    -std::string CryptoCodec::decode(const char * buffer, int64_t size)
    -{
    -    return endecInternal(buffer, size, false);
    -}
    +	//copy from java HDFS code
    +	std::string CryptoCodec::calculateIV(const std::string& initIV, unsigned long counter) {
    +		char IV[initIV.length()];
    +
    +		int i = initIV.length(); // IV length
    +		int j = 0; // counter bytes index
    +		unsigned int sum = 0;
    +		while (i-- > 0) {
    +			// (sum >>> Byte.SIZE) is the carry for addition
    +			sum = (initIV[i] & 0xff) + (sum >> 8);
    +			if (j++ < 8) { // Big-endian, and long is 8 bytes length
    +				sum += (char) counter & 0xff;
    +				counter >>= 8;
    +			}
    +			IV[i] = (char) sum;
    +		}
    +
    +		return std::string(IV, initIV.length());
    +	}
    +
    +	CryptoCodec::CryptoCodec(FileEncryptionInfo *encryptionInfo, shared_ptr<KmsClientProvider> kcp, int32_t bufSize) :
    +		encryptionInfo(encryptionInfo), kcp(kcp), bufSize(bufSize)
    +	{
    +
    +		// Init global status
    +		ERR_load_crypto_strings();
    +		OpenSSL_add_all_algorithms();
    +		OPENSSL_config(NULL);
    +
    +		// Create cipher context
    +		cipherCtx = EVP_CIPHER_CTX_new();
    +		cipher = NULL;
    +
    +		padding = 0;
    +		counter = 0;
    +		is_init = false;
    +	}
    +
    +	CryptoCodec::~CryptoCodec()
    +	{
    +		if (cipherCtx)
    +			EVP_CIPHER_CTX_free(cipherCtx);
    +	}
    +
    +	std::string CryptoCodec::getDecryptedKeyFromKms()
    +	{
    +		ptree map = kcp->decryptEncryptedKey(*encryptionInfo);
    +		std::string key;
    +		try {
    +			key = map.get < std::string > ("material");
    +		} catch (...) {
    +			THROW(HdfsIOException, "CryptoCodec : Can not get key from kms.");
    +		}
    +
    +		int rem = key.length() % 4;
    +		if (rem) {
    +			rem = 4 - rem;
    +			while (rem != 0) {
    +				key = key + "=";
    +				rem--;
    +			}
    +		}
    +
    +		std::replace(key.begin(), key.end(), '-', '+');
    +		std::replace(key.begin(), key.end(), '_', '/');
    +
    +		LOG(INFO, "CryptoCodec : getDecryptedKeyFromKms material is :%s", key.c_str());
    +
    +		key = KmsClientProvider::base64Decode(key);
    +		return key;
    +	}
    +
    +	int CryptoCodec::init(CryptoMethod crypto_method, int64_t stream_offset) {
    +		//check already init
    +		if (is_init)
    +			return 0;
    +
    +		// Get decrypted key from KMS
    +		std::string key;
    +		try {
    +			key = getDecryptedKeyFromKms();
    +		}
    +		catch (...) {
    +			LOG(WARNING, "getDecryptedKeyFromKms failed");
    --- End diff --
    
    You are right, the exception should throw to upper caller to judge how to deal with it.
    Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1269: HAWQ-1506. Support multi-append a file within en...

Posted by radarwave <gi...@git.apache.org>.
Github user radarwave commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1269
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1269: HAWQ-1506. Fix multi-append bug of write a encry...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1269
  
    @linwen @wengyanqing @amyrazz44 @wcl14 help to review, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1269: HAWQ-1506. Support multi-append a file within en...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1269
  
    merged


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1269: HAWQ-1506. Fix multi-append bug of write ...

Posted by wengyanqing <gi...@git.apache.org>.
Github user wengyanqing commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1269#discussion_r129261893
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -25,154 +25,188 @@
     
     using namespace Hdfs::Internal;
     
    -namespace Hdfs {
    -
    -/**
    - * Construct a CryptoCodec instance.
    - * @param encryptionInfo the encryption info of file.
    - * @param kcp a KmsClientProvider instance to get key from kms server.
    - * @param bufSize crypto buffer size.
    - */
    -CryptoCodec::CryptoCodec(FileEncryptionInfo *encryptionInfo, shared_ptr<KmsClientProvider> kcp, int32_t bufSize) : encryptionInfo(encryptionInfo), kcp(kcp), bufSize(bufSize)
    -{
    -
    -    /* Init global status. */
    -    ERR_load_crypto_strings();
    -    OpenSSL_add_all_algorithms();
    -    OPENSSL_config(NULL);
    -
    -    /* Create cipher context. */
    -    encryptCtx = EVP_CIPHER_CTX_new();
    -    cipher = NULL;
    -
    -}
    -
    -/**
    - * Destroy a CryptoCodec instance.
    - */
    -CryptoCodec::~CryptoCodec()
    -{
    -    if (encryptCtx)
    -        EVP_CIPHER_CTX_free(encryptCtx);
    -}
    -
    -/**
    - * Get decrypted key from kms.
    - */
    -std::string CryptoCodec::getDecryptedKeyFromKms()
    -{
    -    ptree map = kcp->decryptEncryptedKey(*encryptionInfo);
    -    std::string key;
    -    try {
    -        key = map.get < std::string > ("material");
    -    } catch (...) {
    -        THROW(HdfsIOException, "CryptoCodec : Can not get key from kms.");
    -    }
    -
    -    int rem = key.length() % 4;
    -    if (rem) {
    -        rem = 4 - rem;
    -        while (rem != 0) {
    -            key = key + "=";
    -            rem--;
    -        }
    -    }
    -
    -    std::replace(key.begin(), key.end(), '-', '+');
    -    std::replace(key.begin(), key.end(), '_', '/');
    -
    -    LOG(INFO, "CryptoCodec : getDecryptedKeyFromKms material is :%s", key.c_str());
    -
    -    key = KmsClientProvider::base64Decode(key);
    -    return key;
    -
    -	
    -}
    -
    -/**
    - * Common encode/decode buffer method.
    - * @param buffer the buffer to be encode/decode.
    - * @param size the size of buffer.
    - * @param enc true is for encode, false is for decode.
    - * @return return the encode/decode buffer.
    - */
    -std::string CryptoCodec::endecInternal(const char * buffer, int64_t size, bool enc)
    -{
    -    std::string key = encryptionInfo->getKey();
    -    std::string iv = encryptionInfo->getIv();
    -    LOG(INFO,
    -            "CryptoCodec : endecInternal info. key:%s, iv:%s, buffer:%s, size:%d, is_encode:%d.",
    -            key.c_str(), iv.c_str(), buffer, size, enc);
    -	
    -    /* Get decrypted key from KMS */
    -    key = getDecryptedKeyFromKms();
    -
    -    /* Select cipher method based on the key length. */
    -    if (key.length() == KEY_LENGTH_256) {
    -        cipher = EVP_aes_256_ctr();
    -    } else if (key.length() == KEY_LENGTH_128) {
    -        cipher = EVP_aes_128_ctr();
    -    } else {
    -        THROW(InvalidParameter, "CryptoCodec : Invalid key length.");
    -    }
    -
    -    /* Init cipher context with cipher method, encrypted key and IV from KMS. */
    -    int encode = enc ? 1 : 0;
    -    if (!EVP_CipherInit_ex(encryptCtx, cipher, NULL,
    -            (const unsigned char *) key.c_str(),
    -            (const unsigned char *) iv.c_str(), encode)) {
    -        LOG(WARNING, "EVP_CipherInit_ex failed");
    -    }
    -    LOG(DEBUG3, "EVP_CipherInit_ex successfully");
    -    EVP_CIPHER_CTX_set_padding(encryptCtx, 0);
    -
    -    /* Encode/decode buffer within cipher context. */
    -    std::string result;
    -    result.resize(size);
    -    int offset = 0;
    -    int remaining = size;
    -    int len = 0;
    -    /* If the encode/decode buffer size larger than crypto buffer size, encode/decode buffer one by one. */
    -    while (remaining > bufSize) {
    -        if (!EVP_CipherUpdate(encryptCtx, (unsigned char *) &result[offset],
    -                &len, (const unsigned char *) buffer + offset, bufSize)) {
    -            std::string err = ERR_lib_error_string(ERR_get_error());
    -            THROW(HdfsIOException, "CryptoCodec : Cannot encrypt AES data %s",
    -                    err.c_str());
    -        }
    -        offset += len;
    -        remaining -= len;
    -        LOG(DEBUG3,
    -                "CryptoCodec : EVP_CipherUpdate successfully, result:%s, len:%d",
    -                result.c_str(), len);
    -    }
    -    if (remaining) {
    -        if (!EVP_CipherUpdate(encryptCtx, (unsigned char *) &result[offset],
    -                &len, (const unsigned char *) buffer + offset, remaining)) {
    -            std::string err = ERR_lib_error_string(ERR_get_error());
    -            THROW(HdfsIOException, "CryptoCodec : Cannot encrypt AES data %s",
    -                    err.c_str());
    -        }
    -    }
    -
    -    return result;
    -}
     
    -/**
    - * Encode buffer.
    - */
    -std::string CryptoCodec::encode(const char * buffer, int64_t size)
    -{
    -    return endecInternal(buffer, size, true);
    -}
    +namespace Hdfs {
     
    -/**
    - * Decode buffer.
    - */	
    -std::string CryptoCodec::decode(const char * buffer, int64_t size)
    -{
    -    return endecInternal(buffer, size, false);
    -}
    +	//copy from java HDFS code
    +	std::string CryptoCodec::calculateIV(const std::string& initIV, unsigned long counter) {
    +		char IV[initIV.length()];
    +
    +		int i = initIV.length(); // IV length
    +		int j = 0; // counter bytes index
    +		unsigned int sum = 0;
    +		while (i-- > 0) {
    +			// (sum >>> Byte.SIZE) is the carry for addition
    +			sum = (initIV[i] & 0xff) + (sum >> 8);
    +			if (j++ < 8) { // Big-endian, and long is 8 bytes length
    +				sum += (char) counter & 0xff;
    +				counter >>= 8;
    +			}
    +			IV[i] = (char) sum;
    +		}
    +
    +		return std::string(IV, initIV.length());
    +	}
    +
    +	CryptoCodec::CryptoCodec(FileEncryptionInfo *encryptionInfo, shared_ptr<KmsClientProvider> kcp, int32_t bufSize) :
    +		encryptionInfo(encryptionInfo), kcp(kcp), bufSize(bufSize)
    +	{
    +
    +		// Init global status
    +		ERR_load_crypto_strings();
    +		OpenSSL_add_all_algorithms();
    +		OPENSSL_config(NULL);
    +
    +		// Create cipher context
    +		cipherCtx = EVP_CIPHER_CTX_new();
    +		cipher = NULL;
    +
    +		padding = 0;
    +		counter = 0;
    +		is_init = false;
    +	}
    +
    +	CryptoCodec::~CryptoCodec()
    +	{
    +		if (cipherCtx)
    +			EVP_CIPHER_CTX_free(cipherCtx);
    +	}
    +
    +	std::string CryptoCodec::getDecryptedKeyFromKms()
    +	{
    +		ptree map = kcp->decryptEncryptedKey(*encryptionInfo);
    +		std::string key;
    +		try {
    +			key = map.get < std::string > ("material");
    +		} catch (...) {
    +			THROW(HdfsIOException, "CryptoCodec : Can not get key from kms.");
    +		}
    +
    +		int rem = key.length() % 4;
    +		if (rem) {
    +			rem = 4 - rem;
    +			while (rem != 0) {
    +				key = key + "=";
    +				rem--;
    +			}
    +		}
    +
    +		std::replace(key.begin(), key.end(), '-', '+');
    +		std::replace(key.begin(), key.end(), '_', '/');
    +
    +		LOG(INFO, "CryptoCodec : getDecryptedKeyFromKms material is :%s", key.c_str());
    +
    +		key = KmsClientProvider::base64Decode(key);
    +		return key;
    +	}
    +
    +	int CryptoCodec::init(CryptoMethod crypto_method, int64_t stream_offset) {
    +		//check already init
    +		if (is_init)
    +			return 0;
    +
    +		// Get decrypted key from KMS
    +		std::string key;
    +		try {
    +			key = getDecryptedKeyFromKms();
    +		}
    +		catch (...) {
    +			LOG(WARNING, "getDecryptedKeyFromKms failed");
    --- End diff --
    
    It's better to throw the exception to upper function then return error.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1269: HAWQ-1506. Support multi-append a file wi...

Posted by interma <gi...@git.apache.org>.
Github user interma closed the pull request at:

    https://github.com/apache/incubator-hawq/pull/1269


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1269: HAWQ-1506. Support multi-append a file within en...

Posted by wengyanqing <gi...@git.apache.org>.
Github user wengyanqing commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1269
  
    LGTM. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---