(verschlüsselte) Dateiübertragung via TCP

Guten Abend zusammen 🙂

zunächst einmal möchte ich als Hintergrund etwas zu mir sagen:
Ich bin eigentlich Ingenieur aus dem Bereich Berechnung. Hierfür schreibe ich oft kleinere Scripts (in den unterschiedlichsten Sprachen), die in der Regel einfach nur funktionieren müssen und ansonsten keine besonderen Anforderungen haben.

Das heißt mein Programmierstil ist gezeichnet von quick'n'dirty. Sicherheit (gar nicht) und Effizienz (höchstens auf eine einzelne Methode bezogen) haben für mich hierbei normalerweise wenig Relevanz.

Da ich mich aber nun an ein etwas größeres Projekt wagen möchte, muss ich mich vorher also zwangsweise etwas professionalisieren. Daher würde ich euch gerne etwas Code vorstellen und hoffe auf Feedback von euch.

In meinem Projekt wird Datenübertragung, die wenn möglich verschlüsselt werden soll, einen wichtigen Part einnehmen.
Zu Beginn habe ich mir daher ein TCP-System mit Client und Listener gebastelt (kann ich bei Bedarf dazu poste, sollte aber nicht nötig sein denke ich). Dabei schickt der Client Nachrichten als strings an den Listener welcher diese direkt an den Client zurückschickt (in Zukunft soll dies natürlich ein weiterer Client sein).

Auf diesem Wege würde ich nun gerne Dateien übertragen. Dazu habe ich mir eine Klasse "DataTransferEngine" gebaut.
Diese hat bietet dem Programm im Prinzip zwei Funktionien:

1. Es erstellt auf Grundlage eines Dateipfades einen string der einige Informationen zu der Datei/dem Absender und die Datei selber in einem base64string enthält
2. Es erstellt eine Klasse ReceivedFile" welche den string aus 1 wieder zurückbauen kann, die entsprechende Datei auf einen Dateipfad kopiert und noch einige Informationen zur Übertragung enthält
   Der string wird jeweils noch mittels TripleDES zerhackt bzw. wieder zusammengesetzt.

Das ganze funktioniert soweit auch ganz gut (Bilder, Textdaten, etc. sind kein Problem) selbst etwas größere Musikdateien werden noch problemlos übertragen.

Meine Fragen an euch wären nun, ob ihr hier grobe Patzer im Programmierstil seht, ob meine Art der Verschlüsselung überhaupt Sinn macht (also nicht mal mittelaufwändigen Methoden standhält) und natürlich generell, ob diese Art der Datenübertragung überhaupt praktikabel ist (große Dateien funktionieren z.B. gar nicht) und was hier bessere Ansätze wären.

Darüber hinaus wäre ich natürlich über jeden Artikel, Post dankbar der hierzu Informationen bietet 🙂

P.s.: Über das Zyan Framework bin ich bereits gestolpert und das klingt schonmal sehr nah an meinen Vorstellungen, leider habe ich hier noch keine Zeit gehabt mich dort weiter einzulesen

P.p.s: Ich habe den Code jetzt nicht weiter kommentiert, da ich denke dass dieser relativ übersichtlich formatiert ist, sollte eine Kommentierung gewünscht sein kann ich diese natürlich gerne nachreichen

    class DataTransferEngine
    {
        private const char Separator = ((char)007);

        public class ReceivedFile
        {
            public readonly string Sender;
            public readonly DateTime SendTime;
            public readonly DateTime ReceiveTime;
            public readonly double SendDuration;
            public readonly FileInfo FileData;
            public readonly bool TransferSuccessful;

            public ReceivedFile(string input, string path)
            {
                try
                {
                    string[] receivedParts = Decrypt(input).Split(Separator);

                    Sender = receivedParts[0];
                    SendTime = DateTime.FromBinary(long.Parse(receivedParts[1]));
                    ReceiveTime = DateTime.Now;
                    SendDuration = Math.Round((ReceiveTime - SendTime).TotalMilliseconds, 2);

                    byte[] bytes = Convert.FromBase64String(receivedParts[3]);


                    File.WriteAllBytes(path + "\\" + receivedParts[2], bytes);
                    FileData = new FileInfo(path + "\\" + receivedParts[2]);

                    TransferSuccessful = true;
                }
                catch (Exception)
                {
                    TransferSuccessful = false;
                }
            }
        }

        public static string SendString(string file)
        {
            if (File.Exists(file))
            {
                FileInfo fileInfo = new FileInfo(file);

                FileStream stream = File.OpenRead(file);
                byte[] fileBytes = new byte[stream.Length];
                stream.Read(fileBytes, 0, fileBytes.Length);
                stream.Close();

                string base64FileData = Convert.ToBase64String(fileBytes);
                string sendstring = Encrypt(Environment.UserName + Separator + DateTime.Now.ToBinary().ToString() + Separator + fileInfo.Name + Separator + base64FileData);

                return sendstring;
            }
            else
                return null;
        }

        private static string Encrypt(string toEncrypt)
        {
            byte[] keyArray;
            byte[] toEncryptArray = UTF8Encoding.UTF8.GetBytes(toEncrypt);

            AppSettingsReader settingsReader = new AppSettingsReader();
            string key = (string)settingsReader.GetValue("SecurityKey", typeof(String));

            MD5CryptoServiceProvider hashmd5 = new MD5CryptoServiceProvider();
            keyArray = hashmd5.ComputeHash(UTF8Encoding.UTF8.GetBytes(key));
            hashmd5.Clear();

            TripleDESCryptoServiceProvider tdes = new TripleDESCryptoServiceProvider();
            tdes.Key = keyArray;
            tdes.Mode = CipherMode.ECB;
            tdes.Padding = PaddingMode.PKCS7;

            ICryptoTransform cTransform = tdes.CreateEncryptor();
            byte[] resultArray = cTransform.TransformFinalBlock(toEncryptArray, 0, toEncryptArray.Length);
            tdes.Clear();
            return Convert.ToBase64String(resultArray, 0, resultArray.Length);
        }

        private static string Decrypt(string cipherString)
        {
            byte[] keyArray;
            byte[] toEncryptArray = Convert.FromBase64String(cipherString);

            AppSettingsReader settingsReader = new AppSettingsReader();
            string key = (string)settingsReader.GetValue("SecurityKey", typeof(String));


            MD5CryptoServiceProvider hashmd5 = new MD5CryptoServiceProvider();
            keyArray = hashmd5.ComputeHash(UTF8Encoding.UTF8.GetBytes(key));
            hashmd5.Clear();

            TripleDESCryptoServiceProvider tdes = new TripleDESCryptoServiceProvider();
            tdes.Key = keyArray;
            tdes.Mode = CipherMode.ECB;
            tdes.Padding = PaddingMode.PKCS7;

            ICryptoTransform cTransform = tdes.CreateDecryptor();
            byte[] resultArray = cTransform.TransformFinalBlock(toEncryptArray, 0, toEncryptArray.Length);

            tdes.Clear();
            return UTF8Encoding.UTF8.GetString(resultArray);
        }
    }

Vielen Dank schonmal und ich hoffe stoße euch nicht allzu übel auf 😉
Timm

Hallo emuuu,

ich bin da jetzt kein Experte was Verschlüsselungen angeht und habe deine Quellcode auch nur kurz überflogen.

Wenn ich es richtig sehe liegt, dein Schwerpunkt auf der Verschlüsselung.

Dazu:
Wenn ich es richtig sehe verwendest du auf beiden Seiten den gleichen Key (der dann auch noch in der App Konfig steht).

Grundlegend solltest du das über einen Private und Public Key lösen. Und die Keys nicht in der App Config speichern. Wenn ich es richtig im Kopf habe, bietet das .NET Framwork das schon passende Methoden an (Auch um die Keys zu speichern) (Bitte selber mal googlen).

Zum Rest.
Die Mischung von normaler Klasse und Statischen Methoden, halte ich für Fragwürdig. Eine klare Trennung ist da meist besser.

Dann ist da noch der Zugriff, in der Klasse auf die App Konfig. Denn würde ich auch auslagern. Damit muss die Klasse wissen wo der Schlüssel gespeichert ist.

Mit freundlichen Grüßen
Björne was verschlüsselungen angegt un dasdd

Vielen Danks schonmal 😃

gesetzt den Fall, dass ich nur eine Kommunikation zwischen einzelnen Clients und dem Server möchte. Könnten dann beide den gleichen Privatekey gespeichert haben?
In dem Fall den ich mir gerade vorstelle gibt der Client dem Server Aufgaben auf (bsp weil der Client zu wenig Rechenkapazität hat oder nur der Server Zugriff auf Daten hat) und erhält dann die Ergebnisse vom Server zurück.

Aus dem Grund ist für mich nicht nur die Verschlüsselung wichtig sondern auch die eigentliche Art der Datenübertragung.

Da der Client eine Anfrage an den Server stellt, kann er da auch seinen Public Key mitgeben. Grundlegend brauchen beiden nicht den gleichen Key.

Schau dir an dir einfach mal WCF an. Damit solltest du eine Verschlüsselte erstellen können.

Hinweis

                Sender = receivedParts[0];  
                 SendTime = DateTime.FromBinary(long.Parse(receivedParts[1]));  
                 ReceiveTime = DateTime.Now;  
                 SendDuration = Math.Round((ReceiveTime - SendTime).TotalMilliseconds, 2);

double SendDuration;

Der Datentyp ist schon richtig gewählt. Negative Zeiten sind nicht unrealistisch.
Es sei denn beim Login/Initialhandshake übermittelt der Server seine Zeit an den
Client und dieser rechnet dann den Offset auf seine Sendezeit.

Oder es fliegt eine Exception, weil es kein negtiven DateTime gibt (bevor er in Millisekunden umgerechnet wird. Da bin ich mir adhoc nicht sicher).

A DateTime is always an absolute position in time. I think what you're searching for is a TimeSpan, which can also be negative.

Du packst mehrere Funktionalitäten in eine Klasse: Verschlüsselung, Protokoll und Verarbeitung. Damit bist Du nicht flexibel und es verstößt noch gegen das Single-Responsibility-Prinzip.

Ich würde eine eigene Ver/Entschlüsselungs-Klasse mit den beiden Methoden 'Decrypt' und 'Encrypt' schreiben bzw. die beiden Methoden aus deiner Klasse rausschnippeln. Diese Klasse implementiert ein einfaches Interface:

Interface ICryptor 
{
  string Decrypt (string data, string key);
  string Encrypt (string data, string key);
}

Als nächstes definiere ich ein Interface, welches die Daten aus einem string extrahiert, bzw. dort hinhein stopfst.

Interface IDataTransportFileInformation
{
  string UserName {get;set;}
  string Filename {get;set}
  Bytes[] Content {get;set;}
  DateTime SendTime{get;set;}
  DateTime ReceiveTime {get;set;}
  TimeSpan Duration {get;}
  string CreateTransportMessage ();
  void ExtractFrom (string transportMessage);
}

Dann eine Klasse, die ein IDataTransportFileInformation-Objekt als Datei speichert bzw. aus einer Datei lädt.

Interface IDataTransportFileProcessor
{
  IDataTransportFileInformation FromFile(string filename);
  void ToFile (IDataTransportFileInformation fileInformation; string path);
}

Mit diesem Baukasten sehen deine Recieve-Methode so aus:

class DataTransferEngine
{
  ICryptor cryptor;
  IDataTransportFileProcessor processor;
   
public DataTransferEngine (ICryptor cryptor,  IDataTransportFileProcessor processor)
  {
    this.cryptor = cryptor;
    this.processor = processor;
  }

  public ReceivedFile(IDataTransportInformation transportInformation, string input, string path)
  {
     string decryptedInput = crypter.Decrypt(Input);
     transportInformation.ExtractFrom (decryptedInput);
     transportInformation.ReceiveTime = DateTime.Now;
     this.processor.ToFile (transportInformation);   
   }
 ...
}

Die try...except- Behandung habe ich herausgenommen, denn sie kaschiert den Grund für den Fehlschlag komplett (Disk voll, Dateiname ungültig, Daten korrupt).

Du erstellst ein IDataTransportInformation-Objekt, und übergibst es zusammen mit dem empfangenen String und dem Zielpfad an 'ReceivedFile'. Dort erfolgt die Verarbeitung. Sofern keine Exception geworfen wird, hast Du deine Datei und im DTI-Objekt alles, was Du über die Datei wissen musst.

Der Konstruktor deiner Klasse empfängt einen Cryptor und einen Processor, d.h. die Klasse erzeugt ihre Abhängigkeiten nicht selbst. Das nennt man DI ('Dependency Injection') und erleichtert das schreiben von Unittests.

Die try...except- Behandung habe ich herausgenommen, denn sie kaschiert den Grund für den Fehlschlag komplett (Disk voll, Dateiname ungültig, Daten korrupt).

Diese würde ich nicht entfernen, da sonst bei jeder Exception das Programm crashed. Grade bei TCP/SQL etc. sollte man immer zu Sicherheit try catch verwenden,da man nie alle Fehler selbst abfangen kann bzw. es enormer Aufwand währe. Jedoch wurde das try catch natürlich falsch implementiert

Hier ein etwas bessere Fehlerbehandlung

try
{
}
catch(Exception error)
{
//error.StackTrace -> in Error-Log speichern
//error.Message -> in Error-Log speichern
//ggf. Datum / Uhrzeit in Error-Log speichern
//MessageBox mit nachricht an den User das etwas nicht funktioniert hat ggf. error.Message anzeigen
//bei Exceptions wie timeout ggf. auto-retry einbauen
}

Natürlich kann man die Fehlerbehandlung auch außerhalb der Klasse gestallten, es sollte aber jedenfalls darauf geachtet werden das die Exceptions irgendwo, ob nun in der Klasse oder außerhalb, abgefangen werden.

Vielen Dank schonmal für die seeehr ausführliche Antwort!

Das werde ich jetzt erstmal nachvollziehen müssen, aber auf jeden Fall schonmal sehr erleuchtend 😃

Eine weitere Frage hätte ich dazu noch:
Die Decrypt- und Encrypt-Methoden, die ich verwendet habe, in wie weit kann man die als "sicher" bezeichnen.

Oder generell: wenn ich Daten über TCP verschicken möchte, welche Art der Verschlüsselung wäre da am effektivsten, wenn der Anspruch ist, dass nicht jeder der den Datenstrom mitkriegt damit sofort (oder mit geringem Ressourcenaufwand) was anfangen kann.