Bug in FRITZBOX_Rename()

Begonnen von amenomade, 20 Mai 2020, 23:31:51

Vorheriges Thema - Nächstes Thema

amenomade

Da ich ein ähnliches Mechanismus implementieren wollte, bin ich darüber gestolpert:

Nehmen wir an, mein Fritzbox Device heisst "oldname" und hat das Passwort "passwort" und FUUID = "XXXXXXXX-XXXX"
4697 sub FRITZBOX_storePassword($$)
4698 {
4699     my ($hash, $password) = @_;
4700      
4701     my $index = $hash->{TYPE}."_".$hash->{NAME}."_passwd";
4702     my $key = getUniqueId().$index;
4703    


$index = FRITZBOX_oldname_passwd
$key = XXXXXXXX-XXXX.FRITZBOX_oldname_passwd

Jetzt wird $key geziffert und  $password mittels gezifferter $key geziffert.
$key ist aber abhängig vom alten Name ! Und damit wurde das Passwort geziffert.
Hier in der Passwort Datei:
FRITZBOX_oldname_passwd:42071544145d1012


Was macht ein  rename oldname newname ?:
290 sub FRITZBOX_Rename($$)
291 {
292     my ($new, $old) = @_;
293    
294     my $old_index = "FRITZBOX_".$old."_passwd";
295     my $new_index = "FRITZBOX_".$new."_passwd";
296    
297     my ($err, $old_pwd) = getKeyValue($old_index);
298    
299     setKeyValue($new_index, $old_pwd);
300     setKeyValue($old_index, undef);
301 }

Also, übernimmt das gezifferte Passwort vom alten index zum neuen index.
So wie es ist, und immer noch mit altem Key (abhängig vom alten Name) geziffert.

Nach Rename in der Passwort Datei:
FRITZBOX_newname_passwd:42071544145d1012

Wenn ich jetzt ein readPassword mache,
4732    my $index = $hash->{TYPE}."_".$hash->{NAME}."_passwd";
4733    my $key = getUniqueId().$index;

Die Funktion versucht mit $key = XXXXXXXX-XXXX.FRITZBOX_newname_passwd zu entziffern.
{ FRITZBOX_readPassword($defs{newname}) }

Ergebnis: "saqu-n$", was nichts mit dem echten Passwort "passwort" zu tun hat.


Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

amenomade

Kennt jemand eine Möglichkeit, @tupol darauf zu bringen? PN habe ich schon geschickt
Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

tupol

FHEM 5.5 auf RPi B Rev.2 (mit LCD4Linux, BMP180 und CUL v3 868.35 MHz), FB7490, Fritz!DECT 200, FS20, FHT80TF-2, S300TH, KS300, Homematic, PRESENCE
Modul-Entwickler von: FRITZBOX, statistics, PROPLANTA, OPENWEATHER, JSONMETER, LUXTRONIK2

amenomade

#3
Ohje, hab keine Ahnung, wie man es macht. Ich weiss, es gibt eine wiki Seite https://wiki.fhem.de/wiki/How_to_write_a_patch aber das ist Chinesisch für mich

Ausserdem ist die Lösung nicht einfach...

In meinem Code habe ich decrypt und encrypt in eigene Funktionen getrennt, und diese Funktionen dann benutzt, um mit der alten Key das Passwort zu entziffern und mit der neuen wieder zu ziffern.
Das heisst aber auch eine Änderung in storePassword und readPassword.

https://forum.fhem.de/index.php?action=dlattach;topic=111294.0;attach=136380

#######################################################################
sub freeairconnect_Rename
{
    my ($new, $old) = @_;
    Log3 $old, 5, "freeairconnect ($old): Rename:  $old $new";
    my $old_index = "freeairconnect_".$old."_passwd";
    my $new_index = "freeairconnect_".$new."_passwd";
   
    my ($err, $old_pwd) = getKeyValue($old_index);
    my $new_pwd = freeairconnect_encryptPassword($new, freeairconnect_decryptPassword($old,$old_pwd));
   
    setKeyValue($new_index, $new_pwd);
    setKeyValue($old_index, undef);
   
    return;

}

#####################################
# checks and stores freeairconnect key used for http call to freeair-connect.de
sub freeairconnect_storePassword
{
    my ($hash, $password) = @_;
    my $name = $hash->{NAME};
    Log3 $name, 5, "freeairconnect ($name): storePassword";
    my $index = "freeairconnect_".$hash->{NAME}."_passwd";
   
    my $enc_pwd = freeairconnect_encryptPassword($name, $password);
   
    my $err = setKeyValue($index, $enc_pwd);
    return "error while saving the password - $err" if(defined($err));
   
    return "password successfully saved";
} # end freeairconnect_storePassword

#########################################################################
# encrypts password
sub freeairconnect_encryptPassword
{
    my ($name, $password) = @_;
    Log3 $name, 5, "freeairconnect ($name): encrypts password";
    my $index = "freeairconnect_".$name."_passwd";
    my $key = getUniqueId().$index;
   
    my $enc_pwd = "";

## no critic (ProhibitStringyEval)
    if(eval "use Digest::MD5;1")
    {
        $key = Digest::MD5::md5_hex(unpack "H*", $key);
        $key .= Digest::MD5::md5_hex($key);
    }
## use critic (ProhibitStringyEval)   
    for my $char (split //, $password)
    {
        my $encode=chop($key);
        $enc_pwd.=sprintf("%.2x",ord($char)^ord($encode));
        $key=$encode.$key;
    }
    return $enc_pwd;
}

#########################################################################
# decrypts password
sub freeairconnect_decryptPassword
{
   my ($name, $password) = @_;
   Log3 $name, 5, "freeairconnect ($name): decryptPassword";
   my $index = "freeairconnect_".$name."_passwd";
   my $key = getUniqueId().$index;
   
   if ( defined($password) ) {
## no critic (ProhibitStringyEval)
      if ( eval "use Digest::MD5;1" ) {
         $key = Digest::MD5::md5_hex(unpack "H*", $key);
         $key .= Digest::MD5::md5_hex($key);
      }
## use critic (ProhibitStringyEval)
      my $dec_pwd = '';
     
      for my $char (map { pack('C', hex($_)) } ($password =~ /(..)/xg)) {
         my $decode=chop($key);
         $dec_pwd.=chr(ord($char)^ord($decode));
         $key=$decode.$key;
      }
     
      return $dec_pwd;
   }
}   
 
 
#####################################
# reads the freeairconnect password
sub freeairconnect_readPassword
{
   my ($hash) = @_;
   my $name = $hash->{NAME};
   Log3 $name, 5, "freeairconnect ($name): storePassword";

   my $index = "freeairconnect_".$hash->{NAME}."_passwd";

   my ($password, $err);

   Log3 $name, 5, "freeairconnect ($name): Read freeairconnect password from file";
   ($err, $password) = getKeyValue($index);

   if ( defined($err) ) {
      Log3 $name, 1, "freeairconnect ($name): unable to read freeairconnect password from file: $err";
      return;
   } 
   
   if ( defined($password) ) {
     
      return freeairconnect_decryptPassword($name, $password);
   }
   else {
      Log3 $name, 1, "freeairconnect ($name): No password in file";
      return;
   }
} # end freeairconnect_readPassword

Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

amenomade

#4
Eine Alternative wäre vielleicht, diese Key unabhängig von dem Devicename zu machen? FUUID könnte reichen. Was meinst Du?

Ich arbeite mich in patches ein ;)

EDIT: so würde es aussehen? (Achtung, noch nicht getestet)
Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

amenomade

#5
Hmmm scheint nicht zu reichen.
Der Index des Passworts wird in der Passwort Datei aktualisiert, aber ich kriege nach einem rename
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in pattern match (m//) at ./FHEM/72_FRITZBOX.pm line 741.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in string ne at ./FHEM/72_FRITZBOX.pm line 758.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value in string ne at ./FHEM/72_FRITZBOX.pm line 758.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 779.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 791.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 807.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 881.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 896.
2020.05.27 00:56:07 1: ERROR: empty name in readingsBeginUpdate
2020.05.27 00:56:07 1: stacktrace:
2020.05.27 00:56:07 1:     main::readingsBeginUpdate           called by ./FHEM/72_FRITZBOX.pm (1794)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Process      called by ./FHEM/72_FRITZBOX.pm (1776)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Done         called by (eval 803) (1)
2020.05.27 00:56:07 1:     (eval)                              called by fhem.pl (1150)
2020.05.27 00:56:07 1:     main::AnalyzePerlCommand            called by fhem.pl (1175)
2020.05.27 00:56:07 1:     main::AnalyzeCommand                called by fhem.pl (1104)
2020.05.27 00:56:07 1:     main::AnalyzeCommandChain           called by ./FHEM/98_telnet.pm (255)
2020.05.27 00:56:07 1:     main::telnet_Read                   called by fhem.pl (3785)
2020.05.27 00:56:07 1:     main::CallFn                        called by fhem.pl (761)
2020.05.27 00:56:07 1: readingsUpdate(,lastReadout,9 values captured in 0.00 s) missed to call readingsBeginUpdate first.
2020.05.27 00:56:07 1: stacktrace:
2020.05.27 00:56:07 1:     main::readingsBulkUpdate            called by ./FHEM/72_FRITZBOX.pm (1916)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Process      called by ./FHEM/72_FRITZBOX.pm (1776)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Done         called by (eval 803) (1)
2020.05.27 00:56:07 1:     (eval)                              called by fhem.pl (1150)
2020.05.27 00:56:07 1:     main::AnalyzePerlCommand            called by fhem.pl (1175)
2020.05.27 00:56:07 1:     main::AnalyzeCommand                called by fhem.pl (1104)
2020.05.27 00:56:07 1:     main::AnalyzeCommandChain           called by ./FHEM/98_telnet.pm (255)
2020.05.27 00:56:07 1:     main::telnet_Read                   called by fhem.pl (3785)
2020.05.27 00:56:07 1:     main::CallFn                        called by fhem.pl (761)
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in pattern match (m//) at ./FHEM/72_FRITZBOX.pm line 741.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in string ne at ./FHEM/72_FRITZBOX.pm line 758.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value in string ne at ./FHEM/72_FRITZBOX.pm line 758.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 779.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 791.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 807.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 881.
2020.05.27 00:56:07 1: PERL WARNING: Use of uninitialized value $host in concatenation (.) or string at ./FHEM/72_FRITZBOX.pm line 896.
2020.05.27 00:56:07 1: ERROR: empty name in readingsBeginUpdate
2020.05.27 00:56:07 1: stacktrace:
2020.05.27 00:56:07 1:     main::readingsBeginUpdate           called by ./FHEM/72_FRITZBOX.pm (1794)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Process      called by ./FHEM/72_FRITZBOX.pm (1776)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Done         called by (eval 806) (1)
2020.05.27 00:56:07 1:     (eval)                              called by fhem.pl (1150)
2020.05.27 00:56:07 1:     main::AnalyzePerlCommand            called by fhem.pl (1175)
2020.05.27 00:56:07 1:     main::AnalyzeCommand                called by fhem.pl (1104)
2020.05.27 00:56:07 1:     main::AnalyzeCommandChain           called by ./FHEM/98_telnet.pm (255)
2020.05.27 00:56:07 1:     main::telnet_Read                   called by fhem.pl (3785)
2020.05.27 00:56:07 1:     main::CallFn                        called by fhem.pl (761)
2020.05.27 00:56:07 1: readingsUpdate(,lastReadout,9 values captured in 0.00 s) missed to call readingsBeginUpdate first.
2020.05.27 00:56:07 1: stacktrace:
2020.05.27 00:56:07 1:     main::readingsBulkUpdate            called by ./FHEM/72_FRITZBOX.pm (1916)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Process      called by ./FHEM/72_FRITZBOX.pm (1776)
2020.05.27 00:56:07 1:     main::FRITZBOX_Readout_Done         called by (eval 806) (1)
2020.05.27 00:56:07 1:     (eval)                              called by fhem.pl (1150)
2020.05.27 00:56:07 1:     main::AnalyzePerlCommand            called by fhem.pl (1175)
2020.05.27 00:56:07 1:     main::AnalyzeCommand                called by fhem.pl (1104)
2020.05.27 00:56:07 1:     main::AnalyzeCommandChain           called by ./FHEM/98_telnet.pm (255)
2020.05.27 00:56:07 1:     main::telnet_Read                   called by fhem.pl (3785)
2020.05.27 00:56:07 1:     main::CallFn                        called by fhem.pl (761)
2020.05.27 00:56:09 3: FRITZBOX: set FritzBox update


Mache ich ein rename wieder auf den alten Name oder nach einem restart von Fhem, ist alles wieder still.

EDIT: diese Log Meldung kommen nicht direkt nach dem rename, sondern erst nach dem ersten set update. Ich vermute, dieses Problem war schon bei einem Rename ohne meinem Patch vorhanden...
Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

amenomade

#6
Ok, habs auch korrigiert.
Und auch getestet (ich habe aber nicht jede einzige Funktion vom Modul wieder getestet)

Das wird aber dazu führen, dass bei einem Update von Fhem, die Benutzer das Passwort verlieren werden, da es nicht mehr mit der gleichen Key geziffert wird...
Vielleicht ist es doch keine gute Idee und muss ich wieder auf meiner erste Idee zurückkommen....?


Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

amenomade

So... jetzt auch das Patch mit der ersten Idee
Zitatdecrypt und encrypt in eigene Funktionen getrennt, und diese Funktionen dann benutzt, um mit der alten Key das Passwort zu entziffern und mit der neuen wieder zu ziffern.
Das heisst aber auch eine Änderung in storePassword und readPassword.


Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

rudolfkoenig

ZitatDas wird aber dazu führen, dass bei einem Update von Fhem, die Benutzer das Passwort verlieren werden, da es nicht mehr mit der gleichen Key geziffert wird...
Man koennte versuchen den Schluessel nach dem alten Namensschema zu lesen, und falls er existiert, es nach dem neuen Schema (nur FUUID) zu speichern, und die alte Variante loeschen.

herrmannj

für mich stellt sich sowieso die Frage welchem Risiko mit dem Verschlüsseln begegnet werden soll. Wenn jemand unautorisiert Zugang zu den Daten hat (Platte, Rechner) dann hat er auch Zugang zum Schlüssel.

amenomade

Zitat von: rudolfkoenig am 27 Mai 2020, 18:32:27
Man koennte versuchen den Schluessel nach dem alten Namensschema zu lesen, und falls er existiert, es nach dem neuen Schema (nur FUUID) zu speichern, und die alte Variante loeschen.

Stimmt, aber irgendwie muss man das alte enziffern und wieder ziffern, wenn das Namenschema sich ändert. Das heisst sowieso encrypt/decrypt trennen und readPassword, Rename und storePassword überarbeiten. Es wäre nur ein bisschen performanter bei einem zukunftigen Rename ohne Änderung des Namenschemas. Oder sehe ich das falsch?
Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

CoolTux

Ich bilde mir ein im GardenaSmartBridge Modul das ganze gefixt zu haben. Vielleicht da mal schauen.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

amenomade

Zitat von: herrmannj am 27 Mai 2020, 18:45:35
für mich stellt sich sowieso die Frage welchem Risiko mit dem Verschlüsseln begegnet werden soll. Wenn jemand unautorisiert Zugang zu den Daten hat (Platte, Rechner) dann hat er auch Zugang zum Schlüssel.
Naja. Das stimmt schon. Aber man muss schon wissen, wie es verschlüsselt wurde, und eine Entschlüsselung bauen. Das ist schon mal schwieriger als "nur" aus einer Datei lesen.
Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

amenomade

#13
Zitat von: CoolTux am 27 Mai 2020, 18:53:38
Ich bilde mir ein im GardenaSmartBridge Modul das ganze gefixt zu haben. Vielleicht da mal schauen.
Wieso sagst Du es nicht früher? ;)
Ja, einfach der Name als zusätzlicher Parameter der Funktionen (read, store). Viel eleganter.
Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

amenomade

#14
So jetzt nach CoolTux Art ;) Danke!

Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus