diff --git a/.componentsignore b/.componentsignore index 94cc4b5f5..b7650209e 100644 --- a/.componentsignore +++ b/.componentsignore @@ -1,6 +1,7 @@ [ "AccessMap", "Adapter", + "AlgJwk", "BaseActivityEmitter", "BaseHttpError", "BaseRouterHandler", diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 9697a9a90..d0d8396a1 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -52,6 +52,8 @@ The following changes are relevant for v5 custom configs that replaced certain f - Notification support was added. - `/http/handler/*` - `/notifications/*` +- IDP private key generation was moved to a separate generator class. + - `/identity/handler/*` ### Interface changes @@ -74,6 +76,7 @@ These changes are relevant if you wrote custom modules for the server that depen - The `--config` CLI parameter now accepts multiple configuration paths, which will be combined. - The `RedisLocker` now accepts more configuration parameters. +- `IdentityProviderFactory` takes an addition `JwkGenerator` as input. ## v5.0.0 diff --git a/config/identity/handler/default.json b/config/identity/handler/default.json index 240dbc839..8a2b6c6f2 100644 --- a/config/identity/handler/default.json +++ b/config/identity/handler/default.json @@ -4,6 +4,7 @@ "css:config/identity/handler/account-store/default.json", "css:config/identity/handler/adapter-factory/webid.json", "css:config/identity/handler/interaction/routes.json", + "css:config/identity/handler/jwks/default.json", "css:config/identity/handler/provider-factory/identity.json" ], "@graph": [ diff --git a/config/identity/handler/jwks/default.json b/config/identity/handler/jwks/default.json new file mode 100644 index 000000000..b129a49c0 --- /dev/null +++ b/config/identity/handler/jwks/default.json @@ -0,0 +1,19 @@ +{ + "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^5.0.0/components/context.jsonld", + "@graph": [ + { + "comment": "The JWK that will be used for signing.", + "@id": "urn:solid-server:default:JwkGenerator", + "@type": "CachedJwkGenerator", + "alg": "ES256", + "storageKey": "jwks", + "storage": { "@id": "urn:solid-server:default:KeyStorage" } + }, + { + "@id": "urn:solid-server:default:KeyStorage", + "@type": "EncodingPathStorage", + "relativePath": "/idp/keys/", + "source": { "@id": "urn:solid-server:default:KeyValueStorage" } + } + ] +} diff --git a/config/identity/handler/provider-factory/identity.json b/config/identity/handler/provider-factory/identity.json index c0fd507c1..54c48cf99 100644 --- a/config/identity/handler/provider-factory/identity.json +++ b/config/identity/handler/provider-factory/identity.json @@ -8,19 +8,16 @@ ], "@id": "urn:solid-server:default:IdentityProviderFactory", "@type": "IdentityProviderFactory", - "args_adapterFactory": { "@id": "urn:solid-server:default:IdpAdapterFactory" }, - "args_baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, - "args_oidcPath": "/.oidc", - "args_interactionHandler": { "@id": "urn:solid-server:auth:password:PromptHandler" }, - "args_credentialStorage": { "@id": "urn:solid-server:auth:password:CredentialsStorage" }, - "args_storage": { - "@type": "EncodingPathStorage", - "relativePath": "/idp/keys/", - "source": { "@id": "urn:solid-server:default:KeyValueStorage" } - }, - "args_showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" }, - "args_errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" }, - "args_responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" }, + "adapterFactory": { "@id": "urn:solid-server:default:IdpAdapterFactory" }, + "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, + "oidcPath": "/.oidc", + "interactionHandler": { "@id": "urn:solid-server:auth:password:PromptHandler" }, + "credentialStorage": { "@id": "urn:solid-server:auth:password:CredentialsStorage" }, + "storage": { "@id": "urn:solid-server:default:KeyStorage" }, + "jwkGenerator": { "@id": "urn:solid-server:default:JwkGenerator" }, + "showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" }, + "errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" }, + "responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" }, "config": { "claims": { "openid": [ "azp" ], diff --git a/src/identity/configuration/CachedJwkGenerator.ts b/src/identity/configuration/CachedJwkGenerator.ts new file mode 100644 index 000000000..8250d74cc --- /dev/null +++ b/src/identity/configuration/CachedJwkGenerator.ts @@ -0,0 +1,74 @@ +import { createPublicKey } from 'crypto'; +import type { KeyObject } from 'crypto'; +import { exportJWK, generateKeyPair, importJWK } from 'jose'; +import type { AsymmetricSigningAlgorithm } from 'oidc-provider'; +import type { KeyValueStorage } from '../../storage/keyvalue/KeyValueStorage'; +import type { AlgJwk, JwkGenerator } from './JwkGenerator'; + +/** + * Generates a key pair once and then caches it using both an internal variable and a {@link KeyValueStorage}. + * The storage makes sure the keys remain the same between server restarts, + * while the internal variable makes it so the storage doesn't have to be accessed every time a key is needed. + * + * Only the private key is stored in the internal storage, using the `storageKey` parameter. + * The public key is determined based on the private key and then also stored in memory. + */ +export class CachedJwkGenerator implements JwkGenerator { + public readonly alg: AsymmetricSigningAlgorithm; + + private readonly key: string; + private readonly storage: KeyValueStorage; + + private privateJwk?: AlgJwk; + private publicJwk?: AlgJwk; + + public constructor(alg: AsymmetricSigningAlgorithm, storageKey: string, storage: KeyValueStorage) { + this.alg = alg; + this.key = storageKey; + this.storage = storage; + } + + public async getPrivateKey(): Promise { + if (this.privateJwk) { + return this.privateJwk; + } + + const jwk = await this.storage.get(this.key); + if (jwk) { + this.privateJwk = jwk; + return jwk; + } + + const { privateKey } = await generateKeyPair(this.alg); + + // Make sure the JWK is a plain node object for storage + const privateJwk = { ...await exportJWK(privateKey) } as AlgJwk; + privateJwk.alg = this.alg; + + await this.storage.set(this.key, privateJwk); + this.privateJwk = privateJwk; + return privateJwk; + } + + public async getPublicKey(): Promise { + if (this.publicJwk) { + return this.publicJwk; + } + + const privateJwk = await this.getPrivateKey(); + + // The main reason we generate the public key from the private key is, so we don't have to store it. + // This allows our storage to not break previous versions where we only used the private key. + // In practice this results in the same key. + const privateKey = await importJWK(privateJwk); + const publicKey = createPublicKey(privateKey as KeyObject); + + const publicJwk = { ...await exportJWK(publicKey) } as AlgJwk; + // These fields get lost during the above proces + publicJwk.alg = privateJwk.alg; + + this.publicJwk = publicJwk; + + return publicJwk; + } +} diff --git a/src/identity/configuration/IdentityProviderFactory.ts b/src/identity/configuration/IdentityProviderFactory.ts index 10bf782ad..dd7b0f704 100644 --- a/src/identity/configuration/IdentityProviderFactory.ts +++ b/src/identity/configuration/IdentityProviderFactory.ts @@ -2,10 +2,9 @@ // import/no-unresolved can't handle jose imports // tsdoc/syntax can't handle {json} parameter import { randomBytes } from 'crypto'; -import type { JWK } from 'jose'; -import { exportJWK, generateKeyPair } from 'jose'; import type { Account, Adapter, + AsymmetricSigningAlgorithm, Configuration, ErrorOut, KoaContextWithOIDC, @@ -26,6 +25,7 @@ import { joinUrl } from '../../util/PathUtil'; import type { ClientCredentials } from '../interaction/email-password/credentials/ClientCredentialsAdapterFactory'; import type { InteractionHandler } from '../interaction/InteractionHandler'; import type { AdapterFactory } from '../storage/AdapterFactory'; +import type { AlgJwk, JwkGenerator } from './JwkGenerator'; import type { ProviderFactory } from './ProviderFactory'; export interface IdentityProviderFactoryArgs { @@ -50,9 +50,13 @@ export interface IdentityProviderFactoryArgs { */ credentialStorage: KeyValueStorage; /** - * Storage used to store cookie and JWT keys so they can be re-used in case of multithreading. + * Storage used to store cookie keys so they can be re-used in case of multithreading. */ storage: KeyValueStorage; + /** + * Generates the JWK used for signing and decryption. + */ + jwkGenerator: JwkGenerator; /** * Extra information will be added to the error output if this is true. */ @@ -67,7 +71,6 @@ export interface IdentityProviderFactoryArgs { responseWriter: ResponseWriter; } -const JWKS_KEY = 'jwks'; const COOKIES_KEY = 'cookie-secret'; /** @@ -87,11 +90,11 @@ export class IdentityProviderFactory implements ProviderFactory { private readonly interactionHandler: InteractionHandler; private readonly credentialStorage: KeyValueStorage; private readonly storage: KeyValueStorage; + private readonly jwkGenerator: JwkGenerator; private readonly showStackTrace: boolean; private readonly errorHandler: ErrorHandler; private readonly responseWriter: ResponseWriter; - private readonly jwtAlg = 'ES256'; private provider?: Provider; /** @@ -107,6 +110,7 @@ export class IdentityProviderFactory implements ProviderFactory { this.interactionHandler = args.interactionHandler; this.credentialStorage = args.credentialStorage; this.storage = args.storage; + this.jwkGenerator = args.jwkGenerator; this.showStackTrace = args.showStackTrace; this.errorHandler = args.errorHandler; this.responseWriter = args.responseWriter; @@ -124,10 +128,12 @@ export class IdentityProviderFactory implements ProviderFactory { * Creates a Provider by building a Configuration using all the stored parameters. */ private async createProvider(): Promise { - const config = await this.initConfig(); + const key = await this.jwkGenerator.getPrivateKey(); + + const config = await this.initConfig(key); // Add correct claims to IdToken/AccessToken responses - this.configureClaims(config); + this.configureClaims(config, key.alg); // Make sure routes are contained in the IDP space this.configureRoutes(config); @@ -148,7 +154,7 @@ export class IdentityProviderFactory implements ProviderFactory { * In the `configureErrors` function below, we configure the `renderError` function of the provider configuration. * This function is called by the OIDC provider library to render errors, * but only does this if the accept header is HTML. - * Otherwise, it just returns the error object iself as a JSON object. + * Otherwise, it just returns the error object itself as a JSON object. * See https://github.com/panva/node-oidc-provider/blob/0fcc112e0a95b3b2dae4eba6da812253277567c9/lib/shared/error_handler.js#L48-L52. * * In this function we override the `ctx.accepts` function @@ -181,7 +187,7 @@ export class IdentityProviderFactory implements ProviderFactory { * Creates a configuration by copying the internal configuration * and adding the adapter, default audience and jwks/cookie keys. */ - private async initConfig(): Promise { + private async initConfig(key: AlgJwk): Promise { // Create a deep copy const config: Configuration = JSON.parse(JSON.stringify(this.config)); @@ -193,8 +199,7 @@ export class IdentityProviderFactory implements ProviderFactory { return factory.createStorageAdapter(name); }; - // Cast necessary due to typing conflict between jose 2.x and 3.x - config.jwks = await this.generateJwks() as any; + config.jwks = { keys: [ key ]}; config.cookies = { ...config.cookies, keys: await this.generateCookieKeys(), @@ -209,35 +214,12 @@ export class IdentityProviderFactory implements ProviderFactory { // Default client settings that might not be defined. // Mostly relevant for WebID clients. config.clientDefaults = { - id_token_signed_response_alg: this.jwtAlg, + id_token_signed_response_alg: key.alg, }; return config; } - /** - * Generates a JWKS using a single JWK. - * The JWKS will be cached so subsequent calls return the same key. - */ - private async generateJwks(): Promise<{ keys: JWK[] }> { - // Check to see if the keys are already saved - const jwks = await this.storage.get(JWKS_KEY) as { keys: JWK[] } | undefined; - if (jwks) { - return jwks; - } - // If they are not, generate and save them - const { privateKey } = await generateKeyPair(this.jwtAlg); - const jwk = await exportJWK(privateKey); - // Required for Solid authn client - jwk.alg = this.jwtAlg; - // In node v15.12.0 the JWKS does not get accepted because the JWK is not a plain object, - // which is why we convert it into a plain object here. - // Potentially this can be changed at a later point in time to `{ keys: [ jwk ]}`. - const newJwks = { keys: [{ ...jwk }]}; - await this.storage.set(JWKS_KEY, newJwks); - return newJwks; - } - /** * Generates a cookie secret to be used for cookie signing. * The key will be cached so subsequent calls return the same key. @@ -263,9 +245,9 @@ export class IdentityProviderFactory implements ProviderFactory { } /** - * Adds the necessary claims the to id and access tokens based on the Solid OIDC spec. + * Adds the necessary claims to the id and access tokens based on the Solid OIDC spec. */ - private configureClaims(config: Configuration): void { + private configureClaims(config: Configuration, jwtAlg: AsymmetricSigningAlgorithm): void { // Returns the id_token // See https://solid.github.io/authentication-panel/solid-oidc/#tokens-id // Some fields are still missing, see https://github.com/CommunitySolidServer/CommunitySolidServer/issues/1154#issuecomment-1040233385 @@ -303,7 +285,7 @@ export class IdentityProviderFactory implements ProviderFactory { audience: 'solid', accessTokenFormat: 'jwt', jwt: { - sign: { alg: this.jwtAlg }, + sign: { alg: jwtAlg }, }, }), }, diff --git a/src/identity/configuration/JwkGenerator.ts b/src/identity/configuration/JwkGenerator.ts new file mode 100644 index 000000000..e3da4e026 --- /dev/null +++ b/src/identity/configuration/JwkGenerator.ts @@ -0,0 +1,31 @@ +import type { JWK } from 'jose'; +import type { AsymmetricSigningAlgorithm } from 'oidc-provider'; + +/** + * A {@link JWK} where the `alg` parameter is always defined. + */ +export interface AlgJwk extends JWK { + alg: AsymmetricSigningAlgorithm; +} + +/** + * Generates an asymmetric JWK. + * + * The functions always need to return the same value. + */ +export interface JwkGenerator { + /** + * The algorithm used for the keys. + */ + readonly alg: AsymmetricSigningAlgorithm; + + /** + * @returns The public key of the asymmetric JWK. + */ + getPublicKey: () => Promise; + + /** + * @returns The private key of the asymmetric JWK. + */ + getPrivateKey: () => Promise; +} diff --git a/src/index.ts b/src/index.ts index 54f6770fd..82c9ba4d5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -138,7 +138,9 @@ export * from './http/Operation'; export * from './http/UnsecureWebSocketsProtocol'; // Identity/Configuration +export * from './identity/configuration/CachedJwkGenerator'; export * from './identity/configuration/IdentityProviderFactory'; +export * from './identity/configuration/JwkGenerator'; export * from './identity/configuration/ProviderFactory'; // Identity/Interaction/Email-Password/Credentials diff --git a/test/unit/identity/configuration/CachedJwkGenerator.test.ts b/test/unit/identity/configuration/CachedJwkGenerator.test.ts new file mode 100644 index 000000000..f997adda1 --- /dev/null +++ b/test/unit/identity/configuration/CachedJwkGenerator.test.ts @@ -0,0 +1,88 @@ +import { generateKeyPair, importJWK, jwtVerify, SignJWT } from 'jose'; +import * as jose from 'jose'; +import { CachedJwkGenerator } from '../../../../src/identity/configuration/CachedJwkGenerator'; +import type { AlgJwk } from '../../../../src/identity/configuration/JwkGenerator'; +import type { KeyValueStorage } from '../../../../src/storage/keyvalue/KeyValueStorage'; + +describe('A CachedJwkGenerator', (): void => { + const alg = 'ES256'; + const storageKey = 'jwks'; + let storageMap: Map; + let storage: jest.Mocked>; + let generator: CachedJwkGenerator; + + beforeEach(async(): Promise => { + storageMap = new Map(); + storage = { + get: jest.fn(async(key: string): Promise => storageMap.get(key)), + set: jest.fn(async(key: string, value: AlgJwk): Promise => storageMap.set(key, value)), + } as any; + + generator = new CachedJwkGenerator(alg, storageKey, storage); + }); + + it('generates a matching key set.', async(): Promise => { + const privateKey = await generator.getPrivateKey(); + expect(privateKey.alg).toBe(alg); + + const publicKey = await generator.getPublicKey(); + expect(publicKey.alg).toBe(alg); + + const privateObject = await importJWK(privateKey); + const publicObject = await importJWK(publicKey); + + const signed = await new SignJWT({ data: 'signed data' }).setProtectedHeader({ alg }).sign(privateObject); + await expect(jwtVerify(signed, publicObject)).resolves.toMatchObject({ + payload: { + data: 'signed data', + }, + }); + + const otherKey = (await generateKeyPair(alg)).publicKey; + await expect(jwtVerify(signed, otherKey)).rejects.toThrow(); + }); + + it('caches the private key in memory.', async(): Promise => { + const spy = jest.spyOn(jose, 'generateKeyPair'); + const privateKey = await generator.getPrivateKey(); + // 1 call from checking the storage + expect(storage.get).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledTimes(1); + + const privateKey2 = await generator.getPrivateKey(); + expect(privateKey).toBe(privateKey2); + expect(spy).toHaveBeenCalledTimes(1); + expect(storage.get).toHaveBeenCalledTimes(1); + spy.mockRestore(); + }); + + it('caches the public key in memory.', async(): Promise => { + const spy = jest.spyOn(jose, 'generateKeyPair'); + const publicKey = await generator.getPublicKey(); + // 1 call from checking the storage + expect(storage.get).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledTimes(1); + + const publicKey2 = await generator.getPublicKey(); + expect(publicKey).toBe(publicKey2); + expect(spy).toHaveBeenCalledTimes(1); + expect(storage.get).toHaveBeenCalledTimes(1); + spy.mockRestore(); + }); + + it('caches the key in storage in case of server restart.', async(): Promise => { + const spy = jest.spyOn(jose, 'generateKeyPair'); + const privateKey = await generator.getPrivateKey(); + // 1 call from checking the storage + expect(storage.get).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledTimes(1); + + const generator2 = new CachedJwkGenerator(alg, storageKey, storage); + + const privateKey2 = await generator2.getPrivateKey(); + expect(privateKey).toBe(privateKey2); + expect(spy).toHaveBeenCalledTimes(1); + expect(storage.get).toHaveBeenCalledTimes(2); + spy.mockRestore(); + }); +}); diff --git a/test/unit/identity/configuration/IdentityProviderFactory.test.ts b/test/unit/identity/configuration/IdentityProviderFactory.test.ts index 05701709d..805f09bf4 100644 --- a/test/unit/identity/configuration/IdentityProviderFactory.test.ts +++ b/test/unit/identity/configuration/IdentityProviderFactory.test.ts @@ -1,10 +1,12 @@ import { Readable } from 'stream'; +import { exportJWK, generateKeyPair } from 'jose'; import type * as Koa from 'koa'; import type { errors, Configuration, KoaContextWithOIDC } from 'oidc-provider'; import type { ErrorHandler } from '../../../../src/http/output/error/ErrorHandler'; import type { ResponseWriter } from '../../../../src/http/output/ResponseWriter'; import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation'; import { IdentityProviderFactory } from '../../../../src/identity/configuration/IdentityProviderFactory'; +import type { JwkGenerator } from '../../../../src/identity/configuration/JwkGenerator'; import type { ClientCredentials, } from '../../../../src/identity/interaction/email-password/credentials/ClientCredentialsAdapterFactory'; @@ -45,6 +47,7 @@ describe('An IdentityProviderFactory', (): void => { let interactionHandler: jest.Mocked; let adapterFactory: jest.Mocked; let storage: jest.Mocked>; + let jwkGenerator: jest.Mocked; let credentialStorage: jest.Mocked>; let errorHandler: jest.Mocked; let responseWriter: jest.Mocked; @@ -77,6 +80,13 @@ describe('An IdentityProviderFactory', (): void => { set: jest.fn((id: string, value: any): any => map.set(id, value)), } as any; + const { privateKey, publicKey } = await generateKeyPair('ES256'); + jwkGenerator = { + alg: 'ES256', + getPrivateKey: jest.fn().mockResolvedValue({ ...await exportJWK(privateKey), alg: 'ES256' }), + getPublicKey: jest.fn().mockResolvedValue({ ...await exportJWK(publicKey), alg: 'ES256' }), + }; + credentialStorage = { get: jest.fn((id: string): any => map.get(id)), set: jest.fn((id: string, value: any): any => map.set(id, value)), @@ -94,6 +104,7 @@ describe('An IdentityProviderFactory', (): void => { oidcPath, interactionHandler, storage, + jwkGenerator, credentialStorage, showStackTrace: true, errorHandler, @@ -179,6 +190,7 @@ describe('An IdentityProviderFactory', (): void => { oidcPath, interactionHandler, storage, + jwkGenerator, credentialStorage, showStackTrace: true, errorHandler, @@ -203,6 +215,7 @@ describe('An IdentityProviderFactory', (): void => { oidcPath, interactionHandler, storage, + jwkGenerator, credentialStorage, showStackTrace: true, errorHandler, @@ -210,10 +223,8 @@ describe('An IdentityProviderFactory', (): void => { }); const result2 = await factory2.getProvider() as unknown as { issuer: string; config: Configuration }; expect(result1.config.cookies).toEqual(result2.config.cookies); - expect(result1.config.jwks).toEqual(result2.config.jwks); - expect(storage.get).toHaveBeenCalledTimes(4); - expect(storage.set).toHaveBeenCalledTimes(2); - expect(storage.set).toHaveBeenCalledWith('jwks', result1.config.jwks); + expect(storage.get).toHaveBeenCalledTimes(2); + expect(storage.set).toHaveBeenCalledTimes(1); expect(storage.set).toHaveBeenCalledWith('cookie-secret', result1.config.cookies?.keys); });