Skip to content

Rewrite aes encryption #257 - #264

Merged
imay merged 1 commit into
apache:masterfrom
kangpinghuang:master
Nov 2, 2018
Merged

Rewrite aes encryption #257#264
imay merged 1 commit into
apache:masterfrom
kangpinghuang:master

Conversation

@kangpinghuang

Copy link
Copy Markdown
Contributor

No description provided.

Comment threadbe/src/aes/aes.cpp Outdated
@@ -0,0 +1,171 @@
// Copyright (c) 2018, Baidu.com, Inc. All Rights Reserved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use ASF's header

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment threadbe/CMakeLists.txt Outdated
add_subdirectory(${TEST_DIR}/exprs)
add_subdirectory(${TEST_DIR}/runtime)
add_subdirectory(${TEST_DIR}/http)
add_subdirectory(${TEST_DIR}/aes)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need this 'aes' directory

you can put aes.h and aes.cpp to be/src/util, and rename these to aes_util.h??

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment threadbe/src/aes/aes.cpp Outdated
}
}

int do_decrypt(EVP_CIPHER_CTX* aes_ctx, const EVP_CIPHER* cipher,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function should be static

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@kangpinghuang
kangpinghuangforce-pushed the master branch 3 times, most recently from 1b04f7b to c5a9a59CompareNovember 1, 2018 12:14
Comment threadbe/src/util/aes_util.cpp Outdated
unsigned char encrypt_key[AES_MAX_KEY_LENGTH / 8];
aes_create_key(key, key_length, encrypt_key, mode);

if (!cipher || (EVP_CIPHER_iv_length(cipher) > 0 && !iv)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when compare pointer use "cipher != nullptr", compare integer use "ret != 0", check bool use "!is_success"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

aes_create_key(key, key_length, encrypt_key, mode);

if (!cipher || (EVP_CIPHER_iv_length(cipher) > 0 && !iv)) {
return AES_BAD_DATA;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when cipher is not null, cipher's memory leak

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cipher can not be delete by manually or it will core dump

Comment threadbe/src/util/aes_util.cpp Outdated

const EVP_CIPHER* get_evp_type(const AesMode mode) {
switch (mode) {
case AES_128_ECB:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the indent is not right, 'case" align with switch, no need to indent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@kangpinghuang
kangpinghuangforce-pushed the master branch 3 times, most recently from 144219d to 6b97f89CompareNovember 2, 2018 02:18
@kangpinghuangkangpinghuang changed the title Update #257: rewrite aes encryptionRewrite aes encryption #257Nov 2, 2018
imay
imay previously approved these changes Nov 2, 2018

@imayimay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

imay
imay approved these changes Nov 2, 2018
@imay
imay merged commit d57e91d into apache:masterNov 2, 2018
hubgeter pushed a commit to hubgeter/doris that referenced this pull request Jan 15, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@kangpinghuang@imay