Change aes key to be generated from user provided passcode#27
Conversation
| ''' | ||
| self.salt = os.urandom(16) #Salt variable (Generates a random byte string) | ||
| self.key = PBKDF2("passphrase", self.salt).read(16) #Creates key using KDF scheme | ||
| self.salt = "Embedded2" |
There was a problem hiding this comment.
do we really need to save this as a variable? its only used in the constructor
| @@ -3,11 +3,11 @@ | |||
| from typing import List, Tuple | |||
|
|
|||
| class Encryptor(object): | |||
There was a problem hiding this comment.
I think this class is backwards. Why do we need to re-encapsulate AESEncryptor with Encryptor? I think it would be much better for AESEncryptor to make all encryptors just implement encryptFace and encryptFrame themselves. This will then resolve the weird import statement from src.jetson.AES import Encryption as AESEncryptor which is very misleading.
stephend017
left a comment
stephend017
left a comment
There was a problem hiding this comment.
So after reviewing this we have a few design flaws. essentially the problem is we are writing the generic class Encryptor at the outer level when we should be writing it at the inner level.
|
@stephend017 Hey Stephen, sorry to address this so late. With school starting and everybody moving to a new team, working on this project has become lower priority and I haven't had time to remove the AES wrapper I've implemented. Would it be alright to merge as-is and address this when the project is picked back up again? |
|
I don't think we should merge it if the project isn't being actively worked on right now. since it's not a critical bug fix there's really no reason to create a bigger mess for whoever picks it up in the future. They'll have this PR and all of its comments as a starting point |
2 participants
No description provided.