Add necessary stuff before oxidized base patch to hide enable password from REST API

[Tibshot]

I would like to have the ability to hide enable password from oxidized's REST API like:

{
"full_name": "a-device",
"group": "default",
"ip": "10.10.10.10",
"last": {
"end": "2016-04-12 12:21:51 UTC",
"start": "2016-04-12 12:21:25 UTC",
"status": "success",
"time": 26.277578618
},
"model": "a-model",
"name": "a-device",
"status": "success",
"time": "2016-04-12 12:21:51 UTC",
"vars": {
"enable": "HIDDEN"
}

This patch doesn't break actual setup for users using clear password.

Patch for oxidized base project is coming (TM).

Tib

[supertylerc]

Looks good to me.

[ytti]

We already have ':remove_secret' var in configs, which models can use to remove secrets from config.

Should we use this, or new? I don't have strong opinion, just pointed it out.

[danilopopeye]

We already have ':remove_secret' var in configs, which models can use to remove secrets from config.

I like this idea 😄

[Tibshot]

From my point of view, I don't understand why :remove_secret is a variable. I prefer the way to define it via rest configuration scope but I agree that is purely subjective...

(Moreover, I didn't know :remove_secret existed before you speak about it :P)

[ytti]

If it would be under rest, do you think it would be still logical that models use it to determine if model will remove secrets from config?

Clearly this is dual-use of single value, I think the use-cases align perfectly, but I might be mistaken.

[Tibshot]

I think that it is more logical if this config param is set under rest scope because it's REST/Web interface that display or not informations.

This directive doesn't remove/rewirte anything within backup files. I have a doubt when you say: "it would be still logical that models use it to determine if model will remove secrets from config".

Does :remove_secret variable remove secret from backuped files ? If yes, my patch don't do that so it's not the same purpose for me. This patch just rewrite the enable var if you have this in your config file:

source:
  default: csv
  csv:
    file: ~/.config/oxidized/router.db
    delimiter: !ruby/regexp /:/
    map:
      name: 0
      model: 1
      username: 2
      password: 3
    vars_map:
      enable: 4

Setting vars_map with enable in config lead /node/show/* API/Web UI calls to return enable password in clear.

I think we don't speak about the same problematic, but I'm not sure ^^. 😋

Tib

[ytti]

I'm not talking about your proposal. I'm talking about current behaviour of 'remote_secret'.

You expressed confusion why it's 'var', and not under 'rest'. I explained, clearly poorly, that it has nothing to do with the webUI, it's about models removing secret information.

How this relates to your proposal is that I'm wondering do the use-cases of 'remove_secret' and your proposal align perfectly? If they do, we don't need two toggles?

[Tibshot]

Ok get it. :)

When I reviewed the code of remove_secret, I thought these two toggles were OK to run together. Because they don't modify stuff on same level. In my opinion, model.rb don't have to rewrite config stuff (following my exemple: vars_map).

Model.rb removes secrets that are often hashed in backup while REST method add hot removing of enable password in JSON/html response (REST method doesn't open/read/write backup file).

I think 'remove_secret' is very useful because each models have different way to store secrets and this feature is a must have for paranoïd. And I agree that method has nothing to do in WebUI. I didn't want to modify this behaviour, be sure of that.

Finally, I think these methods can clearly work together because their job are not the same at all.

Hoping that I answered your questions :)

[ytti]

Sorry for being thick, I'm not actual sure what your argument is.

Do you think we can 'overload' remove_secret to remove them in web too. Or do you think we should have separate on them?

Ultimately this boils down to 'is there use-case, where someone wants to filter secrets ONLY from configs or ONLY from data'. I don't know answer, I would err towards 'lets use same'.

[Tibshot]

No problem.

Actually, I don't have strict idea on this question. I think it could depends of use case as you said before.

From my business case, I need to keep secret in backuped conf but I need to hide enable from API/HTML response. Tricky. 😃
In case of restore, I get file directly from Oxidized, without recreating account or role (lazy). During some internals tool use API for monitoring or inventory. We manage url access with access list in web server directly to isolate them.

I get a bug with this PR, I need to continue to work on it. Nodes consider enable password as 'HIDDEN' for all equipments after first check if I request a web page >_<

Don't accept it for the moment, I will commit a fix soon directly in that PR I guess.

[ytti]

Ok. Excellent, your use-case proves we can't overload the configurations and need separate toggles for each.

So we'll go with your original idea.

[Tibshot]

Ok, great. :)

I find a way to debug this PR, so, I'll test it during the night and I'll push new commit tomorrow to this PR if it's OK.

I think that some volunteers will be necessary to test this patch. I haven't a big machine pool at this time to correctly test it.

[ytti]

Yeah I think we generally need lot of thought about testing and quality, I've been slacking on that, and it's really hurting the project. Currently after changing jobs, I don't even run oxidized myself, and have limited access to devices, it's not good for the project at all :/

[davama]

We are very interested in this patch.
We are willing to test.
Some equipment include:
CISCO FTTXs, Aironet APs, Routers(1900s,2900s,6400s,6500,6800s,Nexus7K)
HP Procurve switches (several models)

Please let me know if that will work.
Also, if i can help, how would i apply said patch?

Thanks
Dave

[davama]

BTW i tried was was suggested here: https://github.com/ytti/oxidized/pull/391

rest_hide_enable: true

It seems that it did hide it from HP configs only.

[Tibshot]

Hi Dave,

PR 391 from oxidized repo is mine too, it has been developed to work with this current PR. I didn't want manage Oxidized config in web project. :)

Hmm, only HP ? I work with Brocade devices and that works like a charm. Did you test it ?

If you want to test this PR, you will need to use ytti/oxidized#391 too. They work together.

By default, if you don't use ytti/oxidized#391, Oxidized Web will work as usual (no hidden enable).

Hoping I'm not confusing...

Tib

[Tibshot]

I commited new stuff.

PR contains all necessary things in order to use hide_enable in oxidized-web.

ytti/oxidized#391 needs to be accepted too for configuring rest_hide_enable: true in oxidized config file.

My test devices are OK (2 days and 1 night of tests) and REST/WebUI interfaces answer correctly with 'HIDDEN'.

Tib

[davama]

Hello Tib
Thanks for the help. I was out so i was not able to test sooner.

After a manual attempt of changing all committed files from this PR i just decided, as your note stated from the get go, to clone your oxidized-web fork into our test environment.

Also made sure PR#391 changes were done. (core.rb, config.rb and rest_hide_enable: true)

I too get the an answer of 'HIDDEN'.

Unfortunately i can still see the 'username priv 15 secret 5 blahblah' on all devices.

Btw, i had made a mistake on the procurve comment. password is still there.

I hope i followed the instructions properly.
Let me know if there is something i missed.

Dave

[Tibshot]

Hello Dave,

Thank you for testing my PRs anyway :)

When you say: "Unfortunately i can still see the 'username priv 15 secret 5 blahblah' on all devices.", you speak about backuped configuration, reachable via /node/fetch/your_device_name ? If yes, this is normal. We decided to split response between, for example:

/node/fetch/your_device_name # SECRET ARE NOT HIDDEN

and

/nodes # PLAIN ENABLE PASS HIDDEN IN HTTP RESPONSE
/node/show/your_device_name # PLAIN ENABLE PASS HIDDEN IN HTTP RESPONSE

If you want to remove secret hashes from backuped configuration too, you need to set :remove_secret in var like @ytti said earlier in this thread.

You can check code on this file: https://github.com/ytti/oxidized/blob/979e6e7260e66c8fecabd6f424e166ba1193f54c/lib/oxidized/model/model.rb at line 87.

I didn't test remove_secret but setting this in oxidized config can suit for your case.

Hope that can help you,

Tib

PS: If you think I didn't understand what you said previously, coyy/paste response of what is wrong for you (with personnal/business stuffs hidden of course).

[davama]

That is much clearer, thanks.

If you want to remove secret hashes from backuped configuration too, you need to set :remove_secret in var like @ytti said earlier in this thread.

this is what i did:
rest_hide_enable: true
#vars: {}
vars: {remove_secret}

you speak about backuped configuration, reachable via /node/fetch/your_device_name ?

Yes, that's what i was referring to. The same line appears in the git repo too.

When i read your comment on

PLAIN ENABLE PASS HIDDEN

I think it dawned on me that maybe you were discussing the actual "enable password" line rather than the local username account line.
Either way i created an enable password to test but both the 'enable pass' and 'username acc' lines still comes up on the /node/fetch/device_name page, as it was designed (if i understand it correctly). But it also appears in the git repo (which im assuming is the backuped config)

Im assuming that if the backuped config secrets are removed then the /nodes/fetch/device_name page will also be removed. Is that correct?
Can both be removed?

Hope it's clear.
Thank you for the continued support.

Dave

[Tibshot]

Yeah, actually, /node/fetch/device_name page is not managed with rest_hide_enable.

But remove_secret in vars normally removes stuff from backuped config showed on /node/fetch/device_name page.

"Im assuming that if the backuped config secrets are removed then the /nodes/fetch/device_name page will also be removed. Is that correct? " -> Yes, /nodes/fetch/device_name page points to your backend storage.

FYI, devices managed with remove_secret are listed here: https://github.com/ytti/oxidized/search?p=1&q=secret

About your configuration, I would have put:

rest_hide_enable: true
vars: 
  remove_secret: true

My equipment aren't 'remove_secret' aware so I can't help you to debug more :(

Tib

[davama]

Thank you for the link. It was very useful. Sorry but this is going to be fairly long, so please bear with me. (hope it's useful to you or someone else like me)

Yeah, actually, /node/fetch/device_name page is not managed with rest_hide_enable.

So what does rest_hide_enable actually do? Do i really need it?

seeing the devices with remove_secret link, i made some modifications to suit our needs. I preface that im no Ruby expert so suggestions as to how to better do it are very welcomed.

ciscosmb.rb; added the following:

cfg.gsub! /^(username (\S+) password (\S+)) (\S+)./, '\1 '
cfg.gsub! /^(encrypted radius-server key)./, '\1 '

As you can see everything stays but just the 'password/key' itself is removed.
Output looks as so:

username manager password encrypted
encrypted radius-server key

ios.rb; added/edited the following:

#cfg.gsub! /username (\S+) privilege (\d+) (\S+)./, ''
cfg.gsub! /^(username (\S+) privilege (\d+) (\S+) (\d+))./, '\1 '
cfg.gsub! /^(radius-server key)./, '\1 '
cfg.gsub! /(pre-shared-key (\S+) (\d+))./, '\1 '

Output looks as so:

username manager privilege 15 secret 5
radius-server key
pre-shared-key local 6
pre-shared-key remote 6

procurve.rb; added/edited the following:

#cfg.gsub! /^(radius-server host)./, '\1 '
cfg.gsub! /^(radius-server key)./, '\1 '
cfg.gsub! /^(password (\S+) user-name (\S+) (\S+))\n .*/, '\1 '

We didn't see a need to hide the actual radius-server hosts, so we commented.
Interestingly our output on the password part was in 2 lines. (HP always has to be special) Thats why you see the '\n' in the code.

Output is as so:

password manager user-name "manager" sha1
radius-server key

Noticed that nxos.rb was not yet supported. So i just added the 'secret' block:

cmd :secret do |cfg|
cfg.gsub! /^(snmp-server community)./, '\1 '
cfg.gsub! /^(snmp-server user (\S+) (\S+) auth (\S+)) (\S+) (priv) (\S+)/, '\1 '
cfg.gsub! /^(username \S+ password \d) (\S+)/, '\1 '
cfg.gsub! /^(radius-server key)./, '\1 '
cfg
end

output:

username manager password 5 role vdc-admin
snmp-server user manager vdc-admin auth md5 localizedkey

Sorry again for the long post. This is working very good!

Any ETA as to when this PR will be added to the next version of oxidized?

Thanks again,
Dave

[Tibshot]

You're welcome. :)

rest_hide_enable just put 'HIDDEN' in HTTP result if you specified plain enable password in source var_maps like:

# part of oxidized configuration file
source:
  default: csv
  csv:
    file: "/etc/oxidized/router.db"
    delimiter: !ruby/regexp /:/
    map:
      name: 0
      model: 1
      username: 2
      password: 3
    vars_map:
      enable: 4

rest_hide_enable DOESN'T remove/rewrite anything in node informations, it just hide any plain password used to connect to equipments on web interface and json responses. :)

I guess you can need it if you want to hide enable password (if you are using var_maps like my previous yaml example) from users who could have access to oxidized web interface.

Tib

[FlorianHeigl]

"Any ETA as to when this PR will be added to the next version of oxidized?"

since it's been 5 years since that question, would it be possible to just give a glance and sign this off in favour of progress?

[robertcheramy]

I've read this PR and the corresponding one in oxidized.

We also have feature requests (#211, #203, #223) needing configuration variables for oxidized-web.

I'm not sure yet that I want to use the oxidized configuration for this.
What I am sure is that if we use the oxidized configuration, the variables have to be grouped together (under rest or under a new name (oxidized-web) for backward compatibility).

[robertcheramy]

I'm not sure yet that I want to use the oxidized configuration for this. What I am sure is that if we use the oxidized configuration, the variables have to be grouped together (under rest or under a new name (oxidized-web) for backward compatibility).

After some thinking, I would like to use the central configuration file (~/.config/oxidized/config) for every static settings (oxidized + oxidized-web). If oxidized-web at some point needs dynamic settings (user1 wants dark theme, user2 wants light theme), this must be handled by oxidized-web, not oxidized. A static setting in oxidized would be the path to the database where such dynamic settings are stored.

I would like to define (and document) the configuration settings inside oxidized-web and just tell oxidized to load the oxidized-web plugin. This opens the possibility to add more plugins from outside the core oxidized code.

The yaml file could look like this:

plugins:
  load:
    - oxidized-web
  oxidized-web:
    listen: 127.0.0.1:8888
    theme: dark
    hide_options:
      - enable
      - password
      - ip

This is a big change and I'd like to fix Issues and release a major version first.

=> I'll open an Issue for this (Edit: #264)
=> This PR will need to be rewritten from scratch. I'm letting it open for now.