patternMinor
VB.NET code to AES encrypt and decrypt
Viewed 0 times
aesanddecryptnetcodeencrypt
Problem
I wrote this for use internally at work and have never worked with encrypted data before, so can you please critique? It's written in VB.NET which I have decent experience in, just not the encryption portion of it.
I have written a simple internal messaging client. Basically a home grown email program but it uses file saves over the local network and never goes online. It also writes all data after encryption only so nothing is ever stored somewhere decrypted except volatile RAM. We don't want our users to have access to outside communication and we want it all encrypted due to HIPAA/PCI. We're a small collections company and want to inexpensively make communication easier internally without opening large risk or letting people get to the outside internet.
I'm storing this as XML data. I have one config file (also encrypted) which contains all the settings and user info. So name and password and userID, etc. When they try to log in, I decrypt the config file and look at the XML to see if their password is right. If it is, I let them into the program and decrypt their individual inbox file which is also XML encrypted with this same routine.
I have one 32 byte key that is saved into my source code and not shared with anyone else in the world but me. And then each file gets a unique IV every time its encrypted or re-encrypted and that IV is also saved to the file along with the encrypted data. I'm saving the bytes to the file as hexadecimal.
Here's the module the handles reading/writing files:
`Imports System.Security.Cryptography
Imports System.IO
Imports System.Text
Module encryption
Public Sub TESTING_in_encryption()
End Sub
Public Function ObfuscateString(str As String) As String
'NOT ENCRYPTION! Just stops casual observers from reading the plain text
Return System.Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes(str))
End Function
Public Function DeObfuscateString(str As String) As String
'NOT ENCRYPTIO
I have written a simple internal messaging client. Basically a home grown email program but it uses file saves over the local network and never goes online. It also writes all data after encryption only so nothing is ever stored somewhere decrypted except volatile RAM. We don't want our users to have access to outside communication and we want it all encrypted due to HIPAA/PCI. We're a small collections company and want to inexpensively make communication easier internally without opening large risk or letting people get to the outside internet.
I'm storing this as XML data. I have one config file (also encrypted) which contains all the settings and user info. So name and password and userID, etc. When they try to log in, I decrypt the config file and look at the XML to see if their password is right. If it is, I let them into the program and decrypt their individual inbox file which is also XML encrypted with this same routine.
I have one 32 byte key that is saved into my source code and not shared with anyone else in the world but me. And then each file gets a unique IV every time its encrypted or re-encrypted and that IV is also saved to the file along with the encrypted data. I'm saving the bytes to the file as hexadecimal.
Here's the module the handles reading/writing files:
`Imports System.Security.Cryptography
Imports System.IO
Imports System.Text
Module encryption
Public Sub TESTING_in_encryption()
End Sub
Public Function ObfuscateString(str As String) As String
'NOT ENCRYPTION! Just stops casual observers from reading the plain text
Return System.Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes(str))
End Function
Public Function DeObfuscateString(str As String) As String
'NOT ENCRYPTIO
Solution
First of all, you have an empty Sub definition:
That code is doing nothing except making it look like you forgot to implement something. Remove it.
Second, obfuscation is by definition not encryption. You name your method
If you feel you do need that comment, you should create a doc comment for the method and mention it there so callers of the function know to not use it for encryption as well as maintainers.
Third, you have dead code:
If you don't need this code, remove it. How are we to know why it is there, and what you originally had it there for? Also, if you use source control (which you totally should), you can see the history of each file, and which commits changed it, so you can safely remove it knowing you can get it back later if you need to.
Public Sub TESTING_in_encryption()
End Sub
That code is doing nothing except making it look like you forgot to implement something. Remove it.
Second, obfuscation is by definition not encryption. You name your method
ObfuscateString(string), so you don't need this comment:'NOT ENCRYPTION! Just stops casual observers from reading the plain text
If you feel you do need that comment, you should create a doc comment for the method and mention it there so callers of the function know to not use it for encryption as well as maintainers.
Third, you have dead code:
'For Each b As Byte In aesAlg.IV
' sb.Append(BitConverter.ToString({b}))
'Next
'For Each b As Byte In msEncrypt.ToArray
' sb.Append(b.ToString("000"))
'Next
If you don't need this code, remove it. How are we to know why it is there, and what you originally had it there for? Also, if you use source control (which you totally should), you can see the history of each file, and which commits changed it, so you can safely remove it knowing you can get it back later if you need to.
Context
StackExchange Code Review Q#112509, answer score: 7
Revisions (0)
No revisions yet.