HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

VB.NET code to AES encrypt and decrypt

Submitted by: @import:stackexchange-codereview··
0
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

Solution

First of all, you have an empty Sub definition:

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.