netfishers-onl / Netshot

Network Configuration and Compliance Management
http://www.netfishers.onl/netshot
242 stars 57 forks source link

FortiGate - new diff every iteration #220

Closed pfunkylol closed 2 years ago

pfunkylol commented 2 years ago

Hello,

It appears the code below from Fortinet_FortiOS.js doesnt work properly since at every run a new diff is created since it detects that the salt for the passwords lines is being changed with every show command.

var previousConfiguration = device.get("configuration"); if (typeof previousConfiguration === "string" && removeChangingParts(previousConfiguration) === removeChangingParts(configuration)) { config.set("configuration", previousConfiguration); } else { config.set("configuration", configuration); }

pizu commented 2 years ago

That is normal as the FG firewall changes the enc each time you do a show command, the key in reality is not being changed.

On Fri, 01 Jul 2022, 09:27 pfunkylol, @.***> wrote:

Hello,

It appears the code below from Fortinet_FortiOS.js doesnt work properly since at every run a new diff is created since it detects that the salt for the passwords lines is being changed with every show command.

var previousConfiguration = device.get("configuration"); if (typeof previousConfiguration === "string" && removeChangingParts(previousConfiguration) === removeChangingParts(configuration)) { config.set("configuration", previousConfiguration); } else { config.set("configuration", configuration); }

— Reply to this email directly, view it on GitHub https://github.com/netfishers-onl/Netshot/issues/220, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVYRHX43MNBXPS2SOVZUWLVR2MX3ANCNFSM52LYS6SA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

SCadilhac commented 2 years ago

Indeed each time a FortiGate dumps out the running configuration, it uses a different salt to produce an encrypted version of the various secrets. Thus the FortiOS driver for Netshot has the following hack: using removeChangingParts function, it strips off all the encrypted (thus regenerated) parts from the config text before comparing the new configuration to the previous one: in case they are similar, it is assumed nothing has changed. This is an approximation: if only a password and nothing else was changed, the driver wouldn't see the delta at all. If @pfunkylol you see a diff at each snapshot, there might be something wrong with the removeChangingParts hack on your configurations and/or FortiOS version, so I will need more details to investigate.

pfunkylol commented 2 years ago

The FortiGate's in question are running 6.4.X and I'm running the latest netshot version.

e.g.

previous config certificate local edit "Fortinet_CA_SSL" set password ENC UVRBksNRrKwLMu52Sj9z96If3AE3fROmn64Okn6SyFKLovYXq/yGzGbqkA2IA4AB0PT3BFWFL4htYp+SHVcl/yjp06eC+OGGTvlFyk4Wc1qi5jXloQAPcOCJhj6UBPDdeENjcNB6VzUvBzBIL+qfyq5F6COMhUfCZZk6F9d2394rHnER84cMuWrdp6A5WDs9gmRVUA==

new config certificate local edit "Fortinet_CA_SSL" set password ENC F9aHkMe5X5QspxAwL0+vtNKzESvLqLidtxzTCeX4tcPUeI6MH0qV4QH6NmzB6owIguTbotfITCSDvPYf9E70WIeVUF/r6dT+sNKO4A8A0zQktYbcDWbuwOJOOr9iDX6bp9VSL2nkIYTX3A7UM6/qnzt4tCeGTJ4AKi9C6WSnPuid01dMyi3uapgerDdGdyKbSIamMg==

Please correct me if I'm wrong but the lines from above should match any/all lines with set password|passwd and should be replaced with an empty field so they can be ignored, but for some reason it doesnt work.

SCadilhac commented 2 years ago

The driver doesn't remove the password hashes from the configuration that will be stored, it rather just temporary removes them when comparing the previous and current configurations. Otherwise the backup wouldn't contain the passwords at all and couldn't be re-injected as it. So whenever a real change is performed, the config diff will show the change and all passwords will look like being changed too. Can you give a try with updated version? https://github.com/netfishers-onl/Netshot/commit/f851a89fdc61e7f8aa35f97eecdf368dc13ebc92

pfunkylol commented 2 years ago

At first glance appears to get the job done. I will keep an eye on it for a couple of days. [INFO] The configuration hasn't changed. Not storing a new one in the DB.

Thanks

pfunkylol commented 2 years ago

Hi @SCadilhac , it appears to work just fine.