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 15 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
```
20 changes: 20 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,26 @@ This deployment does not use any SSL encryption (HTTPS).
A production deployment must include a reverse proxy to securize the connection with SSL.
:::

::: warning
# Authentication Security in Production

## ⚠️ Important Security Requirement

You must set the `kuzzle_security__authToken__secret` environment variable before deploying Kuzzle to production. This secret is used to sign and verify JSON Web Tokens (JWTs) for user authentication.

## Why This Matters
- Prevents tokens from being stored in Elasticsearch
- Improves overall security
- Gives you direct control over token management

## Security Notes
1. **Fallback Warning**: If you don't set this variable, Kuzzle will use a less secure fallback method (not recommended for production)
2. **Token Invalidation**: Changing the secret value will immediately invalidate all existing authentication tokens
3. **User Impact**: Users will need to log in again if the secret changes

## Additional Resources
For other configuration options, see the [sample configuration file](https://github.com/kuzzleio/kuzzle/blob/master/.kuzzlerc.sample.jsonc).
:::
## 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
38 changes: 22 additions & 16 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,29 @@ 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;
}
let storedSeed = await this.exists("config", this._JWT_SECRET_ID);

async _persistSecret() {
const seed =
global.kuzzle.config.security.jwt.secret ||
crypto.randomBytes(512).toString("hex");
if (!configSeed) {
if (!storedSeed) {
storedSeed = crypto.randomBytes(512).toString("hex");
await this.create(
"config",
{ seed: storedSeed },
{ id: this._JWT_SECRET_ID },
);
}

await this.create(
"config",
{ seed },
{
id: this._JWT_SECRET_ID,
},
);
global.kuzzle.log.warn(
"[!] Kuzzle is using generated seed for authentication. This is suitable for development but should NEVER be use in Production. See https://docs.kuzzle.io/core/2/guides/getting-started/deploy-your-application/",
Juiced66 marked this conversation as resolved.
Show resolved Hide resolved
);
}
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
Loading
Loading