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

[GitHub] incubator-hawq pull request #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

GitHub user amyrazz44 opened a pull request:

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

    HAWQ-1500. HAWQ-1501. HAWQ-1502. Support TDE write function.

    

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

    $ git pull https://github.com/amyrazz44/incubator-hawq TDEWrite

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

    https://github.com/apache/incubator-hawq/pull/1265.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 #1265
    
----
commit 1c41023612b799afa95f4408f0b20c516c19a791
Author: amyrazz44 <ab...@pivotal.io>
Date:   2017-07-11T07:43:18Z

    HAWQ-1500. Support TDE by adding a common class HttpClient to get response from KMS.

commit 5d3b0ad6e974ccb693735342181c4c6939b13603
Author: amyrazz44 <ab...@pivotal.io>
Date:   2017-07-11T07:51:32Z

    HAWQ-1501. Support TDE by adding KmsClientProvider class to interact with KMS server.

commit a2c62c5c11b9e05b8c37360d23965d5d18c34e73
Author: amyrazz44 <ab...@pivotal.io>
Date:   2017-07-11T07:57:26Z

    HAWQ-1502. Support TDE write function.

----


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

    https://github.com/apache/incubator-hawq/pull/1265#discussion_r126857880
  
    --- Diff: depends/libhdfs3/src/client/HttpClient.cpp ---
    @@ -0,0 +1,337 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "HttpClient.h"
    +#include "Logger.h"
    +
    +using namespace Hdfs::Internal;
    +
    +namespace Hdfs {
    +
    +#define CURL_SETOPT(handle, option, optarg, fmt, ...) \
    +    res = curl_easy_setopt(handle, option, optarg); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, ##__VA_ARGS__); \
    +    }
    +
    +#define CURL_SETOPT_ERROR1(handle, option, optarg, fmt) \
    +    CURL_SETOPT(handle, option, optarg, fmt, curl_easy_strerror(res));
    +
    +#define CURL_SETOPT_ERROR2(handle, option, optarg, fmt) \
    +    CURL_SETOPT(handle, option, optarg, fmt, curl_easy_strerror(res), \
    +        errorString().c_str())
    +
    +#define CURL_PERFORM(handle, fmt) \
    +    res = curl_easy_perform(handle); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, curl_easy_strerror(res), errorString().c_str()); \
    +    }
    +
    +
    +#define CURL_GETOPT_ERROR2(handle, option, optarg, fmt) \
    +    res = curl_easy_getinfo(handle, option, optarg); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, curl_easy_strerror(res), errorString().c_str()); \
    +    }
    +
    +#define CURL_GET_RESPONSE(handle, code, fmt) \
    +    CURL_GETOPT_ERROR2(handle, CURLINFO_RESPONSE_CODE, code, fmt);
    +
    +HttpClient::HttpClient() : curl(NULL), list(NULL) {
    +
    +}
    +
    +/**
    + * Construct a HttpClient instance.
    + * @param url a url which is the address to send the request to the corresponding http server.
    + */
    +HttpClient::HttpClient(const std::string &url) {
    +	curl = NULL;
    +	list = NULL;
    +	this->url = url;
    +}
    +
    +/**
    + * Destroy a HttpClient instance.
    + */
    +HttpClient::~HttpClient()
    +{
    +	destroy();
    +}
    +
    +/**
    + * Receive error string from curl.
    + */
    +std::string HttpClient::errorString() {
    +	if (strlen(errbuf) == 0)
    +		return "";
    +	return errbuf;
    +}
    +
    +/**
    + * Curl call back function to receive the reponse messages.
    + * @return return the size of reponse messages. 
    + */
    +size_t HttpClient::CurlWriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp)
    +{
    +      size_t realsize = size * nmemb;
    +	  if (userp == NULL || contents == NULL) {
    +		  return 0;
    +	  }
    +      ((std::string *)userp)->append((const char *)contents, realsize);
    +	  LOG(DEBUG2, "HttpClient : Http response is : %s", ((std::string *)userp)->c_str());
    +      return realsize;
    +}
    +
    +/**
    + * Init curl handler and set curl options.
    + */
    +void HttpClient::init() {
    +	if (!initialized)
    +	{
    +		initialized = true;
    +		if (curl_global_init(CURL_GLOBAL_ALL)) {
    +			THROW(HdfsIOException, "Cannot initialize curl client for KMS");
    +		}
    +	}
    +
    +	curl = curl_easy_init();
    +	if (!curl) {
    +		THROW(HdfsIOException, "Cannot initialize curl handle for KMS");
    +	}
    +	
    +    CURL_SETOPT_ERROR1(curl, CURLOPT_ERRORBUFFER, errbuf,
    +        "Cannot initialize curl error buffer for KMS: %s");
    +
    +    errbuf[0] = 0;
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_NOPROGRESS, 1,
    +        "Cannot initialize no progress in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_VERBOSE, 0,
    +        "Cannot initialize no verbose in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_COOKIEFILE, "",
    +        "Cannot initialize cookie behavior in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_HTTPHEADER, list,
    +        "Cannot initialize headers in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_WRITEFUNCTION, HttpClient::CurlWriteMemoryCallback,
    +        "Cannot initialize body reader in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_WRITEDATA, (void *)&response,
    +        "Cannot initialize body reader data in HttpClient: %s: %s");
    +
    +    /* Some servers don't like requests that are made without a user-agent
    +        field, so we provide one */
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0",
    +        "Cannot initialize user agent in HttpClient: %s: %s");
    +	list = NULL;
    +
    +}
    +
    +/**
    + * Do clean up for curl.
    + */
    +void HttpClient::destroy() {
    +	if (curl) {
    +		curl_easy_cleanup(curl);
    +	}
    +	if (list) {
    +		curl_slist_free_all(list);
    +	}
    +}
    +
    +/**
    + * Set url for http client.
    + */
    +void HttpClient::setURL(const std::string &url) {
    +	this->url = url;
    +}
    +
    +/**
    + * Set retry times for http request which can be configured in config file.
    + */
    +void HttpClient::setRequestRetryTimes(int request_retry_times) {
    +	if (request_retry_times < 0) {
    +		THROW(InvalidParameter, "HttpClient : Invalid value for request_retry_times.");
    +	}
    +	this->request_retry_times = request_retry_times;
    +}
    +
    +/**
    + * Set request timeout which can be configured in config file.
    + */
    +void HttpClient::setRequestTimeout(int64_t curl_timeout) {
    +	if (curl_timeout < 0) {
    +		THROW(InvalidParameter, "HttpClient : Invalid value for curl_timeout.");
    +	}
    +	this->curl_timeout = curl_timeout;
    +}
    +
    +/**
    + * Set headers for http client.
    + */
    +void HttpClient::setHeaders(const std::vector<std::string> &headers) {
    +	if (!headers.empty()) {
    +		this->headers = headers;
    +		for (std::string header : headers) {
    +			list = curl_slist_append(list, header.c_str());
    +			if (!list) {
    +				THROW(HdfsIOException, "Cannot add header in HttpClient.");
    +			}
    +		}	
    +	}
    +	else {
    +		LOG(DEBUG1, "HttpClient : Header is empty.");				
    +	}
    +}
    +
    +
    +/**
    + * Set body for http client.
    + */
    +void HttpClient::setBody(const std::string &body) {
    +	this->body = body;
    +}
    +
    +/**
    + * Set expected response code.
    + */
    +void HttpClient::setExpectedResponseCode(int64_t response_code_ok) {
    +	this->response_code_ok = response_code_ok;
    +}
    +
    +/**
    + * Http common method to get response info by sending request to http server.
    + * @param method : define different http methods.
    + * @return return response info.
    + */
    +std::string HttpClient::httpCommon(httpMethod method) {
    +	
    +    /* Set headers and url. */
    +	if (list != NULL) {
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_HTTPHEADER, list,
    +        	        "Cannot initialize headers in HttpClient: %s: %s");
    +	} else {
    +		LOG(DEBUG1, "HttpClient : Http Header is NULL");
    +	}
    +
    +	if (curl != NULL) {
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_URL, url.c_str(),
    +    	        "Cannot initialize url in HttpClient: %s: %s");
    +	} else {
    +		LOG(DEBUG1, "HttpClient : Http URL is NULL");
    +	}
    +
    +	/* Set body based on different http method. */ 
    +	switch(method) {
    +		case HTTP_GET:
    +			break;
    +		case HTTP_POST:
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_COPYPOSTFIELDS, body.c_str(),
    +                "Cannot initialize post data in HttpClient: %s: %s");
    +			break;
    +		case HTTP_DELETE:
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_CUSTOMREQUEST, "DELETE",
    +                "Cannot initialize set customer request in HttpClient: %s: %s");
    +			break;
    +		case HTTP_PUT:
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_CUSTOMREQUEST, "PUT",
    +                "Cannot initialize set customer request in HttpClient: %s: %s");
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_COPYPOSTFIELDS, body.c_str(),
    +                "Cannot initialize post data in HttpClient: %s: %s");
    +			break;
    +	}
    +
    +	/* Do several http request try according to request_retry_times until got the right reponse code. */	
    +	int64_t response_code = -1;	
    +
    +	while (request_retry_times >= 0 && response_code != response_code_ok) {
    +		request_retry_times -= 1;
    +		response = "";
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_TIMEOUT, curl_timeout, 
    +					"Send request to http server timeout: %s: %s");
    +		CURL_PERFORM(curl, "Could not send request in HttpClient: %s %s");
    +		CURL_GET_RESPONSE(curl, &response_code,
    +					"Cannot get response code in HttpClient: %s: %s");
    +	}
    +	LOG(DEBUG1, "HttpClient : The http method is %d. The http url is %s. The http response is %s.", method, url.c_str(), response.c_str());
    +	return response;
    +}
    +
    +/**
    + * Http GET method.
    + */
    +std::string HttpClient::get() {
    +	httpMethod method = HTTP_GET;
    --- End diff --
    
    agree to @wengyanqing 


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

    https://github.com/apache/incubator-hawq/pull/1265#discussion_r126858617
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::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 = map.get<std::string>("material");
    +
    +	int rem = key.length() % 4;
    +	if (rem) {
    +		rem = 4 - rem;
    +		while (rem != 0) {
    +			key = key + "=";
    +			rem--;
    +		}
    +	}
    --- End diff --
    
    Why still align key as 4 bytes? Should it be encoded as based 64?


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

    https://github.com/apache/incubator-hawq/pull/1265#discussion_r127112526
  
    --- Diff: depends/libhdfs3/src/client/HttpClient.h ---
    @@ -0,0 +1,155 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +#ifndef _HDFS_LIBHDFS3_CLIENT_HTTPCLIENT_H_
    +#define _HDFS_LIBHDFS3_CLIENT_HTTPCLIENT_H_
    +
    +#include <string>
    +#include <vector>
    +#include <curl/curl.h>
    +#include "Exception.h"
    +#include "ExceptionInternal.h"
    +
    +typedef enum httpMethod {
    +    HTTP_GET = 0,
    +	HTTP_POST = 1,
    +	HTTP_DELETE = 2,
    +	HTTP_PUT = 3
    +} httpMethod;
    +
    +namespace Hdfs {
    +
    +class HttpClient {
    +public:
    --- End diff --
    
    If we have something to do in the future, I think a jira should be filed.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126863117
  
    --- Diff: depends/libhdfs3/src/client/HttpClient.h ---
    @@ -0,0 +1,155 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +#ifndef _HDFS_LIBHDFS3_CLIENT_HTTPCLIENT_H_
    +#define _HDFS_LIBHDFS3_CLIENT_HTTPCLIENT_H_
    +
    +#include <string>
    +#include <vector>
    +#include <curl/curl.h>
    +#include "Exception.h"
    +#include "ExceptionInternal.h"
    +
    +typedef enum httpMethod {
    +    HTTP_GET = 0,
    +	HTTP_POST = 1,
    +	HTTP_DELETE = 2,
    +	HTTP_PUT = 3
    +} httpMethod;
    +
    +namespace Hdfs {
    +
    +class HttpClient {
    +public:
    --- End diff --
    
    I think we should add a unit test for HttpClient, because libcurl have many tricky behaviors, maybe we forgot some exceptional path.
    Just a reminder, let's do it in future.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

    https://github.com/apache/incubator-hawq/pull/1265#discussion_r126856856
  
    --- Diff: depends/libhdfs3/CMake/FindSSL.cmake ---
    @@ -0,0 +1,26 @@
    +# - Try to find the Open ssl library (ssl)
    +#
    +# Once done this will define
    +#
    +#  SSL_FOUND - System has gnutls
    +#  SSL_INCLUDE_DIR - The gnutls include directory
    +#  SSL_LIBRARIES - The libraries needed to use gnutls
    +#  SSL_DEFINITIONS - Compiler switches required for using gnutls
    +
    +
    +IF (SSL_INCLUDE_DIR AND SSL_LIBRARIES)
    +	# in cache already
    +	SET(SSL_FIND_QUIETLY TRUE)
    +ENDIF (SSL_INCLUDE_DIR AND SSL_LIBRARIES)
    +
    +FIND_PATH(SSL_INCLUDE_DIR openssl/opensslv.h)
    +
    +FIND_LIBRARY(SSL_LIBRARIES crypto)
    +
    +INCLUDE(FindPackageHandleStandardArgs)
    +
    +# handle the QUIETLY and REQUIRED arguments and set SSL_FOUND to TRUE if
    +# all listed variables are TRUE
    +FIND_PACKAGE_HANDLE_STANDARD_ARGS(SSL DEFAULT_MSG SSL_LIBRARIES SSL_INCLUDE_DIR)
    +
    +MARK_AS_ADVANCED(SSL_INCLUDE_DIR SSL_LIBRARIES)
    --- End diff --
    
    Can we leverage the code for ssl and curl in configure?
    By the way, now we should auto-enable with-openssl if with-libhdfs3 is enabled.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support TDE wri...

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

    https://github.com/apache/incubator-hawq/pull/1265
  
    Please unify the indent. We should avoid use both "space" and "tab" for indent in one source file. 


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

    https://github.com/apache/incubator-hawq/pull/1265#discussion_r126856385
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::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 = map.get<std::string>("material");
    +
    +	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(), '_', '/');
    --- End diff --
    
    Can you give the comment for why replacing them?


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126904979
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::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 = map.get<std::string>("material");
    --- End diff --
    
    need a try catch here? when map doesn't has a material field.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support TDE wri...

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

    https://github.com/apache/incubator-hawq/pull/1265
  
    Only support TDE write function on single data node. Will support multiple data nodes in the next pull request.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126905189
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::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 = map.get<std::string>("material");
    +
    +	int rem = key.length() % 4;
    +	if (rem) {
    +		rem = 4 - rem;
    +		while (rem != 0) {
    +			key = key + "=";
    +			rem--;
    +		}
    +	}
    --- End diff --
    
    maybe we should refer the hadoop-kms code to find the answer...


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126638288
  
    --- Diff: depends/libhdfs3/src/client/HttpClient.cpp ---
    @@ -0,0 +1,337 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "HttpClient.h"
    +#include "Logger.h"
    +
    +using namespace Hdfs::Internal;
    +
    +namespace Hdfs {
    +
    +#define CURL_SETOPT(handle, option, optarg, fmt, ...) \
    +    res = curl_easy_setopt(handle, option, optarg); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, ##__VA_ARGS__); \
    +    }
    +
    +#define CURL_SETOPT_ERROR1(handle, option, optarg, fmt) \
    +    CURL_SETOPT(handle, option, optarg, fmt, curl_easy_strerror(res));
    +
    +#define CURL_SETOPT_ERROR2(handle, option, optarg, fmt) \
    +    CURL_SETOPT(handle, option, optarg, fmt, curl_easy_strerror(res), \
    +        errorString().c_str())
    +
    +#define CURL_PERFORM(handle, fmt) \
    +    res = curl_easy_perform(handle); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, curl_easy_strerror(res), errorString().c_str()); \
    +    }
    +
    +
    +#define CURL_GETOPT_ERROR2(handle, option, optarg, fmt) \
    +    res = curl_easy_getinfo(handle, option, optarg); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, curl_easy_strerror(res), errorString().c_str()); \
    +    }
    +
    +#define CURL_GET_RESPONSE(handle, code, fmt) \
    +    CURL_GETOPT_ERROR2(handle, CURLINFO_RESPONSE_CODE, code, fmt);
    +
    +HttpClient::HttpClient() : curl(NULL), list(NULL) {
    +
    +}
    +
    +/**
    + * Construct a HttpClient instance.
    + * @param url a url which is the address to send the request to the corresponding http server.
    + */
    +HttpClient::HttpClient(const std::string &url) {
    +	curl = NULL;
    +	list = NULL;
    +	this->url = url;
    +}
    +
    +/**
    + * Destroy a HttpClient instance.
    + */
    +HttpClient::~HttpClient()
    +{
    +	destroy();
    +}
    +
    +/**
    + * Receive error string from curl.
    + */
    +std::string HttpClient::errorString() {
    +	if (strlen(errbuf) == 0)
    +		return "";
    +	return errbuf;
    +}
    +
    +/**
    + * Curl call back function to receive the reponse messages.
    + * @return return the size of reponse messages. 
    + */
    +size_t HttpClient::CurlWriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp)
    +{
    +      size_t realsize = size * nmemb;
    +	  if (userp == NULL || contents == NULL) {
    +		  return 0;
    +	  }
    +      ((std::string *)userp)->append((const char *)contents, realsize);
    +	  LOG(DEBUG2, "HttpClient : Http response is : %s", ((std::string *)userp)->c_str());
    +      return realsize;
    +}
    +
    +/**
    + * Init curl handler and set curl options.
    + */
    +void HttpClient::init() {
    +	if (!initialized)
    +	{
    +		initialized = true;
    +		if (curl_global_init(CURL_GLOBAL_ALL)) {
    +			THROW(HdfsIOException, "Cannot initialize curl client for KMS");
    +		}
    +	}
    +
    +	curl = curl_easy_init();
    +	if (!curl) {
    +		THROW(HdfsIOException, "Cannot initialize curl handle for KMS");
    +	}
    +	
    +    CURL_SETOPT_ERROR1(curl, CURLOPT_ERRORBUFFER, errbuf,
    +        "Cannot initialize curl error buffer for KMS: %s");
    +
    +    errbuf[0] = 0;
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_NOPROGRESS, 1,
    +        "Cannot initialize no progress in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_VERBOSE, 0,
    +        "Cannot initialize no verbose in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_COOKIEFILE, "",
    +        "Cannot initialize cookie behavior in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_HTTPHEADER, list,
    +        "Cannot initialize headers in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_WRITEFUNCTION, HttpClient::CurlWriteMemoryCallback,
    +        "Cannot initialize body reader in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_WRITEDATA, (void *)&response,
    +        "Cannot initialize body reader data in HttpClient: %s: %s");
    +
    +    /* Some servers don't like requests that are made without a user-agent
    +        field, so we provide one */
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0",
    +        "Cannot initialize user agent in HttpClient: %s: %s");
    +	list = NULL;
    +
    +}
    +
    +/**
    + * Do clean up for curl.
    + */
    +void HttpClient::destroy() {
    +	if (curl) {
    +		curl_easy_cleanup(curl);
    +	}
    +	if (list) {
    +		curl_slist_free_all(list);
    +	}
    +}
    +
    +/**
    + * Set url for http client.
    + */
    +void HttpClient::setURL(const std::string &url) {
    +	this->url = url;
    +}
    +
    +/**
    + * Set retry times for http request which can be configured in config file.
    + */
    +void HttpClient::setRequestRetryTimes(int request_retry_times) {
    +	if (request_retry_times < 0) {
    +		THROW(InvalidParameter, "HttpClient : Invalid value for request_retry_times.");
    +	}
    +	this->request_retry_times = request_retry_times;
    +}
    +
    +/**
    + * Set request timeout which can be configured in config file.
    + */
    +void HttpClient::setRequestTimeout(int64_t curl_timeout) {
    +	if (curl_timeout < 0) {
    +		THROW(InvalidParameter, "HttpClient : Invalid value for curl_timeout.");
    +	}
    +	this->curl_timeout = curl_timeout;
    +}
    +
    +/**
    + * Set headers for http client.
    + */
    +void HttpClient::setHeaders(const std::vector<std::string> &headers) {
    +	if (!headers.empty()) {
    +		this->headers = headers;
    +		for (std::string header : headers) {
    +			list = curl_slist_append(list, header.c_str());
    +			if (!list) {
    +				THROW(HdfsIOException, "Cannot add header in HttpClient.");
    +			}
    +		}	
    +	}
    +	else {
    +		LOG(DEBUG1, "HttpClient : Header is empty.");				
    +	}
    +}
    +
    +
    +/**
    + * Set body for http client.
    + */
    +void HttpClient::setBody(const std::string &body) {
    +	this->body = body;
    +}
    +
    +/**
    + * Set expected response code.
    + */
    +void HttpClient::setExpectedResponseCode(int64_t response_code_ok) {
    +	this->response_code_ok = response_code_ok;
    +}
    +
    +/**
    + * Http common method to get response info by sending request to http server.
    + * @param method : define different http methods.
    + * @return return response info.
    + */
    +std::string HttpClient::httpCommon(httpMethod method) {
    +	
    +    /* Set headers and url. */
    +	if (list != NULL) {
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_HTTPHEADER, list,
    +        	        "Cannot initialize headers in HttpClient: %s: %s");
    +	} else {
    +		LOG(DEBUG1, "HttpClient : Http Header is NULL");
    +	}
    +
    +	if (curl != NULL) {
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_URL, url.c_str(),
    +    	        "Cannot initialize url in HttpClient: %s: %s");
    +	} else {
    +		LOG(DEBUG1, "HttpClient : Http URL is NULL");
    +	}
    +
    +	/* Set body based on different http method. */ 
    +	switch(method) {
    +		case HTTP_GET:
    +			break;
    +		case HTTP_POST:
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_COPYPOSTFIELDS, body.c_str(),
    +                "Cannot initialize post data in HttpClient: %s: %s");
    +			break;
    +		case HTTP_DELETE:
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_CUSTOMREQUEST, "DELETE",
    +                "Cannot initialize set customer request in HttpClient: %s: %s");
    +			break;
    +		case HTTP_PUT:
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_CUSTOMREQUEST, "PUT",
    +                "Cannot initialize set customer request in HttpClient: %s: %s");
    +			CURL_SETOPT_ERROR2(curl, CURLOPT_COPYPOSTFIELDS, body.c_str(),
    +                "Cannot initialize post data in HttpClient: %s: %s");
    +			break;
    +	}
    +
    +	/* Do several http request try according to request_retry_times until got the right reponse code. */	
    +	int64_t response_code = -1;	
    +
    +	while (request_retry_times >= 0 && response_code != response_code_ok) {
    +		request_retry_times -= 1;
    +		response = "";
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_TIMEOUT, curl_timeout, 
    +					"Send request to http server timeout: %s: %s");
    +		CURL_PERFORM(curl, "Could not send request in HttpClient: %s %s");
    +		CURL_GET_RESPONSE(curl, &response_code,
    +					"Cannot get response code in HttpClient: %s: %s");
    +	}
    +	LOG(DEBUG1, "HttpClient : The http method is %d. The http url is %s. The http response is %s.", method, url.c_str(), response.c_str());
    +	return response;
    +}
    +
    +/**
    + * Http GET method.
    + */
    +std::string HttpClient::get() {
    +	httpMethod method = HTTP_GET;
    --- End diff --
    
    temp variable "method" is redundant ?


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126864478
  
    --- Diff: depends/libhdfs3/src/client/KmsClientProvider.cpp ---
    @@ -0,0 +1,318 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "KmsClientProvider.h"
    +#include "Logger.h"
    +#include <gsasl.h>
    +#include <map>
    +#include <boost/property_tree/json_parser.hpp>
    +using namespace Hdfs::Internal;
    +using boost::property_tree::read_json;
    +using boost::property_tree::write_json;
    +
    +namespace Hdfs {
    +
    +/**
    + * Convert ptree format to json format
    + */
    +std::string KmsClientProvider::toJson(const ptree &data)
    +{
    +	std::ostringstream buf;
    +	try {
    +		write_json(buf, data, false);
    +		std::string json = buf.str();
    +		return json;
    +	} catch (...) {
    +		THROW(HdfsIOException, "KmsClientProvider : Write json failed.");
    +	}	
    +}
    +
    +/**
    + * Convert json format to ptree format
    + */
    +ptree KmsClientProvider::fromJson(const std::string &data)
    +{
    +	ptree pt2;
    +	try {
    +		std::istringstream is(data);
    +		read_json(is, pt2);
    +		return pt2;
    +	} catch (...) {
    +		THROW(HdfsIOException, "KmsClientProvider : Read json failed.");
    +	}
    +}
    +
    +/**
    + * Encode string to base64. 
    + */
    +std::string	KmsClientProvider::base64Encode(const std::string &data)
    +{
    +	char * buffer = NULL;
    +	size_t len = 0;
    +	int rc = 0;
    +	std::string result;
    +
    +	LOG(DEBUG1, "KmsClientProvider : Encode data is %s", data.c_str());
    +
    +	if (GSASL_OK != (rc = gsasl_base64_to(data.c_str(), data.size(), &buffer, &len))) {
    --- End diff --
    
    Must use `data.data()` instead of `data.c_str()`, please check whether the similar issues existed in other place.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r127135764
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::shared_ptr<KmsClientProvider> kcp, int32_t bufSize) : encryptionInfo(encryptionInfo), kcp(kcp), bufSize(bufSize)
    --- End diff --
    
    Should use `shared_ptr` instead of `std::shared_ptr`, because libhdfs have two implements of shared_ptr. 
    See: depends/libhdfs3/src/common/Memory.h


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126637139
  
    --- Diff: depends/libhdfs3/src/client/HttpClient.cpp ---
    @@ -0,0 +1,337 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "HttpClient.h"
    +#include "Logger.h"
    +
    +using namespace Hdfs::Internal;
    +
    +namespace Hdfs {
    +
    +#define CURL_SETOPT(handle, option, optarg, fmt, ...) \
    +    res = curl_easy_setopt(handle, option, optarg); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, ##__VA_ARGS__); \
    +    }
    +
    +#define CURL_SETOPT_ERROR1(handle, option, optarg, fmt) \
    +    CURL_SETOPT(handle, option, optarg, fmt, curl_easy_strerror(res));
    +
    +#define CURL_SETOPT_ERROR2(handle, option, optarg, fmt) \
    +    CURL_SETOPT(handle, option, optarg, fmt, curl_easy_strerror(res), \
    +        errorString().c_str())
    +
    +#define CURL_PERFORM(handle, fmt) \
    +    res = curl_easy_perform(handle); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, curl_easy_strerror(res), errorString().c_str()); \
    +    }
    +
    +
    +#define CURL_GETOPT_ERROR2(handle, option, optarg, fmt) \
    +    res = curl_easy_getinfo(handle, option, optarg); \
    +    if (res != CURLE_OK) { \
    +        THROW(HdfsIOException, fmt, curl_easy_strerror(res), errorString().c_str()); \
    +    }
    +
    +#define CURL_GET_RESPONSE(handle, code, fmt) \
    +    CURL_GETOPT_ERROR2(handle, CURLINFO_RESPONSE_CODE, code, fmt);
    +
    +HttpClient::HttpClient() : curl(NULL), list(NULL) {
    +
    +}
    +
    +/**
    + * Construct a HttpClient instance.
    + * @param url a url which is the address to send the request to the corresponding http server.
    + */
    +HttpClient::HttpClient(const std::string &url) {
    +	curl = NULL;
    +	list = NULL;
    +	this->url = url;
    +}
    +
    +/**
    + * Destroy a HttpClient instance.
    + */
    +HttpClient::~HttpClient()
    +{
    +	destroy();
    +}
    +
    +/**
    + * Receive error string from curl.
    + */
    +std::string HttpClient::errorString() {
    +	if (strlen(errbuf) == 0)
    +		return "";
    +	return errbuf;
    +}
    +
    +/**
    + * Curl call back function to receive the reponse messages.
    + * @return return the size of reponse messages. 
    + */
    +size_t HttpClient::CurlWriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp)
    +{
    +      size_t realsize = size * nmemb;
    +	  if (userp == NULL || contents == NULL) {
    +		  return 0;
    +	  }
    +      ((std::string *)userp)->append((const char *)contents, realsize);
    +	  LOG(DEBUG2, "HttpClient : Http response is : %s", ((std::string *)userp)->c_str());
    +      return realsize;
    +}
    +
    +/**
    + * Init curl handler and set curl options.
    + */
    +void HttpClient::init() {
    +	if (!initialized)
    +	{
    +		initialized = true;
    +		if (curl_global_init(CURL_GLOBAL_ALL)) {
    +			THROW(HdfsIOException, "Cannot initialize curl client for KMS");
    +		}
    +	}
    +
    +	curl = curl_easy_init();
    +	if (!curl) {
    +		THROW(HdfsIOException, "Cannot initialize curl handle for KMS");
    +	}
    +	
    +    CURL_SETOPT_ERROR1(curl, CURLOPT_ERRORBUFFER, errbuf,
    +        "Cannot initialize curl error buffer for KMS: %s");
    +
    +    errbuf[0] = 0;
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_NOPROGRESS, 1,
    +        "Cannot initialize no progress in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_VERBOSE, 0,
    +        "Cannot initialize no verbose in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_COOKIEFILE, "",
    +        "Cannot initialize cookie behavior in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_HTTPHEADER, list,
    +        "Cannot initialize headers in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_WRITEFUNCTION, HttpClient::CurlWriteMemoryCallback,
    +        "Cannot initialize body reader in HttpClient: %s: %s");
    +
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_WRITEDATA, (void *)&response,
    +        "Cannot initialize body reader data in HttpClient: %s: %s");
    +
    +    /* Some servers don't like requests that are made without a user-agent
    +        field, so we provide one */
    +    CURL_SETOPT_ERROR2(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0",
    +        "Cannot initialize user agent in HttpClient: %s: %s");
    +	list = NULL;
    +
    +}
    +
    +/**
    + * Do clean up for curl.
    + */
    +void HttpClient::destroy() {
    +	if (curl) {
    +		curl_easy_cleanup(curl);
    +	}
    +	if (list) {
    +		curl_slist_free_all(list);
    +	}
    +}
    +
    +/**
    + * Set url for http client.
    + */
    +void HttpClient::setURL(const std::string &url) {
    +	this->url = url;
    +}
    +
    +/**
    + * Set retry times for http request which can be configured in config file.
    + */
    +void HttpClient::setRequestRetryTimes(int request_retry_times) {
    +	if (request_retry_times < 0) {
    +		THROW(InvalidParameter, "HttpClient : Invalid value for request_retry_times.");
    +	}
    +	this->request_retry_times = request_retry_times;
    +}
    +
    +/**
    + * Set request timeout which can be configured in config file.
    + */
    +void HttpClient::setRequestTimeout(int64_t curl_timeout) {
    +	if (curl_timeout < 0) {
    +		THROW(InvalidParameter, "HttpClient : Invalid value for curl_timeout.");
    +	}
    +	this->curl_timeout = curl_timeout;
    +}
    +
    +/**
    + * Set headers for http client.
    + */
    +void HttpClient::setHeaders(const std::vector<std::string> &headers) {
    +	if (!headers.empty()) {
    +		this->headers = headers;
    +		for (std::string header : headers) {
    +			list = curl_slist_append(list, header.c_str());
    +			if (!list) {
    +				THROW(HdfsIOException, "Cannot add header in HttpClient.");
    +			}
    +		}	
    +	}
    +	else {
    +		LOG(DEBUG1, "HttpClient : Header is empty.");				
    +	}
    +}
    +
    +
    +/**
    + * Set body for http client.
    + */
    +void HttpClient::setBody(const std::string &body) {
    +	this->body = body;
    +}
    +
    +/**
    + * Set expected response code.
    + */
    +void HttpClient::setExpectedResponseCode(int64_t response_code_ok) {
    +	this->response_code_ok = response_code_ok;
    +}
    +
    +/**
    + * Http common method to get response info by sending request to http server.
    + * @param method : define different http methods.
    + * @return return response info.
    + */
    +std::string HttpClient::httpCommon(httpMethod method) {
    +	
    +    /* Set headers and url. */
    +	if (list != NULL) {
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_HTTPHEADER, list,
    +        	        "Cannot initialize headers in HttpClient: %s: %s");
    +	} else {
    +		LOG(DEBUG1, "HttpClient : Http Header is NULL");
    +	}
    +
    +	if (curl != NULL) {
    +		CURL_SETOPT_ERROR2(curl, CURLOPT_URL, url.c_str(),
    +    	        "Cannot initialize url in HttpClient: %s: %s");
    +	} else {
    +		LOG(DEBUG1, "HttpClient : Http URL is NULL");
    --- End diff --
    
    It should be error if URL is NULL


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

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


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support TDE wri...

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

    https://github.com/apache/incubator-hawq/pull/1265
  
    Great job. 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.
---

[GitHub] incubator-hawq pull request #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

    https://github.com/apache/incubator-hawq/pull/1265#discussion_r127118606
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::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 = map.get<std::string>("material");
    +
    +	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, "material is :%s", key.c_str());
    --- End diff --
    
    Suggest provide more clear log message, and if this function is called frequently, use DEBUG3 instead of INFO. 


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126904920
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::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 = map.get<std::string>("material");
    --- End diff --
    
    need a try catch here? when map doesn't has a material field.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r126862923
  
    --- Diff: depends/libhdfs3/mock/MockHttpClient.h ---
    @@ -0,0 +1,53 @@
    +/********************************************************************
    + * 2014 - 
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +#ifndef _HDFS_LIBHDFS3_MOCK_HTTPCLIENT_H_
    +#define _HDFS_LIBHDFS3_MOCK_HTTPCLIENT_H_
    +
    +#include "gmock/gmock.h"
    +
    +#include "client/HttpClient.h"
    +#include "client/KmsClientProvider.h"
    +#include <boost/property_tree/ptree.hpp>
    +
    +using boost::property_tree::ptree;
    +
    +class MockHttpClient: public Hdfs::HttpClient {
    --- End diff --
    
    I think we should add a unit test for HttpClient, because libcurl has many tricky behaviors, maybe we forgot some exceptional path. 
    Just a reminder, let's do it in future.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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

    https://github.com/apache/incubator-hawq/pull/1265#discussion_r126856563
  
    --- Diff: depends/libhdfs3/src/client/CryptoCodec.cpp ---
    @@ -0,0 +1,163 @@
    +/********************************************************************
    + * 2014 -
    + * open source under Apache License Version 2.0
    + ********************************************************************/
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CryptoCodec.h"
    +#include "Logger.h"
    +
    +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, std::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 = map.get<std::string>("material");
    +
    +	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, "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:%ld is_encode:%b", 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 {
    +		cipher = EVP_aes_192_ctr();
    +	}
    --- End diff --
    
    If key.length() is out of range, should we throw an expectation?


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support ...

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/1265#discussion_r128675385
  
    --- Diff: depends/libhdfs3/src/client/OutputStreamImpl.cpp ---
    @@ -278,7 +316,10 @@ void OutputStreamImpl::append(const char * buf, int64_t size) {
     
     void OutputStreamImpl::appendInternal(const char * buf, int64_t size) {
         int64_t todo = size;
    +    if (fileStatus.isFileEncrypted()) {
    +        buf = cryptoCodec->encode(buf, size).c_str();
    --- End diff --
    
    encode() reture a **temporary** string object, so buf point a uncertain memory.
    MUST fix it.


---
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 #1265: HAWQ-1500. HAWQ-1501. HAWQ-1502. Support TDE wri...

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

    https://github.com/apache/incubator-hawq/pull/1265
  
    @linwen @interma  Please feel free to review this pr. Thank you.


---
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.
---