Skip to content

SOLR-15771: bin/solr auth enable should model best practices for security.json#1851

Merged
epugh merged 25 commits intoapache:mainfrom
epugh:SOLR-15771
Sep 12, 2023
Merged

SOLR-15771: bin/solr auth enable should model best practices for security.json#1851
epugh merged 25 commits intoapache:mainfrom
epugh:SOLR-15771

Conversation

@epugh
Copy link
Copy Markdown
Contributor

@epugh epugh commented Aug 18, 2023

https://issues.apache.org/jira/browse/SOLR-15771

Description

Set up a properly configured security.json with multiple levels of authentication demonstrating a good place to start.

Solution

have a barebones security.json in Solr that is then modified by the bin/solr auth tool

Tests

bats

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

epugh added 2 commits August 18, 2023 07:37
Have a stock security.json that is shipped with Solr and used by the auth tool
@epugh epugh requested a review from janhoy August 18, 2023 11:40
@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 18, 2023

I’m struggling to get pathing to work to include solr/core/src/resources/security.json to load in the ref guide, maybe someone has some ideas?

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 18, 2023

Okay, I thought that the Antora "Collector Extension" would somehow give me a reference to solr/core/src/resources/security.json so I could include it in the docs, but I never quite got it to work. So at this point I just checked in a copy of security.json into the /examples directory. One nice thing, I was able to update Antora to the latest with no issues... Maybe someone could look at this and have a better idea?

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 18, 2023

Aside from the Antora stuff, this is ready for review and potential commit. I think this COULD be backported to 9x.

Copy link
Copy Markdown
Contributor

@janhoy janhoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments inline

@janhoy
Copy link
Copy Markdown
Contributor

janhoy commented Aug 22, 2023

Also, there should be some agreement between security-dashboard screen in Admin UI and this default security.json. I.e. if you bootstrap a cluster with this default, it should not trigger any of the "best practice" warnings in the security dashboard. Have not checked to see if that is the case. But the UI has some nice recommendations built-in.

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 22, 2023

@janhoy could I talk you into pushing up a commit to the security.json that implements that changes you mentioned? For all that I try to work on security stuff, I don't actually grok it too well, (part of why I like this model best practices so I never have to think about it again!).... I;d love those changes you suggested!

Signed-off-by: Jan Høydahl <janhoy@users.noreply.github.com>
@janhoy
Copy link
Copy Markdown
Contributor

janhoy commented Aug 22, 2023

I compiled my suggested changes as a PR to your branch: epugh#5 with these changes:

  • Renames role user as search
  • Adds role index which can do /update
  • Require role superadmin for security-edit
  • Set blockUnknown: false
  • Open up /solr/admin/info/health, /api/node/health, /solr/admin/metrics to the world
  • User search can only search (and do clusterstatus), user index can do update and search, user admin can do all admin commands, update and search, and user superadmin has all roles.
  • Changed password for each user to same as username
  • Removed unnecessary "index" json keys from sample

See screenshot for how it looks in security screen - no warnings there.
securityjson-screenshot

Worth to note that the search and index users now lacks permissions that would be required to use the Admin UI (config-read, collection-admin-read etc), so the UI is useless for other users than admin. Perhaps we should change that and let both search and index users get access to read those configs so they can use the Admin UI?

As an alternative to predefining all users, we could predefine only the "superadmin" user and generate a strong random password for it that would be printed on the CLI. Then document how this user now can enter the "security" screen of admin UI to create more users, and assign them one or more of the predefined four roles.

Suggested update to default security json
@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 22, 2023

My assumption is that "search" and "index" belong primarily to machines, i.e your ETL pipeline and your search API, so wouldn't use the admin UI anyway.... And might even have passwords defined that aren't really reachable by humans!

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 22, 2023

also, I think if we want to have really strong security, I think at that point, we are sort of suggesting that it happens at start up... (Someday I hope to follow the Rails pattern of having environment variables that are global, but also overridden for dev, test, and production so that you might have strong security in prod, but minimal or none in dev).

@janhoy
Copy link
Copy Markdown
Contributor

janhoy commented Aug 22, 2023

Re. syncing - could we link to the file on github instead of displaying it verbatim in ref-guide?

This is a tiny step towards secure-by-default, making it easy to get a production grade security config up and running.

PS: Crave is not happy, perhaps need to squash, rebase and force-push.

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 22, 2023

Do we have other examples of linking to Github in teh ref guide? I could definitly link to just the main branch of it.

And yeah, generally I find Crave hates me after a couple of commits, and I just ignore it. Since "squash, rebase and force-push" feels very risky and when I do things like that I get urgent IM's from Houston from mucking up the source tree... ;-)

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 22, 2023

In Solr it is common to configure settings in https://github.com/apache/solr/blob/main/solr/bin/solr.in.sh[solr.in.sh]

From solr-in-docker.adoc...

@HoustonPutman
Copy link
Copy Markdown
Contributor

Do we have other examples of linking to Github in teh ref guide? I could definitly link to just the main branch of it.

We should link to the correct branch. If the capability isn't there already, I'll add it to your PR.

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 22, 2023

Thanks @HoustonPutman !

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 22, 2023

Now I am getting excited about this PR ;-). Better Auth defaults and Better Ref Guide ;-)

((ObjectNode) credentialsNode)
.put(username, Sha256AuthenticationProvider.getSaltedHashedValue(password));
JsonNode userRoleNode = securityJson1.get("authorization").get("user-role");
((ObjectNode) userRoleNode).put(username, "admin");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it documented somewhere that the --credentials myUser:myPass arg of the CLI will be an admin role user, and what that means? Should it instead define the "superadmin" user?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I think, that if you are able to run bin/solr auth, then you probably are a "superadmin" and we should set you to that role, and document it in the Ref Guide. I wonder if a feedback message "User X,Y has been granted superadmin permissions" would be useful.....

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we could do stuff like:

bin/solr auth enable --credentials root:xxxx  --type basic # superuser creds
bin/solr auth adduser --credentials jane:xxxx --role admin # admin human user
bin/solr auth adduser --credentials frontend:xxxx --role search # search machine user
bin/solr auth adduser --credentials indexer:xxxx --role index # indexer machine user
bin/solr auth deluser jane

Then the first "enable" command simply adds the root user. And the adduser commands will add new users with predefined roles. And the script can make sure that an admin user also has index and search roles etc.

It's not crucial to have since we have a nice AdminUI for adding users, but sounds like a win for those who want to script a simple setup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea.... So, at some point I am hoping that as the v2 API's come online, we move the CLI to using them. And that at that point adding a command like "adduser" or "deleteuser" becomes super easy because we have nice clean SolrJ client objects (methods?) that map to the v2 apis with all the docs, and that we don't add more custom logic on the auth tool....

{
"authentication": {
"blockUnknown": false,
"class": "solr.BasicAuthPlugin",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about type=kerberos?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Humm.. My somewhat snarky response is "What is this Kerberos thing you speak of"? So, the auth tool didn't support that previously, so I never thought about it. Maybe for a seperate ticket? I wonder if we need to support "point at a existing security.json"? Or, do we add another param to the command?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never used it. Looks like you'll configure your entire KRB environment in other files and sysprops, and then at the end you create security.json with little more than the "class" property. When creating user:role mapping, the username needs to match the kerberos principal, so I suppose that a user could be mapped to role here too, although it would not be adduser, but perhaps mapuser?

Probably best to document that for kerberos we only support initial bootstrap of security.json, no user:role mapping, which can be done in the UI (?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we know so little about KRB, we already have the text "Currently, this script only enables Basic Authentication, and is only available when using SolrCloud mode" in the ref guide, which I think covers the concern. I don't know anything about KRB, and don't think I ever will!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, we have a -type command... so yes, we do need to update the ref guide...

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 29, 2023

@HoustonPutman this PR is getting close to be ready to merge, wanted to bug you on the ref guide "branch" magic links!

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 29, 2023

Ugh... test_ssl.bats failing.. thought it was ssl, but running not in ssl, I can create a collection with my user:password, but I can't query it!!!

 curl -u name:password 'http://localhost:8983/solr/test/select?q=*:*' 
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
<title>Error 403 Unauthorized request, Response code: 403</title>
</head>
<body><h2>HTTP ERROR 403 Unauthorized request, Response code: 403</h2>
<table>
<tr><th>URI:</th><td>/solr/test/select</td></tr>
<tr><th>STATUS:</th><td>403</td></tr>
<tr><th>MESSAGE:</th><td>Unauthorized request, Response code: 403</td></tr>
<tr><th>SERVLET:</th><td>default</td></tr>
</table>

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Aug 30, 2023

Okay, I didn't realize the superadmin doesn't extend admin extend search/index, instead you have to assign all of them...

I wonder if we should map alll roles to superadmin, some roles to admin, just a few to index and search? so that if you have just "superadmin" role, but not admin etc, then it works the way you would expect? @janhoy ???

@janhoy
Copy link
Copy Markdown
Contributor

janhoy commented Aug 30, 2023

I wonder if we should map alll roles to superadmin

That is absolutely an option. Not as flexible but fairly logical and easier to grok.

@epugh
Copy link
Copy Markdown
Contributor Author

epugh commented Sep 9, 2023

Ugh..... so I went to do the mapping of explicit individual permissions per Role, and wihtout the abilityt o comment in JSON, it started feeling VERY error prone.... I mulled a bit more on it, but no joy. So.. I think I'm going to call this done.

I know right now the ref guide link is to the main repo of Github and I tried to chagne that a bit but no luck... @HoustonPutman if you are going to get to that, let me know, but I'm thinking maybe that could be it's own ticket and we could merge this? I want to start looking at basic auth for the CLI, and having this in there would help with testing...

@epugh epugh merged commit b163a0e into apache:main Sep 12, 2023
epugh added a commit that referenced this pull request Sep 12, 2023
…rity.json (#1851)

Introduce a security.json that defines four roles: `search`, `index`, `admin` and `superadmin`, and assigns the `superadmin` to the user created by running bin/solr auth.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants