Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration of JWK for JWT Creation for extensions requests #2680

Closed
cwperks opened this issue Apr 14, 2023 · 7 comments
Closed

Configuration of JWK for JWT Creation for extensions requests #2680

cwperks opened this issue Apr 14, 2023 · 7 comments
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@cwperks
Copy link
Member

cwperks commented Apr 14, 2023

This issue is P1--Do only if last task available

For the initial implementation, the JWT will be signed with an HMAC 512 secret signing key known to the security plugin. User's should have the ability to customize the JWK (JSON Web Key) that is used to sign the token passed to an extension.

Below is an example configuration for an Elliptic Curve based signature. RFC for JSON Web Key:

jwt_signing_key:
    kty: EC // (Key Type) Parameter
    alg: ES256 // (Algorithm) Parameter
    crv: P-256
    use: sig // (Public Key Use) Parameter
    d: '870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE'
    x: 'MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4'
    y: '4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM'

A PR was introduced in the feature/extensions branch that allows for the creation of JWTs. Currently this only works for HMAC 512 as mentioned. To make the existing code configurable, a configuration field will need to be added to the security configuration file and then parsed into the settings.

After adding that ability, you will want to add various different configuration options based on some of the most common types of signature algorithms. Then you can test that the configuration file is being parsed correctly for these types.

Next, you will want to go into the JWTVendor class added into the branch and make it compatible with the new configuration options. Tests for this part will look like the existing JWTVendor tests but use the difference configuration options.

Completion of this issue will look like a PR (or two) which introduces the above features and associated tests.

@cwperks cwperks converted this from a draft issue Apr 14, 2023
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Apr 14, 2023
@stephen-crawford stephen-crawford added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Apr 17, 2023
@stephen-crawford
Copy link
Contributor

[Triaging] This issue is part of the ongoing work to support extensions.

@stephen-crawford
Copy link
Contributor

I am going to move this into the Extensions Backlog but mark it as P1 and explain what that means.

@stephen-crawford stephen-crawford changed the title [Fast Follow] Configuration of JWK for JWT Creation for extensions requests [Fast Follow (P1: Do only if last task available)] Configuration of JWK for JWT Creation for extensions requests Apr 18, 2023
@stephen-crawford stephen-crawford changed the title [Fast Follow (P1: Do only if last task available)] Configuration of JWK for JWT Creation for extensions requests [Fast Follow] Configuration of JWK for JWT Creation for extensions requests Apr 18, 2023
@MaciejMierzwa
Copy link
Contributor

MaciejMierzwa commented Jun 1, 2023

Hi, I'm thinking about changing configuration for RSA, to allow user to just put in his key in pem format instead of configuring all of the parameters (https://www.rfc-editor.org/rfc/rfc7518#section-6.3.2). What do you think about it @cwperks , @scrawfor99?
Something like:

String key = privateKeySetting
                        .replace("-----BEGIN PRIVATE KEY-----", "")
                        .replaceAll(System.lineSeparator(), "")
                        .replace("-----END PRIVATE KEY-----", "");

                byte[] encoded = BaseEncoding.base64().decode(key);

                KeyFactory keyFactory = null;
                try {
                    keyFactory = KeyFactory.getInstance("RSA");

                    PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(encoded);
                    RSAPrivateCrtKeyImpl privateKey = (RSAPrivateCrtKeyImpl) keyFactory.generatePrivate(keySpec);

                    jwk.setProperty(RSA_MODULUS, privateKey.getModulus());
                    jwk.setProperty(RSA_PUBLIC_EXP, privateKey.getPublicExponent());
                    jwk.setProperty(RSA_PRIVATE_EXP, privateKey.getPrivateExponent());
                    [...] etc
                } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
                    throw new RuntimeException(e);
                }

@stephen-crawford
Copy link
Contributor

stephen-crawford commented Jun 1, 2023

Hi @MaciejMierzwa, I have not checked with @cwperks and am not as familiar with the context around what you have been working on as he may be. That being said, I think that would be a good option but would suggest we have the ability to do both over one or the other. I checked out the documentation you linked and I can see that there are quite a few parameters to be added. My concern about only having a direct key input is that we would likely then run into people who either wanted to automate the process and could not or did not know how to make the correct key. Because we need the private key and the public key to be related, we would need to have the user provide both or the private key and the info to create the associated public key.

If we are able to allow for both options I think that would work since then people who wanted to skip some of the configuration could do so.

@MaciejMierzwa
Copy link
Contributor

I added option to sign JWT using RSA/EC private keys.
For comparison which should we suggest I think it depends on the use case and what we're trying to achieve. If security is the only metric then it's ECDSA -> RSA -> HMAC with ECDSA being the hardest to crack. Regarding performance, HMAC is the fastest one, then comes ECDSA and RSA.
ECDSA provides the same security while having a shorter key, but RSA is well-established/known.
I'm not an expert on security so here's what I've found online regarding this topic:
https://www.ssl2buy.com/wiki/rsa-vs-ecc-which-is-better-algorithm-for-security
https://cheapsslsecurity.com/p/ecc-vs-rsa-comparing-ssl-tls-algorithms/

@peternied
Copy link
Member

Hi, I'm thinking about changing configuration for RSA, to allow user to just put in his key in pem format instead of configuring all of the parameters

@MaciejMierzwa I am in favor of this suggestion, the security plugin configuration is about configuring OpenSearch's security, it isn't as useful to have the idiosyncratic aspects.

As far as the tradeoff between the different algorithms, I'd recommend starting with sensible defaults (+easier to get right when setting up and + more secure), and we can always expand the options over time.

@davidlago
Copy link

Closing PR that took at stab at this #2805 as we figure out #3267 first.

@peternied peternied changed the title [Fast Follow] Configuration of JWK for JWT Creation for extensions requests Configuration of JWK for JWT Creation for extensions requests Sep 7, 2023
@cwperks cwperks closed this as completed Feb 13, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Security for Extensions Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
Status: Done
Development

No branches or pull requests

5 participants