oxidecomputer / oxidecomputer/omicron

Wrap console session tokens and access tokens in `secrect::SecretString` or similar

Open
#8,018 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

authn customer enhancement Good for new hires security
Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Description

What

The console session token should be in a type that will zeroize the memory when it is dropped, so that it isn't at risk of accidental exposure due to something like Heartbleed, and that will obscure derived Debug impls, so it isn't accidentally logged. The secrecy crate provides a type to make handling a little easier, which is already a dependency in omicron, though it's not a hard requirement in case something else is desired.

https://github.com/oxidecomputer/omicron/blob/5cfd73568a0a60a609f572b03efa86b38f5f642b/nexus/src/app/session.rs#L24

https://github.com/oxidecomputer/omicron/blob/74a5c0c4a8293961c364292f4089063387cb4a88/nexus/db-model/src/device_auth.rs#L61

Notes

A couple things I noticed:

  1. the public ConsoleSession type can't exactly just swap token: String for token: SecretString, because the latter can't be used for deriving diesel::Insertable.
  2. ConsoleSession is used for other things that the database insert, such as session update. I presume that only the update operation would need a type that has the actual session token secret, while other operations such as update would only need to reference a session via a non-secret identified, which was brought up in https://github.com/oxidecomputer/omicron/issues/7818.

https://github.com/oxidecomputer/omicron/blob/74a5c0c4a8293961c364292f4089063387cb4a88/nexus/db-model/src/console_session.rs#L11-L18

I imagine the above is true for the access tokens as well.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with nexus/src/app/session.rs, nexus/db-model/src/device_auth.rs, and nexus/db-model/src/console_session.rs, then read issue 7818 for the session identity discussion. Trace the session and access-token database and update operations, and determine how secret-bearing values can avoid accidental Debug exposure while remaining compatible with persistence. Done means both token types have safe handling without breaking their existing uses.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
authentication, security
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.