Skip to content

Commit edb1408

Browse files
foxriver76marc2016
foxriver76
and
marc2016
authored
Fix encrypted config (#2401)
* Fixed UpdateConfig for encryptedNative * Added optional parameter useAutoEncryption in _updateConfig * Changes after review. * fix type issues of PR * added test for updateConfig * small refactoring of reading system secret * typo * on update config, ensure that we use newest config in all cases * rm useless check --------- Co-authored-by: Marc Lammers <marc@lammers.dev>
1 parent 13bdcd5 commit edb1408

File tree

4 files changed

+130
-48
lines changed

4 files changed

+130
-48
lines changed

‎packages/adapter/src/lib/adapter/adapter.ts‎

Lines changed: 56 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { PluginHandler } from '@iobroker/plugin-base';
1010
import semver from 'semver';
1111
import path from 'path';
1212
import { getObjectsConstructor, getStatesConstructor } from '@iobroker/js-controller-common-db';
13-
import { getSupportedFeatures, isMessageboxSupported } from './utils';
13+
import { decryptArray, encryptArray, getSupportedFeatures, isMessageboxSupported } from './utils';
1414
// eslint-disable-next-line @typescript-eslint/no-var-requires
1515
const extend = require('node.extend');
1616
import type { Client as StatesInRedisClient } from '@iobroker/db-states-redis';
@@ -2434,8 +2434,8 @@ export class AdapterClass extends EventEmitter {
24342434
}
24352435

24362436
private async _updateConfig(options: InternalUpdateConfigOptions): ioBroker.SetObjectPromise {
2437-
// merge the old and new configuration
2438-
const _config = Object.assign({}, this.config, options.newConfig);
2437+
const { newConfig } = options;
2438+
24392439
// update the adapter config object
24402440
const configObjId = `system.adapter.${this.namespace}`;
24412441
let obj;
@@ -2449,7 +2449,21 @@ export class AdapterClass extends EventEmitter {
24492449
throw new Error(tools.ERRORS.ERROR_DB_CLOSED);
24502450
}
24512451

2452-
obj.native = _config;
2452+
const oldConfig = obj.native;
2453+
let mergedConfig: Record<string, unknown>;
2454+
2455+
// merge the old and new configuration
2456+
if ('encryptedNative' in obj && Array.isArray(obj.encryptedNative)) {
2457+
const secret = await this.getSystemSecret();
2458+
decryptArray({ obj: oldConfig, secret, keys: obj.encryptedNative });
2459+
mergedConfig = { ...oldConfig, ...newConfig };
2460+
encryptArray({ obj: mergedConfig, secret, keys: obj.encryptedNative });
2461+
} else {
2462+
mergedConfig = { ...oldConfig, ...newConfig };
2463+
}
2464+
2465+
obj.native = mergedConfig;
2466+
24532467
return this.setForeignObjectAsync(configObjId, obj);
24542468
}
24552469

@@ -2497,25 +2511,34 @@ export class AdapterClass extends EventEmitter {
24972511
const value = (this.config as InternalAdapterConfig)[attribute];
24982512

24992513
if (typeof value === 'string') {
2500-
if (this._systemSecret !== undefined) {
2501-
return tools.maybeCallbackWithError(callback, null, tools.decrypt(this._systemSecret, value));
2502-
} else {
2503-
try {
2504-
const data = await this.getForeignObjectAsync('system.config');
2505-
if (data?.native) {
2506-
this._systemSecret = data.native.secret;
2507-
}
2508-
} catch {
2509-
// do nothing - we initialize default secret below
2510-
}
2511-
this._systemSecret = this._systemSecret || DEFAULT_SECRET;
2512-
return tools.maybeCallbackWithError(callback, null, tools.decrypt(this._systemSecret, value));
2513-
}
2514+
const secret = await this.getSystemSecret();
2515+
return tools.maybeCallbackWithError(callback, null, tools.decrypt(secret, value));
25142516
} else {
25152517
return tools.maybeCallbackWithError(callback, `Attribute "${attribute}" not found`);
25162518
}
25172519
}
25182520

2521+
/**
2522+
* Get the system secret, after retrived once it will be read from cache
2523+
*/
2524+
private async getSystemSecret(): Promise<string> {
2525+
if (this._systemSecret !== undefined) {
2526+
return this._systemSecret;
2527+
}
2528+
2529+
try {
2530+
const data = await this.getForeignObjectAsync('system.config');
2531+
if (data?.native) {
2532+
this._systemSecret = data.native.secret;
2533+
}
2534+
} catch {
2535+
// do nothing - we initialize default secret below
2536+
}
2537+
2538+
this._systemSecret = this._systemSecret || DEFAULT_SECRET;
2539+
return this._systemSecret;
2540+
}
2541+
25192542
// external signature
25202543
setTimeout(cb: TimeoutCallback, timeout: number, ...args: any[]): ioBroker.Timeout | undefined;
25212544
/**
@@ -8870,10 +8893,10 @@ export class AdapterClass extends EventEmitter {
88708893
// ignore
88718894
}
88728895

8873-
if (data && data.common) {
8896+
if (data?.common) {
88748897
this.defaultHistory = data.common.defaultHistory;
88758898
}
8876-
if (data && data.native) {
8899+
if (data?.native) {
88778900
this._systemSecret = data.native.secret;
88788901
}
88798902

@@ -8890,10 +8913,10 @@ export class AdapterClass extends EventEmitter {
88908913
// ignore
88918914
}
88928915

8893-
if (_obj && _obj.rows) {
8894-
for (let i = 0; i < _obj.rows.length; i++) {
8895-
if (_obj.rows[i].value.common && _obj.rows[i].value.common.type === 'storage') {
8896-
this.defaultHistory = _obj.rows[i].id.substring('system.adapter.'.length);
8916+
if (_obj?.rows) {
8917+
for (const row of _obj.rows) {
8918+
if (row.value.common && row.value.common.type === 'storage') {
8919+
this.defaultHistory = row.id.substring('system.adapter.'.length);
88978920
break;
88988921
}
88998922
}
@@ -11210,15 +11233,15 @@ export class AdapterClass extends EventEmitter {
1121011233
// Read dateformat if using of formatDate is announced
1121111234
if (this._options.useFormatDate) {
1121211235
adapterObjects.getObject('system.config', (err, data) => {
11213-
if (data && data.common) {
11236+
if (data?.common) {
1121411237
this.dateFormat = data.common.dateFormat;
1121511238
this.isFloatComma = data.common.isFloatComma;
1121611239
this.language = data.common.language;
1121711240
this.longitude = data.common.longitude;
1121811241
this.latitude = data.common.latitude;
1121911242
this.defaultHistory = data.common.defaultHistory;
1122011243
}
11221-
if (data && data.native) {
11244+
if (data?.native) {
1122211245
this._systemSecret = data.native.secret;
1122311246
}
1122411247
return tools.maybeCallback(cb);
@@ -11681,18 +11704,8 @@ export class AdapterClass extends EventEmitter {
1168111704
await this.subscribeObjectsAsync('*');
1168211705
}
1168311706

11684-
// read the systemSecret
11685-
if (this._systemSecret === undefined) {
11686-
try {
11687-
const data = await adapterObjects.getObjectAsync('system.config');
11688-
if (data?.native) {
11689-
this._systemSecret = data.native.secret;
11690-
}
11691-
} catch {
11692-
// ignore
11693-
}
11694-
this._systemSecret = this._systemSecret || DEFAULT_SECRET;
11695-
}
11707+
// initialize the system secret
11708+
await this.getSystemSecret();
1169611709

1169711710
// Decrypt all attributes of encryptedNative
1169811711
const promises = [];
@@ -11740,12 +11753,11 @@ export class AdapterClass extends EventEmitter {
1174011753
});
1174111754

1174211755
if (this._options.instance === undefined) {
11743-
this.version =
11744-
this.pack && this.pack.version
11745-
? this.pack.version
11746-
: this.ioPack && this.ioPack.common
11747-
? this.ioPack.common.version
11748-
: 'unknown';
11756+
this.version = this.pack?.version
11757+
? this.pack.version
11758+
: this.ioPack?.common
11759+
? this.ioPack.common.version
11760+
: 'unknown';
1174911761
// display if it's a non-official version - only if installedFrom is explicitly given and differs it's not npm
1175011762
const isNpmVersion =
1175111763
!this.ioPack ||
@@ -11846,7 +11858,6 @@ export class AdapterClass extends EventEmitter {
1184611858

1184711859
/**
1184811860
* Calls the ready handler, if it is an install run it calls the install handler instead
11849-
* @private
1185011861
*/
1185111862
private _callReadyHandler(): void {
1185211863
if (

‎packages/adapter/src/lib/adapter/utils.ts‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1-
import { isObject, isControllerUiUpgradeSupported } from '@iobroker/js-controller-common/tools';
1+
import { isObject, isControllerUiUpgradeSupported, encrypt, decrypt } from '@iobroker/js-controller-common/tools';
22
import { SUPPORTED_FEATURES, SupportedFeature } from './constants';
33

4+
interface EncryptArrayOptions {
5+
/** The objects whose values should be en/decrypted */
6+
obj: Record<string, unknown>;
7+
/** Keys which need to be en/decrypted */
8+
keys: string[];
9+
/** Secret to use for en/decryption */
10+
secret: string;
11+
}
12+
413
/**
514
* Check if messagebox is configured for given instance
615
* This means, at least one of the properties has a value different from false
@@ -28,3 +37,35 @@ export function getSupportedFeatures(): SupportedFeature[] {
2837

2938
return SUPPORTED_FEATURES;
3039
}
40+
41+
/**
42+
* Encrypt given keys of given object
43+
*
44+
* @param options keys, object and secret information
45+
*/
46+
export function encryptArray(options: EncryptArrayOptions): void {
47+
const { secret, obj, keys } = options;
48+
49+
for (const attr of keys) {
50+
const val = obj[attr];
51+
if (typeof val === 'string') {
52+
obj[attr] = encrypt(secret, val);
53+
}
54+
}
55+
}
56+
57+
/**
58+
* Decrypt given keys of given object
59+
*
60+
* @param options keys, object and secret information
61+
*/
62+
export function decryptArray(options: EncryptArrayOptions): void {
63+
const { secret, obj, keys } = options;
64+
65+
for (const attr of keys) {
66+
const val = obj[attr];
67+
if (typeof val === 'string') {
68+
obj[attr] = decrypt(secret, val);
69+
}
70+
}
71+
}

‎packages/controller/src/main.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4188,7 +4188,7 @@ async function startInstance(id: ioBroker.ObjectIDs.Instance, wakeUp = false): P
41884188
}
41894189

41904190
const args =
4191-
instance && instance._id && instance.common
4191+
instance?._id && instance.common
41924192
? [instance._id.split('.').pop(), instance.common.loglevel || 'info']
41934193
: [0, 'info'];
41944194

@@ -4642,7 +4642,7 @@ async function startInstance(id: ioBroker.ObjectIDs.Instance, wakeUp = false): P
46424642
proc.process = cp.fork(adapterMainFile, args, {
46434643
execArgv: tools.getDefaultNodeArgs(adapterMainFile),
46444644
stdio: ['ignore', 'ignore', 'pipe', 'ipc'],
4645-
// @ts-expect-error missing from types but we already tested it is needed
4645+
// @ts-expect-error missing from types, but we already tested it is needed
46464646
windowsHide: true,
46474647
cwd: adapterDir!
46484648
});

‎packages/controller/test/lib/testAdapterHelpers.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,36 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
451451
expect(decrypted).to.equal('topSecret');
452452
});
453453

454+
it(
455+
context.name + ' ' + context.adapterShortName + ' adapter: updateConfig needs to respect encryptedNative',
456+
async () => {
457+
const oldConfig = await context.adapter.getForeignObjectAsync(
458+
`system.adapter.${context.adapter.namespace}`
459+
);
460+
461+
const passphrase = 'SavePassword123';
462+
463+
await context.adapter.updateConfig({ secondPassword: passphrase });
464+
const newConfig = await context.adapter.getForeignObjectAsync(
465+
`system.adapter.${context.adapter.namespace}`
466+
);
467+
468+
// non encrypted and non updated params stay the same
469+
expect(newConfig?.native.paramString).to.exist;
470+
expect(newConfig?.native.paramString).to.be.equal(oldConfig?.native.paramString);
471+
472+
// encrypted non updated passwords, decrypt to the same value
473+
expect(newConfig?.native.password).to.exist;
474+
expect(context.adapter.decrypt(newConfig?.native.password)).to.be.equal(
475+
context.adapter.decrypt(oldConfig?.native.password)
476+
);
477+
478+
// updated encrypted value is correctly decrypted
479+
expect(newConfig?.native.secondPassword).to.exist;
480+
expect(context.adapter.decrypt(newConfig?.native.secondPassword)).to.be.equal(passphrase);
481+
}
482+
);
483+
454484
// setState object validation
455485
for (const method of ['setState', 'setStateChanged', 'setForeignState', 'setForeignStateChanged']) {
456486
describe(`${context.name} ${context.adapterShortName} adapter: ${method} validates the state object`, () => {

0 commit comments

Comments
 (0)