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

feat: do not store clear api keys in internal storage #2555

Merged
merged 16 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci/scripts/run-test-cluster.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ trap 'docker compose -f $YML_FILE logs' err

docker compose -f $YML_FILE up -d

# don't wait on 7512: nginx will accept connections far before Kuzzle does
KUZZLE_PORT=17510 ./bin/wait-kuzzle
KUZZLE_PORT=17511 ./bin/wait-kuzzle
KUZZLE_PORT=17512 ./bin/wait-kuzzle
KUZZLE_PORT=7512 ./bin/wait-kuzzle

trap - err

Expand Down
3 changes: 3 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"plugins": ["kuzzle"],
"extends": ["plugin:kuzzle/default", "plugin:kuzzle/node"],
"parserOptions": {
"ecmaVersion": 2020
},
"rules": {
"sort-keys": "warn",
"kuzzle/array-foreach": "warn"
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,5 +123,5 @@ npm run test:unit
### Functional tests

```bash
KUZZLE_FUNCTIONAL_TESTS="test:functional:websocket" NODE_VERSION="20" ./.ci/scripts/run-test-cluster.sh
KUZZLE_FUNCTIONAL_TESTS="test:functional:websocket" NODE_VERSION="20" ES_VERSION=8 ./.ci/scripts/run-test-cluster.sh
```
12 changes: 12 additions & 0 deletions doc/2/guides/getting-started/deploy-your-application/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ This deployment does not use any SSL encryption (HTTPS).
A production deployment must include a reverse proxy to securize the connection with SSL.
:::

::: warning
#### Production Deployment: Auth Token Secret

For every production deployment of Kuzzle, it is essential to set the `kuzzle_security__authToken__secret` environment variable. This ensures that the JWT secrets used for authenticating requests are generated externally and not stored in Elasticsearch. By managing the secret through an environment variable, you enhance security, prevent potential data exposure, and ensure tokens remain valid only as long as the secret remains unchanged.

Important: If the `kuzzle_security__authToken__secret` value is changed when Kuzzle restarts, all existing tokens will be invalidated. This ensures that only tokens signed with the current secret remain valid, adding an extra layer of security.

For default configuration values, you can refer to [the sample Kuzzle configuration file](https://github.com/kuzzleio/kuzzle/blob/master/.kuzzlerc.sample.jsonc).

Note: If the secret is not set, Kuzzle will fallback to a less secure method of generating and storing the secret, which is not recommended for production environments.
:::

Juiced66 marked this conversation as resolved.
Show resolved Hide resolved
## Prepare our Docker Compose deployment

We are going to write a `docker-compose.yml` file that describes our services.
Expand Down
148 changes: 83 additions & 65 deletions lib/core/security/tokenRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import { Token } from "../../model/security/token";
import { User } from "../../model/security/user";
import ApiKey from "../../model/storage/apiKey";
import debugFactory from "../../util/debug";
import { Mutex } from "../../util/mutex";
import { ObjectRepository } from "../shared/ObjectRepository";
import { sha256 } from "../../util/crypto";

const securityError = kerror.wrap("security", "token");
const debug = debugFactory("kuzzle:bootstrap:tokens");
Expand Down Expand Up @@ -61,8 +61,6 @@ export class TokenRepository extends ObjectRepository<Token> {
}

async init() {
await this.loadApiKeys();

/**
* Assign an existing token to a user. Stores the token in Kuzzle's cache.
* @param {String} hash - JWT
Expand Down Expand Up @@ -136,6 +134,24 @@ export class TokenRepository extends ObjectRepository<Token> {
global.kuzzle.onAsk("core:security:token:verify", (hash) =>
this.verifyToken(hash),
);

// ? those checks are necessary to detect JWT seed changes and delete existing tokens if necessary
const existingTokens = await global.kuzzle.ask(
"core:cache:internal:searchKeys",
"repos/kuzzle/token/*",
);

if (existingTokens.length > 0) {
try {
const [, token] = existingTokens[0].split("#");
await this.verifyToken(token);
rolljee marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
// ? seed has changed
if (e.id === "security.token.invalid") {
await global.kuzzle.ask("core:cache:internal:del", existingTokens);
}
}
}
}

/**
Expand Down Expand Up @@ -189,8 +205,10 @@ export class TokenRepository extends ObjectRepository<Token> {
async generateToken(
user: User,
{
algorithm = global.kuzzle.config.security.jwt.algorithm,
expiresIn = global.kuzzle.config.security.jwt.expiresIn,
algorithm = global.kuzzle.config.security.authToken.algorithm ??
global.kuzzle.config.security.jwt.algorithm,
expiresIn = global.kuzzle.config.security.authToken.expiresIn ??
global.kuzzle.config.security.jwt.expiresIn,
bypassMaxTTL = false,
type = "authToken",
singleUse = false,
Expand All @@ -211,7 +229,8 @@ export class TokenRepository extends ObjectRepository<Token> {
const maxTTL =
type === "apiKey"
? global.kuzzle.config.security.apiKey.maxTTL
: global.kuzzle.config.security.jwt.maxTTL;
: global.kuzzle.config.security.authToken.maxTTL ??
global.kuzzle.config.security.jwt.maxTTL;

if (
!bypassMaxTTL &&
Expand Down Expand Up @@ -251,10 +270,21 @@ export class TokenRepository extends ObjectRepository<Token> {

if (type === "apiKey") {
encodedToken = Token.APIKEY_PREFIX + encodedToken;
} else {
encodedToken = Token.AUTH_PREFIX + encodedToken;

// For API keys, we don't persist the token
const expiresAt =
parsedExpiresIn === -1 ? -1 : Date.now() + parsedExpiresIn;
return new Token({
_id: `${user._id}#${encodedToken}`,
expiresAt,
jwt: encodedToken,
ttl: parsedExpiresIn,
userId: user._id,
});
}
encodedToken = Token.AUTH_PREFIX + encodedToken;

// Persist regular tokens
return this.persistForUser(encodedToken, user._id, {
singleUse,
ttl: parsedExpiresIn,
Expand Down Expand Up @@ -308,36 +338,41 @@ export class TokenRepository extends ObjectRepository<Token> {
return this.anonymousToken;
}

const isApiKey = token.startsWith(Token.APIKEY_PREFIX);
const tokenWithoutPrefix = this.removeTokenPrefix(token);

let decoded = null;

try {
decoded = jwt.verify(this.removeTokenPrefix(token), global.kuzzle.secret);

decoded = jwt.verify(tokenWithoutPrefix, global.kuzzle.secret);
// probably forged token => throw without providing any information
if (!decoded._id) {
throw new jwt.JsonWebTokenError("Invalid token");
}
} catch (err) {
if (err instanceof jwt.TokenExpiredError) {
throw securityError.get("expired");
}

if (err instanceof jwt.JsonWebTokenError) {
throw securityError.get("invalid");
}

if (err instanceof jwt.TokenExpiredError) {
throw securityError.get("expired");
}

throw securityError.getFrom(err, "verification_error", err.message);
}

let userToken: Token;
if (isApiKey) {
return this._verifyApiKey(decoded, token);
}

let userToken;
Juiced66 marked this conversation as resolved.
Show resolved Hide resolved

try {
userToken = await this.loadForUser(decoded._id, token);
} catch (err) {
if (err instanceof UnauthorizedError) {
throw err;
}

throw securityError.getFrom(err, "verification_error", err.message);
}

Expand All @@ -352,6 +387,38 @@ export class TokenRepository extends ObjectRepository<Token> {
return userToken;
}

async _verifyApiKey(decoded, token: string) {
const fingerprint = sha256(token);

const userApiKeys = await ApiKey.search({
query: {
term: {
userId: decoded._id,
fmauNeko marked this conversation as resolved.
Show resolved Hide resolved
},
},
});

const targetApiKey = userApiKeys?.find(
(apiKey) => apiKey.fingerprint === fingerprint,
);

if (!targetApiKey) {
throw securityError.get("invalid");
}

const apiKey = await ApiKey.load(decoded._id, targetApiKey._id);

const userToken = new Token({
_id: `${decoded._id}#${token}`,
expiresAt: apiKey.expiresAt,
jwt: token,
ttl: apiKey.ttl,
userId: decoded._id,
});

return userToken;
}

removeTokenPrefix(token: string) {
return token
.replace(Token.AUTH_PREFIX, "")
Expand Down Expand Up @@ -436,55 +503,6 @@ export class TokenRepository extends ObjectRepository<Token> {
await Promise.all(promises);
}

/**
* Loads authentication token from API key into Redis
*/
private async loadApiKeys() {
const mutex = new Mutex("ApiKeysBootstrap", {
timeout: -1,
ttl: 30000,
});

await mutex.lock();

try {
const bootstrapped = await global.kuzzle.ask(
"core:cache:internal:get",
BOOTSTRAP_DONE_KEY,
);

if (bootstrapped) {
debug("API keys already in cache. Skip.");
return;
}

debug("Loading API keys into Redis");

const promises = [];

await ApiKey.batchExecute({ match_all: {} }, (documents) => {
for (const { _source } of documents) {
promises.push(
this.persistForUser(_source.token, _source.userId, {
singleUse: false,
ttl: _source.ttl,
}),
);
}
});

await Promise.all(promises);

await global.kuzzle.ask(
"core:cache:internal:store",
BOOTSTRAP_DONE_KEY,
1,
);
} finally {
await mutex.unlock();
}
}

/**
* The repository main class refreshes automatically the TTL
* of accessed entries, letting only unaccessed entries expire
Expand Down
34 changes: 17 additions & 17 deletions lib/kuzzle/internalIndexHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class InternalIndexHandler extends Store {
const bootstrapped = await this.exists("config", this._BOOTSTRAP_DONE_ID);

if (bootstrapped) {
await this._initSecret();
return;
}

Expand Down Expand Up @@ -150,7 +151,7 @@ class InternalIndexHandler extends Store {
await this.createInitialValidations();

debug("Bootstrapping JWT secret");
await this._persistSecret();
await this._initSecret();

// Create datamodel version
await this.create(
Expand Down Expand Up @@ -202,24 +203,23 @@ class InternalIndexHandler extends Store {
await Bluebird.all(promises);
}

async getSecret() {
const response = await this.get("config", this._JWT_SECRET_ID);
async _initSecret() {
const { authToken, jwt } = global.kuzzle.config.security;
const configSeed = authToken?.secret ?? jwt?.secret;

return response._source.seed;
}

async _persistSecret() {
const seed =
global.kuzzle.config.security.jwt.secret ||
crypto.randomBytes(512).toString("hex");
let storedSeed = await this.exists("config", this._JWT_SECRET_ID);

await this.create(
"config",
{ seed },
{
id: this._JWT_SECRET_ID,
},
);
if (!configSeed && !storedSeed) {
storedSeed = crypto.randomBytes(512).toString("hex");
await this.create(
"config",
{ seed: storedSeed },
{ id: this._JWT_SECRET_ID },
);
}
global.kuzzle.secret = configSeed
? configSeed
: (await this.get("config", this._JWT_SECRET_ID))._source.seed;
}
}

Expand Down
3 changes: 0 additions & 3 deletions lib/kuzzle/kuzzle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,6 @@ class Kuzzle extends KuzzleEventEmitter {
// This will init the cluster module if enabled
this.id = await this.initKuzzleNode();

// Secret used to generate JWTs
this.secret = await this.internalIndex.getSecret();

this.vault = vault.load(options.vaultKey, options.secretsFile);

await this.validation.init();
Expand Down
6 changes: 3 additions & 3 deletions lib/model/storage/apiKey.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ApiKey extends BaseModel {
serialize({ includeToken = false } = {}) {
const serialized = super.serialize();

if (!includeToken) {
if (!includeToken && this.token) {
delete serialized._source.token;
}

Expand Down Expand Up @@ -107,15 +107,15 @@ class ApiKey extends BaseModel {
description,
expiresAt: token.expiresAt,
fingerprint,
token: token.jwt,
ttl: token.ttl,
userId: user._id,
},
apiKeyId || fingerprint,
);

await apiKey.save({ refresh, userId: creatorId });

apiKey.token = token.jwt;

return apiKey;
}

Expand Down
3 changes: 1 addition & 2 deletions lib/model/storage/baseModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ class BaseModel {
searchBody,
options,
);

return resp.hits.map((hit) => this._instantiateFromDb(hit));
}

Expand All @@ -232,7 +231,7 @@ class BaseModel {
static truncate({ refresh } = {}) {
return this.deleteByQuery({ match_all: {} }, { refresh });
}

// ? This looks not in use anymore ?
static batchExecute(query, callback) {
return global.kuzzle.internalIndex.mExecute(
this.collection,
Expand Down
Loading
Loading