Ensure Non-Empty JWT Bundles Before Adding to FetchJWTBundles Response

[maxlambrecht]

Pull Request check list

• Commit conforms to CONTRIBUTING.md?
• Proper tests/regressions included?
• Documentation updated?

Affected functionality

Description of change

This PR addresses a specific issue encountered when a Federated X.509 bundle is configured via the SPIRE Server command ./spire-server bundle set. In the current behavior, while the FetchJWTBundles operation correctly includes the Federated trust domain within the bundles' map, it fails to account for scenarios where the associated JWT bundle is empty. This oversight leads to the inclusion of empty JWT bundle values in the response map. Such occurrences can cause errors in clients consuming the Workload API, as they might not be equipped to handle empty JWT bundle entries properly.

Proposed Change:
To mitigate this issue this PR introduces a validation step that ensures a JWT bundle is not empty before its inclusion in the response map.

Update:
To effectively address the identified issue, we've chosen to include a JSON object with an empty array {"keys": []} in the JwtBundles response for any federated trust domain that lacks an associated JWT bundle. This solution not only resolves the problem of handling empty JWT bundles but also maintains the response's debuggability.
It has been tested that this response can be handled correctly by Go, Java, Rust, and Python client libraries.

Which issue this PR fixes

[azdagron]

Thanks for opening this, @maxlambrecht!

One concern with omitting the bundle from the map completely is that this situation becomes ambiguous with a situation where there is no federation relationship set up with the other trust domain at all. This can complicate debugging.

Before we consider taking this change, and potentially impacting debuggability, can you give more details on how you've seen clients fail in this scenario? Was it because the JWKS contained a keys field with a null value (e.g. {"keys": null}) or did they parse that correctly but have a problem because the JWKS had no keys? What did the client end up doing when encountering the problematic JWKS?

[maxlambrecht]

Thanks for opening this, @maxlambrecht!

One concern with omitting the bundle from the map completely is that this situation becomes ambiguous with a situation where there is no federation relationship set up with the other trust domain at all. This can complicate debugging.

Before we consider taking this change, and potentially impacting debuggability, can you give more details on how you've seen clients fail in this scenario? Was it because the JWKS contained a keys field with a null value (e.g. {"keys": null}) or did they parse that correctly but have a problem because the JWKS had no keys? What did the client end up doing when encountering the problematic JWKS?

Yes, the issue arises because the JWKS contains a keys field with a null value, {"keys": null}, which leads to the client failing to parse it and subsequently throwing an exception. This failure mode suggests that the client does not currently handle this as an expected response, leading to errors when encountering such JWKS.

Given this, should we consider updating the client's behavior to treat a null keys value as a valid set of keys? I believe a similar issue might be present in libraries such as go-spiffe and java-spiffe.

[azdagron]

Go doesn't have this problem. It follows the JSON standard and can successfully unmarshal null into a slice.

However, if we're worried about other languages failing to treat null properly, and they would succeed if we instead marshaled it with an empty list (e.g. {"keys": []}), that would be an easy change to make on SPIRE and would still allow us to include the bundle.

[maxlambrecht]

Thanks for the clarification on Go's handling of null values and the suggestion to use an empty list ({"keys": []}) for compatibility. I'll check how the Java, Rust, and Python libraries handle this scenario to ensure they can also process an empty list correctly. I'll follow up with my findings and any necessary steps to align our implementations.

[azdagron]

Thank you @maxlambrecht !

[maxlambrecht]

I've updated the Python library to gracefully handle both {"keys": null} and {"keys": []} scenarios. However, the Java and Rust SPIFFE libraries currently do not support {"keys": null} but can handle {"keys": []} without issue.

Adopting your suggestion to return {"keys": []} for empty JWKS documents in SPIRE would indeed enhance compatibility across these libraries.

[maxlambrecht]

I've implemented the proposed changes to include a JSON object with an empty array {"keys": []} in the JwtBundles response for federated trust domains that lack an associated JWT bundle.