Update admin token validation so it can be hashed instead of clear

Feature Requests
1

From looking at the current code, there doesn’t appear to be any legit reason for this to be in environment/config in cleartext.

Seems like changing this:

fn _validate_token(token: &str) -> bool {
    match CONFIG.admin_token().as_ref() {
        None => false,
        Some(t) => crate::crypto::ct_eq(t.trim(), token.trim()),
    }
}

to something including something close to this: (at least as a first check - to support both):

    crypto::verify_password_hash(
        token.trim().as_bytes(),
        t.trim(), # assuming the salt will handle passing in actual value like crypt() c lib function
        t.trim(),
        self.password_iterations as u32,
    )

Intent would be that it would get the cleartext token out of the environment/config/etc. I’d even suggest that the container log - if presented with a cleartext token, should output a “you can change to this for more security”.

Certainly anyone with access to the host could directly access DB, but if properly configured, that wouldn’t be an externally accessible exposure.

The other side of this - I’d like to have the admin token usable, but only provide to certain people for remote access - that may not include all of the people with system admin permission.

1 Like
2

Well, we can’t just update this, since that would break all current configurations.
Also, your argument of trying to keep-out other sys-admins which have full access to the systems will break, since they would probably be able to change that hashed token, login, do stuff, change it back.

If you don’t want it to be in the -e or .env, you can also use docker secrets with compose v3:

Though, we may want to add some more/alternative authentication methods for the admin interface in the future.

3

You wouldn’t have to break anything additional - just do a “If it is in as a hash, check as hash, otherwise check as exact match.” Could even be a separate env var in that case ADMIN_TOKEN_HASH vs ADMIN_TOKEN.

Not really worried about the ‘admin actually going in and doing it - since that admin could also just change the code if they wanted to’ – more the ‘I want to have the (other team) generate and provide the hash, to which the admins don’t know the password and aren’t exposed’. Right now, I can’t give that other team ability to have the admin “password” without that password being directly visible to the admins.

Future goal of being able to flag certain users as ‘admin section authorized’ and using the admin token solely for initial setup – would be preferred, but that falls in the ‘more/alternative’ comment you make above.

4

I do not understand it actually.
You do not want an someone how can login into the admin-interface to see the password, he used to login with, visible in the admin-interface?

Or, you have some sys-admin who needs to setup bitwarden_rs for an other team but you do not want that sys-admin to know the admin-interface password?

Seems very complex to me. Maybe better to just have one bitwarden_rs instance, and just create multiple organizations? I kinda miss the use case here.

Besides that, i do understand the request for a more safer way to login. I have basic auth infront of the /admin path for example.

5

It’s the ‘sys admin setting it up, but doesn’t need to know the admin token’ case. Just seemed like this was a case where there wasn’t really any need for that token to be in cleartext in the config - unlike the database credentials for example, which obviously have to be known.

I do like the idea of the /admin path being independently protected, maybe that’s the simpler approach, and easy to do with the proxy.

6

Yea, that is just a nice extra safeguard.

Though, your request for a different method is not on its own. There are some other requests regarding this, but it’s not very high on our list since we mostly try to keep compatible with upstream and fixing bugs when we can.