-
Notifications
You must be signed in to change notification settings - Fork 0
Address review comments #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Address review comments #6
Conversation
| // counter field. | ||
| // The first 31 bits of the initial counter field are set to 0, the last bit | ||
| // is set | ||
| // to 1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it necessary to break the line twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that. will fix it. Thanks
|
|
||
| // Update last two bytes with new page ordinal (instead of creating new page AAD | ||
| // from | ||
| // scratch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 line breaks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that. will fix it. Thanks
| constexpr int8_t kOffsetIndex = 7; | ||
|
|
||
| /// Do AES_GCM_CTR_V1 or AES_GCM_V1 encryption | ||
| class AesEncryptor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change comment to "Performs AES encryption operations with GCM or CTR ciphers"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| ~AesEncryptor(); | ||
|
|
||
| /// Size different between plaintext and ciphertext, for this cipher. | ||
| int CiphertextSizeDelta(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
size differenCe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, Wes suggested calling this function "ciphertext_size_delta()", " (since this is just accessing an attribute)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this function in AesEncryptor::AesEncryptorImpl (it's accessing an attribute in this class)
| /// Size different between plaintext and ciphertext, for this cipher. | ||
| int CiphertextSizeDelta(); | ||
|
|
||
| /// Key length is passed only for validation. If different from value in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please start comment with "Encrypts plaintext with the key and aad. "
| int Encrypt(const uint8_t* plaintext, int plaintext_len, uint8_t* key, int key_len, | ||
| uint8_t* aad, int aad_len, uint8_t* ciphertext); | ||
|
|
||
| int SignedFooterEncrypt(const uint8_t* footer, int footer_len, uint8_t* key, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comment "/// Encrypts plaintext footer, in order to compute the signature (tag)".
| std::unique_ptr<AesEncryptorImpl> impl_; | ||
| }; | ||
|
|
||
| /// Do AES_GCM_CTR_V1 or AES_GCM_V1 decryption |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change comment to "Performs AES decryption operations with GCM or CTR ciphers"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| /// Size different between plaintext and ciphertext, for this cipher. | ||
| int CiphertextSizeDelta(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
differenCe. also, Wes suggested calling this function "ciphertext_size_delta()", " (since this is just accessing an attribute)"
| int CiphertextSizeDelta(); | ||
|
|
||
| /// Key length is passed only for validation. If different from value in | ||
| /// constructor, exception will be thrown. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please start comment with "Decrypts ciphertext using the key and aad. "
No description provided.