Rearchitect to remove Gin server package, add JWT-protected HTTP routes #10

LRitzdorf · 2024-07-09T18:12:59Z

This PR is a rework of the cloud-init server's high-level structure. Notably, it:

Replaces the Gin web server package with the OpenCHAMI-standard Chi package
Adds a set of endpoints under /cloud-init-secure whose access is controlled via JWT authorization
- This is to be used in conjunction with OpenCHAMI/TPM-manager, to allow secret storage in cloud-init without exposure to compute clients
When requesting the cloud-init configuration for a name, the name is initially parsed as a MAC address. If no matching MACs are found, it is treated as a hostname, and used as a lookup key into the internal datastore. This allows admins to query configuration data for a particular node without using its MAC address, and to query configuration data for a node group (which was not previously possible).
We now accept an OIDC token endpoint, rather than a single static JWT. This is used to fetch new JWTs as required throughout the service's lifetime, allowing it to run for longer than the lifetime of a single token.

See also, the updated CHANGELOG.md.

This code was originally written before OpenCHAMI services standardized on Chi. Also, Gin uses a very different Context-based interface for handler functions, and is therefore not easily interoperable with existing handlers like those used for JWT verification.

We now return a 404 error when the cloud-init datastore for the requested name is empty. In context, this means that the requested name: a) has no node-specific cloud-init data assocated with it b) is not part of any SMD groups whose data it would otherwise inherit

The actual verification isn't yet implemented — this work forms a base for that.

This copies much JWKS helper code (`cmd/cloud-init-server/auth.go`) from https://github.com/OpenCHAMI/smd/blob/cdac4f1a606401a2b150513b935143238e3eb026/cmd/smd/routers.go with certain irrelevant-to-us parts stripped out.

This is the key that must be included in future GET/DELETE/etc. requests in order to access the node in question. May be different from the originally specified name, particularly if the latter includes special characters which may not be URL-safe.

This ensures the flag is always given a value (even an empty one), preventing parse errors.

This seriously complicates the data-retrieval process, especially when talking to SMD. Also, since we're dealing with MAC addresses here, the colons will *always* be "slugged" into dashes, so there's no real way to avoid this mangling.

Your parsers will thank me.

This is the case when no xname can be found for the given MAC address, in which case we consider the possibility that the input isn't a MAC address at all, and use it as directly as a datastore key.

Apparently Chi gets mad if you don't follow their expected setup order.

These are based on the routers present in SMD.

We use this to grab a new token when necessary.

As long as they follow the standard output format.

davidallendj · 2024-07-17T16:29:35Z

internal/smdclient/oidc.go

+}
+
+// Refresh the cached access token, using the provided JWT server
+func (s *SMDClient) RefreshToken() error {


One thing to keep in mind is that OPAAL's token endpoint returns a JWT bearer token without having to perform an authorization grant that's expected from OAuth 2 servers. I may be worth implementing some way to do a client credentials grant (which is what OPAAL is doing here) to get a token later on. For now though, this should be fine.

davidallendj

Looks good. I left one comment specifically about the OPAAL token endpoint and how it does not require performing an authorization grant like a typical OAuth 2 server implementation would.

alexlovelltroy

LGTM

alexlovelltroy and others added 16 commits May 7, 2024 13:18

replacing copypasta with real binary name in build automation

04a82e9

Implement second HTTP route, to be protected via JWT

d1779ea

The actual verification isn't yet implemented — this work forms a base for that.

Implement JWT-based authentication for secure route

e884218

This copies much JWKS helper code (`cmd/cloud-init-server/auth.go`) from https://github.com/OpenCHAMI/smd/blob/cdac4f1a606401a2b150513b935143238e3eb026/cmd/smd/routers.go with certain irrelevant-to-us parts stripped out.

Add JWKS URL flag to Dockerfile

40858b4

Actions: fix tag format, use stable attestation

7475dd3

Pass empty strings to optional flags when no values are set

2c6d29b

This ensures the flag is always given a value (even an empty one), preventing parse errors.

Don't slug-ify node IDs (MAC addresses)

acfb305

This seriously complicates the data-retrieval process, especially when talking to SMD. Also, since we're dealing with MAC addresses here, the colons will *always* be "slugged" into dashes, so there's no real way to avoid this mangling.

Memstore: use log module instead of fmt for all output

efc4b0d

Your parsers will thank me.

Memstore: fall back to using given name if no SMD results

08c1e9f

This is the case when no xname can be found for the given MAC address, in which case we consider the possibility that the input isn't a MAC address at all, and use it as directly as a datastore key.

Routers: define middlewares before routes, as required

ef839f9

Apparently Chi gets mad if you don't follow their expected setup order.

Add standard middlewares to the root router

cccd40d

These are based on the routers present in SMD.

Update changelog for new release

22fb651

Fix minor spelling error in logs :)

7cb7c62

LRitzdorf requested review from travisbcotton and njones-lanl July 9, 2024 22:30

Handlers: remove dependency on weird module from Gitea

ecf9812

LRitzdorf mentioned this pull request Jul 16, 2024

Update recipes to support JWT-backed cloud-init OpenCHAMI/deployment-recipes#67

Merged

LRitzdorf force-pushed the lritzdorf/rearchitect branch from 48a0a12 to 0af7c7a Compare July 16, 2024 22:29

LRitzdorf added 3 commits July 17, 2024 09:21

smdclient: support fetching new token from OPAAL

22ee13d

main: accept OPAAL URL argument, instead of single token

68075ea

We use this to grab a new token when necessary.

Dockerfile: provide OPAAL URL argument to executable

2735311

LRitzdorf force-pushed the lritzdorf/rearchitect branch from 0af7c7a to 2735311 Compare July 17, 2024 15:22

LRitzdorf added 2 commits July 17, 2024 09:25

Update changelog to include OPAAL server support

f2edfe2

Generalize OPAAL interface to any OIDC server

4a9b067

As long as they follow the standard output format.

LRitzdorf requested a review from davidallendj July 17, 2024 16:22

davidallendj reviewed Jul 17, 2024

View reviewed changes

davidallendj approved these changes Jul 17, 2024

View reviewed changes

oidc: add note about OAuth2 authorization grant support

ce70174

alexlovelltroy approved these changes Jul 17, 2024

View reviewed changes

alexlovelltroy merged commit e99542a into main Jul 17, 2024

LRitzdorf deleted the lritzdorf/rearchitect branch July 17, 2024 16:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rearchitect to remove Gin server package, add JWT-protected HTTP routes #10

Rearchitect to remove Gin server package, add JWT-protected HTTP routes #10

LRitzdorf commented Jul 9, 2024 •

edited

Loading

davidallendj Jul 17, 2024

davidallendj left a comment •

edited

Loading

alexlovelltroy left a comment

Rearchitect to remove Gin server package, add JWT-protected HTTP routes #10

Rearchitect to remove Gin server package, add JWT-protected HTTP routes #10

Conversation

LRitzdorf commented Jul 9, 2024 • edited Loading

davidallendj Jul 17, 2024

Choose a reason for hiding this comment

davidallendj left a comment • edited Loading

Choose a reason for hiding this comment

alexlovelltroy left a comment

Choose a reason for hiding this comment

LRitzdorf commented Jul 9, 2024 •

edited

Loading

davidallendj left a comment •

edited

Loading