PasswordSafe- und Generator

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
Antworten
Citral
User
Beiträge: 12
Registriert: Dienstag 7. Juni 2016, 04:27
Kontaktdaten:

Huhu,

bin absoluter Neuling was Python angeht und bin dankbar für jeden nützlichen Ratschlag wie ich den Code besser und sauberer hinbekomme.

Das ist auch somit mein erstes Pythonprojekt. Wenn ich das gelernte nicht für mich nutzbar umsetzen kann, fehlt mir die Motivation um weiterzumachen. Deswegen habe ich gleich was bisschen anspruchvolleres ausgesucht und einen PasswordSafe programmiert.

Es hat ziemlich viel CO2 durch Google und Denkarbeit produziert, aber es macht Spaß und für mein erstes Projekt find ich es gar net mal so schlecht.

Hier der Code:
[pastebin]z1y0m5Hi[/pastebin]
http://pastebin.com/z1y0m5Hi

Also her mit eurem Feedback :)

PS: die Pastebin-tags hier im Forum funktionieren nicht.
DasIch
User
Beiträge: 2718
Registriert: Montag 19. Mai 2008, 04:21
Wohnort: Berlin

Bei deiner pwsafe Klasse missbrauchst du Klassen als Namespace. Wenn du einen weiteren Namespace haben willst, nutz ein Modul.

random mit os.urandom() zu seeden ist unnötig und wahrscheinlich auch nicht sonderlich sinnvoll. Das random Modul hat eine SystemRandom Klasse die auf os.urandom() aufbaut und dir alles bietet was du so brauchst, nutzt die.

Dateien und andere Ressourcen immer mit dem with Statement verwenden.

SHA256 ist wie schon im anderen Thread erwähnt hierfür überhaupt nicht geeignet. Nimm eine KDF wie pbkdf2, bcrypt, scrypt oder argon2.

Auf Modul Ebene gehören ausschliesslich Definitionen, also Importe, Konstanten, Klassen und Funktionen.

Wo wir beim Stichwort Funktionen sind, Funktionen sind eine tolle Sache die du viel zu wenig nutzt.

global ist in diesem Fall sinnlos und tut nichts. Grundsätzlich sollte man es auch sonst nicht benutzen.

Ein Masterpasswort sollte eine Passphrase sein die man sich gut merken kann, eine Kombination aus 3-4 zufälligen Wörtern ist hier optimal und eine sichere Lösung. Was du machst führt dazu dass sich der Nutzer das Passwort garantiert nicht merken kann und es sich irgendwo aufschreibt, was eine schlechte Idee ist.

Passwörter für einzelne Seiten die man sich dank des Passwortmanagers ohnehin nicht merken muss kann man dementsprechend nahezu unbegrenzt kompliziert machen. Das ist ein gewaltiger Vorteil den man sich zu nutzen machen sollte. Da kann ein Passwort dann durchaus z.B. 50 Zeichen lang sein statt nur 12.

Webseiten und Usernamen sind bei deinem Ansatz für andere einsehbar. Das muss so nicht sein und sollte dementsprechend auch nicht sein.

Wenn man try except nutzt ohne eine es auf bestimmte Exceptions einzugrenzen, wird der Code schwerer verständlich und versteckt Fehler.
BlackJack

(Inhalt überschneidet sich ein wenig mit DasIch's Beitrag, aber ich bin zu faul die Duplikate jetzt aus Meinem zu entfernen :-))

@Citral: Der Kommentar beim Import von `choice` sollte da nicht stehen. Selbstverständlichkeiten sollte man nicht kommentieren. Natürlich kann man den Namen `choice` nicht verwenden wenn man `random` importiert hat. Das wäre auch ziemlich undurchsichtig wenn Importe einfach so Namen definieren würden die man der Importzeile nicht ansieht.

Die Klasse `pwsafe` ist keine Klasse, also sollte die da so nicht stehen. Du definierst da einfach nur einen Haufen Funktionen. Wenn die in einen eigenen Namensraum sollen, dann steck die in ein Modul, dafür sind die da.

Auf Modulebene gehören nur Definitionen von Konstanten, Funktionen, und Klassen. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst.

Wenn Du `random.seed` etwas zuweist, dann überschreibst Du damit die `seed()`-Funktion im `random`-Modul. Das ist sicher nicht das was Du wolltest. Einen `seed()` aus `os.urandom()` zu setzen ist auch nicht besonders sicher, denn das ist in der Regel das was automatisch beim ersten Import von `random` passiert. Wenn Du ”besseren” Zufall als vom Mersenne Twister haben möchtest und es `os.urandom()` gibt, dann müsstest Du ein `random.SystemRandom`-Exemplar erstellen und verwenden.

Namen sollte man nicht kryptisch abkürzen. `gen()` ist nicht so aussagekräftig wie `generate_password()` und `createpw` ist ein komischer Name für ein Passwort.

Daten sollte man genau wie Code nicht wiederholen. So etwas wie einen (festen) Dateinamen definiert man einmal als Konstante. Dann besteht nicht die Gefahr das man sich bei der x-ten Verwendung vertippt, und wenn man den Wert mal ändern möchte, muss man nicht das ganze Programm durchsuchen.

`save()` könnte etwas passender als `append_password()` bezeichnet werden. Denn bei `save()`/`save_password()` würde ich erwarten das ein eventuell gespeichertes Passwort für das gleiche Paar von Benutzer/Site überschrieben wird. Denn eigentlich ist so ein Passwort-Safe ja eine Abbildung von diesem Paar auf ein Passwort.

Dateien sollte man wo es geht mit der ``with``-Anweisung zusammen öffnen. Das stellt sicher das die Datei auf jeden Fall geschlossen wird, egal warum/wie der Block verlassen wird.

Gute Namen zu finden ist manchmal nicht so einfach. Das kann man umgehen in dem man nicht jedes kleine Zwischenergebnis an einen Namen bindet. Namen für Zwischenergebnisse können manchmal zur Verständlichkeit beitragen, aber bei `encpwec`, `encpwdc`, `decryptedpw`, und `decryptedpwdc` muss man den Code verstehen der diesen Namen Werte zuweist, die bringen also keinen Mehrwert.

Das Hauptprogramm ist zu lang für *eine* Funktion.

Die ``while``-Schleife im Hauptprogramm mit dem `i` ist merkwürdig. Da steckt Code drin der nur ein einziges mal ausgeführt wird. Und zwar beim ersten Durchlauf. Das gehört vor die Schleife. Und dann kann man sich das `i` auch sparen.

Vergiss das es ``global`` gibt. Das hat in sauberen Programmen nichts zu suchen und ist hier auch völlig überflüssig.

Die Kommandos 'generate', 'newentry', und 'newentrypw' teilen sich jeweils Code den man nicht mehrfach im Programm stehen haben sollte.

Nackte ``except:``\s ohne konkrete Ausnahme(n) sind böse! Das behandelt *alle* Ausnahmen, auch solche mit denen Du gar nicht rechnest, und die deshalb auch nicht sinnvoll mit dem Code in dem Zweig behandelt werden können. Zum Beispiel greift das auch wenn Du irgendwo einen Programmierfehler gemacht hast und Beispielsweise einen Namen falsch geschrieben hast. Solche Fehler sind dann nur schwer zu finden wenn die von einer unpassenden Fehlerbehandlung ”verschluckt” werden.

Mit `exit()` wäre ich sparsam(er). Es reicht hier einfach die Schleife zu verlassen.

Ich lande dann ungefähr bei so etwas (ungetestet):

Code: Alles auswählen

import base64
import getpass
import hashlib
import string
from random import SystemRandom

import simplecrypt

SAFE_FILENAME = 'safe.svd'
GENERATED_PASSWORD_LENGTH = 12

def generate_password(length):
    chars = string.ascii_letters + string.digits + '!@#$%^&*()'
    random = SystemRandom()
    return ''.join(random.choice(chars) for _ in range(length))


def append_password(site, user, password):
    # 
    # TODO Use `csv` module.
    # 
    with open(SAFE_FILENAME, 'a') as safe_file:
        safe_file.write(site + ';' + user + ';' + password + '\n')


def read(): #nicht fertig. Gibt Leerzeilen zwischen Datensätzen an
    with open(SAFE_FILENAME, 'r') as safe_file:
        for line in safe_file:
            print(line)


def sha256(password):
    return hashlib.sha256(password.encode('utf-8')).hexdigest()


def encrypt(masterpassword, password):
    return base64.b64encode(
        simplecrypt.encrypt(masterpassword, password)
    ).decode('utf-8')


def decrypt(masterpassword, password):
    return simplecrypt.decrypt(
        masterpassword, base64.b64decode(password.encode('utf-8'))
    ).decode('utf-8')


def main():
    masterpassword = sha256(
        getpass.getpass('Bitte gebe das Masterpasswort ein:')
    )
    while True:
        print('Welchen Befehl möchtest du ausführen?')
        print(
            'generate(Passwort generieren), newentry(Neuen Eintrag), '
            'newentrypw(Neuer Eintrag inkl. generiertem Passwort),\n '
            'read(Datenbank abfragen), decrypt(Passwort entschlüsseln), '
            'exit(Programm beenden)'
        )
        command = input('Befehl: ')
        # 
        # TODO Factor out repeating code from the commands.
        # 
        if command == 'generate':
            try:
                length = int(input('Passwortlänge: '))
            except ValueError:
                length = GENERATED_PASSWORD_LENGTH
                print(
                    'Keine oder falsche Eingbe. '
                    'Der Standardwert wird benutzt.\n'
                )
            print('##################################')
            print(
                'Das generierte Passwort lautet:\n' + generate_password(length)
            )
            print('##################################')
            print('Kehre zum Hauptmenü zurück.\n')

        elif command == 'newentry':
            append_password(
                input('Webseitenname: '),
                input('Username: '),
                encrypt(masterpassword, getpass.getpass('Passwort:'))
            )
            print('Erfolgreich gespeichert. Kehre zum Hauptmenü zurück\n')

        elif command == 'newentrypw':
            site = input('Webseitenname:')
            user = input('Username:')
            try:
                length = int(input('Passwortlänge: '))
            except ValueError:
                length = GENERATED_PASSWORD_LENGTH
                print('Falsche oder keine Angabe. Standardwert wird benutzt')
            password = generate_password(length)
            print('##################################')
            print('Das Passwort lautet:\n'+password)
            print('##################################')
            append_password(site, user, encrypt(masterpassword, password))
            print('Erfolgreich gespeichert. Kehre zum Hauptmenü zurück\n')

        elif command == 'read':
            print(
                "Webseite ; User ; verschlüsseltes Passwort (ohne Semikolon ';')"
            )
            reading = read()
            print(reading)

        elif command == 'decrypt':
            try:
                encrypted_password = input(
                    'Gebe das zu entschlüsselnde Passwort ein:'
                )
                print('##################################')
                print('Das Passwort lautet:')
                print(decrypt(masterpassword, encrypted_password))
                print('##################################')
            except simplecrypt.DecryptionException:
                print(
                    'Fehler: Bitte überprüfe das Masterpasswort und das zu '
                    'entschlüsselnde Passwort auf Richtigkeit\n'
                )

        elif command == 'exit':
            break

        else:
            print(
                'Leider hab ich dich nicht verstanden!.'
                ' Kehre zum Hauptmenü zurück.'
            )


if __name__ == '__main__':
    main()
Bei dem Menü ist unschön, dass der Code a) viel zu lang für eine Funktion ist, und sich b) der Text für die Kommandos wiederholt, einmal am Anfang wo sie alle ausgegeben werden, und dann bei den Vergleichen mit der Eingabe. Das ist eine Fehlerquelle und es ist umständlich Kommandos zu entfernen, umzubenennen, oder zu entfernen. Das `cmd`-Modul könnte hier hilfreich sein.
Citral
User
Beiträge: 12
Registriert: Dienstag 7. Juni 2016, 04:27
Kontaktdaten:

Hallo,
dann mal von oben nach unten wie ein Programm ;).
Vielen Dank für eure Antworten, haben mir geholfen mich tiefergehend mit Python zu beschäftigen und ich weiß schonmal was ich als nächstes lernen werde.

@Ich bzw. Du oder doch Ich?
random mit os.urandom() zu seeden ist unnötig und wahrscheinlich auch nicht sonderlich sinnvoll. Das random Modul hat eine SystemRandom Klasse die auf os.urandom() aufbaut und dir alles bietet was du so brauchst, nutzt die.
Ist die einzige Funktion, samt der Zeile dadrunter die ich kopiert und leicht modifiziert habe. Ganz ehrlich ich weiß da selber nicht genau, was der Code macht, also hab da noch kleine Verständnisschwierigkeiten. Werde ich mir bei Gelegenheit näher anschauen.
Dateien und andere Ressourcen immer mit dem with Statement verwenden.
Wird umgesetzt :!:
SHA256 ist wie schon im anderen Thread erwähnt hierfür überhaupt nicht geeignet. Nimm eine KDF wie pbkdf2, bcrypt, scrypt oder argon2.
Wird ja eh nicht gespeichert, sondern weiterverarbeitet. crypt(sha256masterpw, zuverschlüsselnde pw). Denke mir das es so noch ne nummer sicherer ist, als direkt das Masterpasswort hierfür zu nehmen. Kenne noch nicht allzuviele Verschlüsselungsmöglichkeiten und habe deswegen auf etwas bekanntes zurückgegriffen. Werde mich aber weiter da einarbeiten.
Auf Modul Ebene gehören ausschliesslich Definitionen, also Importe, Konstanten, Klassen und Funktionen.
Sorry für die dumme frage... aber was heißt genau Modul Ebene?
Wo wir beim Stichwort Funktionen sind, Funktionen sind eine tolle Sache die du viel zu wenig nutzt.
Mein Verständnis von Funktionen:
Standardfunktionen(if, while, usw.)
Funktionen der Bibliothek inkl. Module (?)
und selbstentwickelte Funktionen/Module

Beschäftige mich noch nichtmal 2 Wochen mit Python und habe noch nicht viele kennenlernen können. Ich hoffe dies ändert sich schnell ;).
global ist in diesem Fall sinnlos und tut nichts. Grundsätzlich sollte man es auch sonst nicht benutzen.
Okay,
hab dies aus Unsicherheit reingeschrieben, da ich die Variable auch in den Funktionen und if-abfragen gebraucht habe. War der Meinung das in if-abfragen definierte Variablen nicht außerhalb der Abfrage funktionieren. Aber nochmals darüber nachgedacht, ist es bestimmt Unsinn und werde ich mal testen.
Passwörter für einzelne Seiten die man sich dank des Passwortmanagers ohnehin nicht merken muss kann man dementsprechend nahezu unbegrenzt kompliziert machen. Das ist ein gewaltiger Vorteil den man sich zu nutzen machen sollte. Da kann ein Passwort dann durchaus z.B. 50 Zeichen lang sein statt nur 12.
Nun es soll Seiten geben die Passwörter in 8-ca. 20 Zeichen erlauben... oder ganz eng kalkuliert. 12 Zeichen sollten also für 99,9% aller Seiten funktionieren, bei Bedarf kann man die Länge selber angeben oder den Standardwert ändern.

@BlackJack
Der Kommentar beim Import von `choice` sollte da nicht stehen. Selbstverständlichkeiten sollte man nicht kommentieren. Natürlich kann man den Namen `choice` nicht verwenden wenn man `random` importiert hat. Das wäre auch ziemlich undurchsichtig wenn Importe einfach so Namen definieren würden die man der Importzeile nicht ansieht.
War ne Randnotiz beim scripten, weil ich es persönlich nicht verstanden habe. Ich denke/dachte wenn random geimportet wurde, dass damit alle Funktionen innerhalb des Moduls mitkopiert wurde und Choice auch eine funktion von random ist.
Aber ich meine es mittlerweile gecheckt zu haben. Da random bereits importiert, könnte ich die Zeile rauslöschen und dafür

Code: Alles auswählen

        createpw = ''.join([[b]random.[/b]choice(chars) for i in range(length)])
zu ändern. Funktionieren tuts bei mir.
Daten sollte man genau wie Code nicht wiederholen. So etwas wie einen (festen) Dateinamen definiert man einmal als Konstante. Dann besteht nicht die Gefahr das man sich bei der x-ten Verwendung vertippt, und wenn man den Wert mal ändern möchte, muss man nicht das ganze Programm durchsuchen.
Ja daran hab ich nicht gedacht :)

und danke fürs modifizieren des Codes... habe mir einiges abgucken können und werde es demnächst umsetzten. Natürlich ohne Copy & Paste, wo wäre sonst der Lernfaktor ;)
BlackJack

Citral: Modulebene = Anweisungen die beim Import ausgeführt werden, also alles was nicht in einer Funktions- oder Klassendefinition steht, die auf Modulebene steht.

``if``, ``while``, und so weiter sind keine Funktionen. Funktionen sind Objekte die man Aufrufen kann. Also ein Ausdruck der irgendwas ergibt, wo man Klammern mit Argumenten dahinter schreiben kann. Das stimmt nicht ganz, weil das die Beschreibung von ”callables” ist, und da fallen auch Sachen wie Klassen (kann man als Frabrikfunktion für Exemplare der Klasse ansehen) oder Methoden drunter (kann man als an ein Ojekt gebundene Funktionen ansehen).
Antworten