chore: Move JWK generation to separate class

.componentsignore

@@ -1,6 +1,7 @@
 [
   "AccessMap",
   "Adapter",
+  "AlgJwk",
   "BaseActivityEmitter",
   "BaseHttpError",
   "BaseRouterHandler",

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
 

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": [

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" }
+    }
+  ]
+}

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" ],

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<string, AlgJwk>;
+
+  private privateJwk?: AlgJwk;
+  private publicJwk?: AlgJwk;
+
+  public constructor(alg: AsymmetricSigningAlgorithm, storageKey: string, storage: KeyValueStorage<string, AlgJwk>) {
+    this.alg = alg;
+    this.key = storageKey;
+    this.storage = storage;
+  }
+
+  public async getPrivateKey(): Promise<AlgJwk> {
+    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<AlgJwk> {
+    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;
+  }
+}

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<string, ClientCredentials>;
   /**
-   * 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<string, unknown>;
+  /**
+   * 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<string, ClientCredentials>;
   private readonly storage: KeyValueStorage<string, unknown>;
+  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<Provider> {
-    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<Configuration> {
+  private async initConfig(key: AlgJwk): Promise<Configuration> {
     // 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 },
           },
         }),
       },

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<AlgJwk>;
+
+  /**
+   * @returns The private key of the asymmetric JWK.
+   */
+  getPrivateKey: () => Promise<AlgJwk>;
+}

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

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<string, AlgJwk>;
+  let storage: jest.Mocked<KeyValueStorage<string, AlgJwk>>;
+  let generator: CachedJwkGenerator;
+
+  beforeEach(async(): Promise<void> => {
+    storageMap = new Map();
+    storage = {
+      get: jest.fn(async(key: string): Promise<AlgJwk | undefined> => storageMap.get(key)),
+      set: jest.fn(async(key: string, value: AlgJwk): Promise<any> => storageMap.set(key, value)),
+    } as any;
+
+    generator = new CachedJwkGenerator(alg, storageKey, storage);
+  });
+
+  it('generates a matching key set.', async(): Promise<void> => {
+    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<void> => {
+    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<void> => {
+    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<void> => {
+    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();
+  });
+});

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<InteractionHandler>;
   let adapterFactory: jest.Mocked<AdapterFactory>;
   let storage: jest.Mocked<KeyValueStorage<string, any>>;
+  let jwkGenerator: jest.Mocked<JwkGenerator>;
   let credentialStorage: jest.Mocked<KeyValueStorage<string, ClientCredentials>>;
   let errorHandler: jest.Mocked<ErrorHandler>;
   let responseWriter: jest.Mocked<ResponseWriter>;
@@ -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);
   });