Skip to content

Conversation

@fitzthum
Copy link
Member

@fitzthum fitzthum commented Nov 3, 2025

Requiring an admin to provide a signed JWT is a bit clunky. Instead, let's let them login with a username and password.

To facilitate this, introduce a new kbs endpoint where an admin can provide their credentials and receive a token. This endpoint can support multiple login methods in the future, but for now introduce a simple password backend.

Now with the KBS-Client, you can do the following:

$ ./kbs-client admin-login default_admin LYfmThesp8rGba
Login Succeeded
$ ./kbs-client config get-reference-values
"{}"

This PR updates the docker compose config to use the password backend by default, but it doesn't change any of the other configs (to avoid breaking various flows).

If the password backend is used but no personas are specified, one user will automatically be added. The username be default_admin and the password will be randomly generated and written to the KBS log at startup. This seems like a good way to balance ease-of-use and security.

@fitzthum fitzthum requested a review from a team as a code owner November 3, 2025 22:22
@fitzthum
Copy link
Member Author

fitzthum commented Nov 3, 2025

Will fix tests tomorrow.

@fitzthum fitzthum force-pushed the admin-2 branch 10 times, most recently from 88e120a to f8f40be Compare November 4, 2025 19:37
@fitzthum
Copy link
Member Author

fitzthum commented Nov 4, 2025

Tests passing PTAL @confidential-containers/trustee-maintainers

I have two more PRs coming after this. 1) OAuth2 support 2) Adding regex-based allowed paths to admin personas

Copy link
Contributor

@mkulke mkulke left a comment

Choose a reason for hiding this comment

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

would a random password per-process harmonize with kubernetes deployments? (replications, etc)

@fitzthum
Copy link
Member Author

fitzthum commented Nov 5, 2025

would a random password per-process harmonize with kubernetes deployments? (replications, etc)

This is just the default for the most basic case where nothing is configured. There are a few options for handling multiple Trustee instances. First, define a persona with a password in the config file and share the config file between instances (as you normally would). Now this persona can login at any of the instances. Second, the password config allows you to specify a token key pair from a file. This is the key pair that signs and validates the token. So if you give all your instances the same one, a token from one instance can be used with any other instances.

Finally, in future PRs we will support external login services, all the instances could share one OAuth2 service or something similar.

@Xynnn007
Copy link
Member

Xynnn007 commented Nov 6, 2025

imo @mkulke is concerned that randomly generated passwords might render KBS non-stateless.

Typically, we store login passwords in a database to ensure the KBS code logic is stateless. Different KBS instance can connect to a same DB instance.

Ideally, the code logic of backend for password-based login should connect to the given database to retrieve the username and corresponding password.

I will continue to release pull requests to replace the key-value cache in different places and add a Postgres database to Docker Compose and Kubernetes, which will help you to achieve this goal. However, I'll be quite busy these next two weeks, so PRs might be slower, but I can help to review the remaining parts first.

Does this sound good to you?

@fitzthum
Copy link
Member Author

fitzthum commented Nov 6, 2025

Typically, we store login passwords in a database to ensure the KBS code logic is stateless. Different KBS instance can connect to a same DB instance.

This PR is not stateful. Let's be clear, the password generation thing is only a simple default for admin that don't want to define any personas. If that's too confusing we can drop the feature, but I think it's handy for docker-compose.

There is no support for adding personas or updating passwords at runtime. Personas must be statically configured in the config file. This is no dependency on key-value storage. Maybe in the future we could add an admin endpoint for configuring personas. This would require storage. We can think about this in future PRs.

Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

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

Some first comments for the work

ParseAuthHeaderFailed(#[from] actix_web::error::ParseError),

#[error("Failed to parse admin login request")]
ParseAdminLoginFailed(#[from] serde_json::Error),
Copy link
Member

Choose a reason for hiding this comment

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

This error enum is not related to this commit(3c97a83)?

Copy link
Member Author

Choose a reason for hiding this comment

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

moved to next commit. i think the idea was that this was a generic error that other impls can use

/// The number of hours that an admin token will be valid for.
/// After the token expires, an admin must login again and receive
/// a new token.
pub admin_token_life_hours: u64,
Copy link
Member

Choose a reason for hiding this comment

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

Let's set this to seconds than hours for a better grained.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seconds seems too small. How about minutes? I am thinking that people probably won't want to login in very often; maybe every day or two.

Copy link
Member

@Xynnn007 Xynnn007 Nov 8, 2025

Choose a reason for hiding this comment

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

Yes seconds are small. What matters here is grain. No matter minutes or seconds the user can configure a big number. I am ok with minutes.

/// to allow Trustee to restart with invalidating existing tokens.
/// The key pair should be in PEM format.
/// If no path is provided, a random key pair will be generated.
pub key_pair_path: Option<PathBuf>,
Copy link
Member

Choose a reason for hiding this comment

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

If this field is not used, let's delete it. Also, if we want to use the key pair, we can set pri and pub key separately with path.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is used in the commit that adds support for the key_pair_path. I can move if it's too confusing.

}

// TODO read key pair from file
let key_pair = Ed25519KeyPair::generate();
Copy link
Member

Choose a reason for hiding this comment

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

This is what I meant. We can't prevent users from executing the built-in random generation logic during deployment. The best approach is to decouple this logic from the rest of the code, since the keypair is a stateful attribute, and it would be better to add it to the database or some other location.

We can have this initial logic in the db setting up script.

Copy link
Member Author

Choose a reason for hiding this comment

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

See next commits. This is only done if you don't specify a keypair from a file.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Xynnn007 would you mind elaborating your objection? I don't really see too much of a problem with this. If I'm understanding correctly the randomly generated key pair only means that auth tokens can't be shared between KBS instances. I wouldn't see the disabled functionality as fundamental (in fact I can imagine this could possibly even be desired) and it certainly doesn't look like a security problem to me either.

Copy link
Member

@Xynnn007 Xynnn007 Nov 21, 2025

Choose a reason for hiding this comment

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

Oh I am not against of a random key. I prefer a KBS core logic would better be stateless. Thus the key-generation logic would better to be outside of KBS logic. It will help to deploy multiple instances of KBS to serve as HA

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sure we all agree that statelessness and HA are important goals for KBS so the decision here seems to be more about whether the KBS needs to be stateless by default or not.

Apart from what I said before (i.e. I can imagine KBS deployments with a single KBS instance which don't care much about statelessness), I think it could also be said that by not inserting a key into the config file, the admin effectively states that they don't consider the key to be part of state. But I understand this might depend on point of view.

Copy link
Member

@Xynnn007 Xynnn007 Nov 21, 2025

Choose a reason for hiding this comment

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

Yes, the reason statelessness is needed is to decouple state and computational logic in multi-replica deployment scenarios. Whether a stateless design is appropriate, or whether multi-replica deployment should be supported, is a question whether the project should address. While I personally support this direction, we still need to discuss and reach a consensus.

A compromise way is to support both, by some specific switches in the config to forward those state things to outer services.

#[derive(Clone, Debug, Default, Deserialize, PartialEq)]
#[serde(default)]
pub struct PasswordAdminConfig {
pub personas: Vec<PasswordPersona>,
Copy link
Member

Choose a reason for hiding this comment

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

Before we get too complex in this, usually the admin panel can only be loginned by one persona, s.t. administrator.

I suggest clarifying the following questions before considering introducing multiple personas/roles:

  1. What are the specific responsibilities of each persona? Why are multiple personas needed?

  2. Will different personas affect the scope of allowed data/control plane interfaces or something else?

  3. How should the different data and control plane interfaces allowed for different roles be defined?

  4. What are some typical user stories or processes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a future PR to introduce the concept of roles for each persona. The simplest approach is to allow each persona to have a set of allowed paths defined by a regex. This allows one persona to be in charge of reference values while another handles policies or certain plugins and so on. You can also filter access to particular secrets with the regex, but this will require some other considerations.

We've had some specific requests internally for a better separation of privileges for admin. You can imagine a situation where one company has a Trustee deployment that is used my different teams. For instance, one team is in charge of building the workload images. They know what the RVs are. Another team might control access to secrets. Another team might handle compliance. Etc.

Anyway before we get to any of that, it's still a step forward to support multiple admin personas even if they don't have different privileges. This is better than having to share a private key or a password when more than one person is configuring Trustee.

usually the admin panel can only be loginned by one persona, s.t. administrator.

Even if there is just on type of admin (unlikely in the future), there are still going to be multiple people acting as that admin.

Copy link
Member

Choose a reason for hiding this comment

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

I suggest decoupling the functionality between PRs. This PR can simply introduce a single administrator account and password login method, because it's relatively clear, while you can fully implement multi-role functionality in another single PR, including:

  1. Multi-role functionality

  2. How to define the different APIs that different roles can access

  3. Multi-role documentation and usage flow

Copy link
Member Author

Choose a reason for hiding this comment

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

2,3 are left to future work already.

1 is trivial to support; just add a for loop. I put it in this PR since the new simple backend that uses client-signed tokens already supports multiple personas. I could move it to another PR, but seems reasonable to have it here. 2 and 3 are much more complicated.

Copy link
Member

Choose a reason for hiding this comment

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

Username and password login (corresponding to one administrator who can manage all APIs) is a clear and feasible feature. Including it as a whole in this PR is reasonable. Regarding multiple administrators, in the current PR, each administrator has the same permissions—all APIs, which looks somehow "redundant". You mentioned that permissions will differ in the future, so it's incomplete for now. Let's try to introduce individual sub-features in a decoupled manner. (Implicitly coupled features make reviews more difficult; later PRs would take time to review, and comparing and independently tracking the previous state each time is very time-consuming for reviewers. Independent features also make reviews easier :-) )

so please, decouple them into different PRs, if it's not a complete ability.

Copy link
Member Author

Choose a reason for hiding this comment

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

so it's incomplete for now

No it isn't. Having multiple logins for the same persona/privilege is still an improvement. This means that multiple admin do not have to share credentials, which is especially important when using a username and password. If we had username and password support but could only have one username, that would be incomplete.

I don't think it's worth the time to split this into another PR, especially since the simple admin backend already supports admin personas in exactly the same way.

Copy link
Member

Choose a reason for hiding this comment

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

This is what I meant. You've introduced a multi-role login mechanism, but there's no difference between each role. Why do we need multiple roles? Why can't all administrators use the same administrator account and password? Just like using a Windows administrator account to log in to the operating system.

We only need additional design when we have a specific problem to solve. It's unwise to design multiple roles just for the sake of a seemingly great solution without defining a specific problem and differentiating the permissions for each role. I strongly suggest introducing multiple roles along with defining the capabilities of different roles as a PR; otherwise, I cannot accept this half-finished product.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Not that it matters much but) I suspect not having personas would amount to password-sharing which, as a 90's survivor ;-), I'd consider an anti-pattern. With credentials shared among multiple people someone will leak them sooner or later and it will not be possible to find out who. It's not possible at all to find out who did what anyway since system logs only say it was done by a shared account. I don't think this is good practice for a security-sensitive environment, credentials should generally map to individual people.

For that reason I think allowing for multiple personas does add considerable value even if all personas have the same actual rights, at least initially.

Also, I suspect that the "persona" terminology might be a bit misleading here. In my experience persona is more of a concept from authorisation, rather than authentication which I believe we're dealing with here for now. Personally I'd call "account" or even just "login" what's currently called persona. A "RVSP admin" persona or a "policy admin" persona could later be assigned to these accounts. However even if everybody agrees with me on this, seeing as #1014 introduced the persona term into the simple admin auth plugin already it might be better to allow this PR to go in as is and bulk-refactor later.

#[derive(Clone, Debug, Default, Deserialize, PartialEq)]
#[serde(default)]
pub struct PasswordAdminConfig {
pub personas: Vec<PasswordPersona>,
Copy link
Member

Choose a reason for hiding this comment

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

I suggest decoupling the functionality between PRs. This PR can simply introduce a single administrator account and password login method, because it's relatively clear, while you can fully implement multi-role functionality in another single PR, including:

  1. Multi-role functionality

  2. How to define the different APIs that different roles can access

  3. Multi-role documentation and usage flow

Ok(())
}
Err(e) => {
info!("Access not granted due to: \n{}", e);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
info!("Access not granted due to: \n{}", e);
warn!("Access not granted due to: \n{}", e);

Copy link
Member Author

Choose a reason for hiding this comment

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

Should it be a warning when there is a failed login? A warning would suggest that something bad or unusual has happened, but there is nothing inherently wrong with a failed login. That said, a bunch of failed logins could suggest something bad is happening.,

Copy link
Member

Choose a reason for hiding this comment

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

Yes. During ABS and AS processes, there are already a lot of info-level logs (including RCAR request arrivals, etc.). The severity of administrator login failure events is obviously higher than that of RCAR request arrivals, otherwise it would be overwhelmed by the massive amount of info logs.

.map(PathBuf::from)
.ok_or(anyhow!("Could not find home directory."))?;

path.push(KBS_CLIENT_DIRECTORY);
Copy link
Member

Choose a reason for hiding this comment

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

I like this per-users config path.

Comment on lines +72 to +74
// If no personas are specified, create one admin persona
// with a random password.
// Print the password to the log.
Copy link
Member

Choose a reason for hiding this comment

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

I suggest that to use a fixed default password like simple "admin" or so. This would help use to do test and debug things as the logging is sometimes not that easy to catch.

Copy link
Member Author

@fitzthum fitzthum Nov 12, 2025

Choose a reason for hiding this comment

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

This would be easier to use, but it doesn't seem like a good idea to have a fixed password. I think we should move away from non-secure defaults whenever possible.

Copy link
Member

Choose a reason for hiding this comment

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

This is a design consideration. We need to envision a scenario where multiple KBS instances are deployed simultaneously in a Kubernetes cluster. Regarding the deployer's choice of password, there are two possibilities:

  1. They forgot or intentionally didn't provide a password: All services must initialize with the same default password - if each instance initializes different password, the launch will be fine, but the logining will be of great trouble as the deployers do not know which backend KBS will the load balancer lead to.

  2. They set a password: All services use the same password.

1 requires the default password to be the same. Although we don't recommend using default passwords, it's unavoidable in testing or various scenarios. And we'd better mention the default password and DONOTs in the documents.

2 requires the KBS logic to be stateless, meaning the password must be stored in shared storage, not in individual instances' memory; otherwise, inconsistencies will easily arise.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @mkulke you might be interested in this and please share some good practices upon this

Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit puzzled by this. It's pretty unorthodox to log a password in clear text in the log, usually you'd be doing the opposite and make sure secrets are not being logged (accidentally). a security scanner in your environment might flag this behaviour.

The other thing that is unusual: the secret in this case is the service's own admin password (not some secret the service requires itself to access e.g. the database). Independent of the distributed/ephemeral state question, a service should not keep the clear text of a password anywhere, usually it should be a salted, slow hash.

we're not doing anything exotic here and I would advise against rolling our own security (see the last reported vuln). Trustee is conceptually a pretty vanilla CRUD service with AuthN/AuthZ requirements. Those are solved problems that should be well-covered by prior art in established libraries, frameworks, etc.

I understand we're not there yet and we want to be pragmatic to support some development scenario until then. But why not have an --insecure-unauthorized-admin-api flag instead of custom password logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway, please give a concrete suggestion for how we should do authentication for docker compose

admittedly, I have no experience with using docker compose for anything other than in a local dev environment. So maybe this pattern makes sense for compose. someone who knows more about docker compose should probably chime in.

Copy link
Member

Choose a reason for hiding this comment

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

Independent of the distributed/ephemeral state question, a service should not keep the clear text of a password anywhere, usually it should be a salted, slow hash.

Right. The hashed value should be set into the db, than the password plaintext.

Copy link
Member Author

Choose a reason for hiding this comment

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

The goal in this PR is to add the admin session abstraction and provide a simple implementation of it that doesn't rely on any external services. We can add more sophisticated auth integrations soon. I do want to do something good for docker-compose tho. If the password in the log seems too weird I can drop that commit, but I am not sure what to do instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with the docker-compose scenario either atm so I can't speak to that.

I do agree though that printing a newly-generated passwd into log seems better than having a global default one. What I'm not sure about though is the assumption that only KBS admins will have access to KBS logs anyway. Am I wrong to think that host machine/cluster admins might well have access to KBS logs and that members of this group don't necessarily have to be also the folks authorised to make admin changes to KBS?

I think it's a good idea to follow established practices however I believe that when it comes to creating an account with login/password credentials, the established practice is basically to make the user create the account (think e.g. Linux OS installation). I don't know how useful this is to us, I don't think we have a reliable opportunity to do that anywhere in the deployment process.

So if we can't do that, the next established practice would be not to create an account I suspect. How bad would that be? If KBS launches with the password auth mode but with no accounts defined, it will still run but admin operations will be impossible, is that correct? How does this compare to printing an admin passwd into KBS log? It seems

  • KBS is launched without prior configuration
  • admin has to know they need to check the log for their password, and
  • admin needs to do so

vs

  • KBS is launched without prior configuration
  • admin gets an "access denied" when trying to admin, or perhaps is asked even earlier for credentials. Both of these will likely remind them that they haven't configured any even without knowledge of KBS, or at worst
  • admin has to know they need to create an account
  • admin has to check the KBS config file and create an account.

Is not creating an account automatically so much worse (a honest question)? It seems that in either case the admin is not able to do admin ops initially after launch and in either case they gain the ability by doing a small amount of work which is admittedly a bit greater in case of no-default-account which however seems overall much cleaner and more customary.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @pmores there are new thoughts in the issue #926 which you might be interested in

Today, we expect admin users to generate an admin token by themselves.
While the KBS client handles the complexity here, this is not a very
good flow.

Instead, let's add an admin login endpoint that will allow admin users
to authenticate and get an admin token generated by Trustee.

This endpoint will be fulfilled by the admin backend. The details of how
a user will login and the token they will receive are left to the
backend.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
The password backend lets admin login with a username and password. The
password is stored as a (salted) argon2 hash in the config file.

The username and password are checked when the admin first logs in. Then
they are given a token (with the username as the subject) that they can
use to make admin requests.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Add an admin login interface to the kbs-client lib.

Also, adjust the lib to take an admin token rather than an admin key as
input. This was the caller can provide the admin token that is returned
from the login interface.

This commit only changes the lib. The binary still compiles, but it will
not work correctly with this commit. The binary will be adjusted in a
future commit.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
#[derive(Clone, Debug, Default, Deserialize, PartialEq)]
#[serde(default)]
pub struct PasswordAdminConfig {
pub personas: Vec<PasswordPersona>,
Copy link
Member

Choose a reason for hiding this comment

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

Username and password login (corresponding to one administrator who can manage all APIs) is a clear and feasible feature. Including it as a whole in this PR is reasonable. Regarding multiple administrators, in the current PR, each administrator has the same permissions—all APIs, which looks somehow "redundant". You mentioned that permissions will differ in the future, so it's incomplete for now. Let's try to introduce individual sub-features in a decoupled manner. (Implicitly coupled features make reviews more difficult; later PRs would take time to review, and comparing and independently tracking the previous state each time is very time-consuming for reviewers. Independent features also make reviews easier :-) )

so please, decouple them into different PRs, if it's not a complete ability.

Comment on lines +72 to +74
// If no personas are specified, create one admin persona
// with a random password.
// Print the password to the log.
Copy link
Member

Choose a reason for hiding this comment

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

This is a design consideration. We need to envision a scenario where multiple KBS instances are deployed simultaneously in a Kubernetes cluster. Regarding the deployer's choice of password, there are two possibilities:

  1. They forgot or intentionally didn't provide a password: All services must initialize with the same default password - if each instance initializes different password, the launch will be fine, but the logining will be of great trouble as the deployers do not know which backend KBS will the load balancer lead to.

  2. They set a password: All services use the same password.

1 requires the default password to be the same. Although we don't recommend using default passwords, it's unavoidable in testing or various scenarios. And we'd better mention the default password and DONOTs in the documents.

2 requires the KBS logic to be stateless, meaning the password must be stored in shared storage, not in individual instances' memory; otherwise, inconsistencies will easily arise.

Ok(())
}
Err(e) => {
info!("Access not granted due to: \n{}", e);
Copy link
Member

Choose a reason for hiding this comment

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

Yes. During ABS and AS processes, there are already a lot of info-level logs (including RCAR request arrivals, etc.). The severity of administrator login failure events is obviously higher than that of RCAR request arrivals, otherwise it would be overwhelmed by the massive amount of info logs.

Let's make sure that password login for admins really works.

First, make the integration tests a little more generic so that they can
handle the simple admin backend and the password backend.

Then, add some tests using the password backend and checking various
corner cases.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Add a new command to the kbs-client binary allowing admin to login. If
login is successful, the admin token will be stored locally and used for
future admin commands.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Let's use the password admin backend for docker compose. Admin will be
required to login (which is an extra step), but once they do, they won't
have to specify the private key anywhere (which seems like an
improvement).

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
We want to balance security and ease-of-use.

To do so, let's tweak the default behavior of the password backend. If
no personas have been specified, let's create one called
`default_admin`.

Rather than using an insecure password here, let's
randomly generate a password and print it in the log.

This way an admin doesn't have to configure the admin backend at all and
we don't need to use one static password. As long as the admin can see
the Trustee logs, they can easily find the password and login.

For more complex scenarios the admin can configure the personas, which
will disable the default admin.

We already consider the Trustee logs to be sensitive, so it should be ok
to print the admin password there.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Let's explicitly set the default lifespan for the admin tokens (to 24
hours).

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
By default the jwt_simple crate allows 15 minutes of slew when checking
the expiration date of a token (and other time-based fields). This is to
avoid issues with time descrepancies.

To me 15 minutes seems excessive. Let's reduce this to 3 minutes.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
The password admin backend uses a token signing keypair to sign the
admin tokens that are granted once the password check is successful.

Allow this keypair to be loaded from a file. This allows multiple
instances of Trustee to share the same admin tokens and allows Trustee
to restart without invalidating all the tokens that have previously been
issued (assuming that the admin uses this config).

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Since the user may need to take action (login again) when the admin
token expires, make sure the error message for this case is explicit.

We don't want to surface all of the errors from token verification to
avoid any chance of information leakage. Instead, check if specifically
if the token is expired.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Update the kbs config document to include information about the new
password backend.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
The docker e2e test uses the docker compose deployment. Since we changed
the docker compose deployment to use the password backend, we need to
tweak the flow here.

Rather than setting up the admin keypair (which hasn't been needed for a
while with docker compose), we find the admin password in the KBS log
and use the admin-login interface.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
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.

4 participants