Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

triggerUri exposes plain text Blue Iris Username and Password #371

Closed
TonyBrobston opened this issue Sep 8, 2020 · 20 comments · Fixed by #378, #383, #387 or #390
Closed

triggerUri exposes plain text Blue Iris Username and Password #371

TonyBrobston opened this issue Sep 8, 2020 · 20 comments · Fixed by #378, #383, #387 or #390
Labels
enhancement New feature or request

Comments

@TonyBrobston
Copy link
Contributor

TonyBrobston commented Sep 8, 2020

Is your feature request related to a problem? Please describe.
When adding a triggerUri for a Blue Iris system that requires authentication, my user and pw are in a file in plain text.

Describe the solution you'd like
Preferably this would be a secret, environment variable, or in a file of key/value pairs that get's ignored when a user backs up their configuration (similar to home assistant: https://www.home-assistant.io/docs/configuration/secrets/).

The best solution in my mind would be to take mustache templating further. For example in the docs there is:

"http://192.168.1.100:81/admin?trigger&camera=FrontDoorHD&user=username&pw=password"

This could become:

"http://192.168.1.100:81/admin?trigger&camera=FrontDoorHD&user={blueIris.username}&pw={blueIris.password}"

This could look inside settings.json. Then alternatively settings.json could use mustache templating to look to an environment variable or secret or key/value pair file; which would be a benefit to all the other settings that have a username/password. Then the mustache templating in triggerUri could consistently look to settings.json. Something like:

{
  "blueIris": {
    "username": "{ENV.BLUE_IRIS_USERNAME}",
    "password": "{ENV.BLUE_IRIS_PASSWORD}"
  }
}

...or possibly SECRET.BLUE_IRIS_PASSWORD or something along those lines.

Describe alternatives you've considered
I described a couple alternatives above.

Additional context
I think the main problem here is the need to separate configuration from secrets. If my server dies, how do I get my configuration back to get up and running again; ideally it's backed up, but it shouldn't be backed up with plain text secrets. With my other docker containers I have written a shell script that grabs usernames/passwords from LastPass using their cli and then uses sed to drop them in .env files and then I reference those .env files in my docker-compose (https://docs.docker.com/compose/environment-variables/#the-env_file-configuration-option).

ALSO, I love node/typescript and can probably provide some assistance in this implementation.

ALSO ALSO, thanks to everyone that has contributed to this project. I absolutely love it!

@neilenns
Copy link
Owner

neilenns commented Sep 8, 2020

That's a pretty solid idea. The system already uses Docker secrets to load settings.json and triggers.json. Conceptually what you describe makes a ton of sense: have the username/password in a secrets file somewhere then mustache template it.

By all means if you want to take a crack at coding this up feel free. Before opening a PR I suggest drafting up a plan for where you'll pull the username/password from. I'm not entirely sure the best place to be honest. Should it be in settings.json (easy to add)? Or a dedicated new secrets.json (requires a lot more boilerplate/copying of code)? Can people specify more than one if they need it for different services? What about things like the various IDs used for pushbullet etc. which are kinda like passwords too?

Here is where the mustache template is set up:

https://github.com/danecreekphotography/node-deepstackai-trigger/blob/acb23278148d2e18b7f8aaa674e7385578595b28/src/MustacheFormatter.ts#L24

You'd need to add the username and password there, and make use of the optionallyEncode method to make sure the URLEncoding is done properly. I wouldn't use "blueiris" anywhere, however, as webRequest is generic and used for more than just BlueIris NVRs.

@neilenns
Copy link
Owner

neilenns commented Sep 8, 2020

Here's a link to the contribution guide that gives some info on how to get up and running using VSCode dev containers: https://github.com/danecreekphotography/node-deepstackai-trigger/blob/main/CONTRIBUTING.md. You'll probably hit some hiccups, the dev containers can be a smidge touchy. If you decide to give it a go and have any issues getting the first F5 going let me know.

@neilenns neilenns added the enhancement New feature or request label Sep 8, 2020
@neilenns
Copy link
Owner

neilenns commented Sep 8, 2020

One other thought: environment variables that then populate into settings.json seems like overkill. The whole point to Docker secrets is the files themselves can have the secret in them and then you just don't back up the file. To make that really work as Docker expects it's probably best to split them out to a file that isn't settings.json, so settings.json can still get checked in but a separate secrets file holds the actual secrets and doesn't get checked in.

@TonyBrobston
Copy link
Contributor Author

@danecreekphotography Thanks for the response. I gave this all a read and I think we're mostly on the same page. When I have some time to dig in further I will; I'm not sure when that will be, hopefully the next few days.

@neilenns
Copy link
Owner

neilenns commented Sep 8, 2020

I'll just leave this sit for a bit, you're welcome to poke around and take your time. If you have questions just comment back here and I'll do my best to answer.

@TonyBrobston
Copy link
Contributor Author

@danecreekphotography Just wanted to pop in to mention that this is still on my mind and something I am planning to contribute on, assuming everything works out; I haven't looked into the implementation details much yet, I've been pretty busy with other things in life. Feel free to assign this to me, I don't think I'm able to.

Thinking a bit more into this, just wanted to add my thoughts and get some feedback from you.

  1. Docker secrets seem like the way to go, especially since that pattern is currently being used for other things.
  2. It would be ideal if this was generic enough that secrets could be a list of key/value pairs and those secrets could be consumed everywhere (initially I picture settings.json and triggerUri's). Seems like secrets are key/value pairs. When consuming the secrets I picture something like:
"http://192.168.1.100:81/admin?trigger&camera=FrontDoorHD&user={SECRET.BLUE_IRIS_USERNAME}&pw={SECRET.BLUE_IRIS_PASSWORD}"

with some sort of secret file, I'm not sure if this would be a .txt file or .yaml or something else:

BLUE_IRIS_USERNAME: "someUsernameHere"
BLUE_IRIS_PASSWORD: "somePasswordHere"

I also think it is necessary to allow secrets in settings.json and potentially other places. This would allow us to separate other secrets for MQTT, Pushbullet, etc.

If this seems ok to you, I'll start looking more specifically into implementation details and the information you mentioned in previous comments. I also believe strongly in TDD, so I plan to do all of this with a test-first approach.

@neilenns
Copy link
Owner

neilenns commented Sep 27, 2020

No rush, whenever you get to it.

For the secret file please use json as the format as it's the format used for all other configurations in the system. You could do something as simple as:

{
   "blueIrisUsername": "username",
   "blueIrisPassword": "password"
}

You'll need to decide where you want them to work. Each place where they get used will have to go through the mustache formatter and it's probably overkill to try and make it work for every property and a user-defined list of values.

I suggest sticking to ones that are likely to have secrets in them rather than trying to make fully generic: the URLs and the various API keys for the different services. This means you can include a json schema for the secrets file, validate against it at runtime, and also have Intellisense for end users during editing.

jest is already integrated into the project so you should have no trouble doing TDD.

@TonyBrobston
Copy link
Contributor Author

@danecreekphotography I created a PR to add tests where I plan to modify code. #378

@neilenns
Copy link
Owner

neilenns commented Oct 9, 2020

I added a couple of super minor things, take a peek and let me know what you think. Then I can merge it.

@neilenns neilenns reopened this Oct 11, 2020
@neilenns
Copy link
Owner

Re-opening, was auto-closed by github on merge.

@neilenns
Copy link
Owner

When you do your next PR make sure you pull in my change for #380 from main.

@TonyBrobston
Copy link
Contributor Author

@danecreekphotography Am I right in thinking that settings.json will always have some settings? In other words, {} is invalid? It seems like at a minimum deepstackUri has to be set? ...or is there a default?

@neilenns
Copy link
Owner

I had to go double-check the settings schema, it's been a while. Yes, deepstackUri is required and that is the only required property.

https://github.com/danecreekphotography/node-deepstackai-trigger/blob/771e1a955dc5eaf9c716e384465107498224f102/src/schemas/settings.schema.json#L9

@TonyBrobston
Copy link
Contributor Author

@danecreekphotography This is still a work in progress. But this is where I'm at, at the moment. #383

Happy path is working, I need to consider a few more edge cases in helper. I removed some error logic, mainly the errors that would come back from JSONC.parse. I think it's worth discussing a different approach. I think JSONC is doing a great job of fixing broken json; so maybe it's better to log.verbose the settings right before the mustache render? I think this would clear up any confusion around what settings are actually used.

@neilenns
Copy link
Owner

I'm always up for logging more stuff in verbose mode. Since there's no UI to this thing it's only the logs that saves people when there are issues.

@neilenns
Copy link
Owner

Merged to the issue371 branch and generated a Docker. @TonyBrobston give danecreekphotography/node-deepstackai-trigger:issue371 a go and let me know how it works. I'll do the same on my end.

@schford
Copy link

schford commented Oct 31, 2020

Why do you even need username and password - just setup Blue Iris to not need authentication from the local lan.

In Blue Iris - go to settings then Web Server then Advanced, in there change Authentication to Require from Non-LAN only. - then limit access by IP Address to what ever - in my example it is ^192.168.0.*

@TonyBrobston
Copy link
Contributor Author

@schford With security it is recommended to have many layers of security. Access to my network is one layer of security, requiring username/password to Blue Iris is another layer. If it's possible to have more layers, I will.

@neilenns
Copy link
Owner

@schford This isn't specific to BlueIris username/password. The API keys and whatnot for the other notification methods are required and considered secrets as well.

@neilenns
Copy link
Owner

neilenns commented Nov 6, 2020

Merged and released as v5.5.0. The latest image is currently building and should be available within the next 10 minutes. Thank you again for the great contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment