Skip to content

Commit 9389280

Browse files
mschmickingclaude
andcommitted
feat(auth): pin the ioBroker TLS certificate on first use
`allowSelfSigned` exists because ioBroker refuses passwords over plain HTTP, so an authenticated instance is forced onto HTTPS and a home instance signs its own certificate. But switching off CA validation removed the only proof of who the server is: the tool would hand the stored password to anything answering on that address. CodeQL flagged the symptom (js/disabling-certificate-validation on socket.ts) — SECURITY.md already conceded the cause. Identity now comes from a pinned SHA-256 fingerprint instead. It is recorded on the first connection, verified on every one after that, and a change stops the command *before* anything is sent. Interactively the user is asked; without a TTY it fails and names `iob-sync trust`. This is what ssh does, including the residual weakness: the very first connection is still trusted blindly. Two layers, both needed. `probeCertificate` reads the certificate over a bare TLS connection without sending an HTTP request, a cookie or a password, which is what makes it possible to ask the user while the answer still matters. The pinned https.Agent re-checks on every connection, because an attacker in the path can relay the probe untouched and interfere only with the connection that carries the credentials. `rejectUnauthorized` is no longer assigned a literal `false` anywhere: it is always `!allowSelfSigned`. The value is the user's decision, not a constant, and writing it as one both misreports what the code does and is the shape CodeQL matches on. `certFingerprint` is optional, so every config written before this keeps loading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c2542ae commit 9389280

18 files changed

Lines changed: 968 additions & 74 deletions

AGENTS.md

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,23 @@ enabled is also on HTTPS — usually with a self-signed certificate. `allowSelfS
190190
used to be honoured only on the websocket; the HTTP login path used global `fetch`,
191191
which cannot accept an untrusted certificate without an `undici` Agent, so login died
192192
at the handshake before sending anything. `client/auth.ts` therefore uses `node:https`
193-
directly rather than `fetch`. `test/fixtures/` holds a committed throwaway certificate
193+
directly rather than `fetch`. `test/fixtures/` holds a generated throwaway certificate
194194
so this path is tested against a real handshake.
195195

196+
Because `allowSelfSigned` removes the only proof of who the server is, identity comes
197+
from a pinned SHA-256 fingerprint instead (`client/tls.ts`, `certFingerprint` in the
198+
config): recorded on first use, checked on every connection, and a mismatch stops the
199+
command **before** credentials are sent. Two layers, both needed — `probeCertificate`
200+
reads the certificate without sending anything, which is what makes it possible to ask
201+
the user at a point where the answer still matters; the pinned `https.Agent` re-checks
202+
on every connection, because an attacker can relay the probe untouched and interfere
203+
only with the connection carrying the password.
204+
205+
`rejectUnauthorized` is never assigned a literal `false` anywhere in `src/`. It is
206+
always `!allowSelfSigned` — the value is the user's decision, not a constant, and
207+
writing it as one both misreports what the code does and trips CodeQL's
208+
`js/disabling-certificate-validation`.
209+
196210
### Two bugs the watch tests caught
197211

198212
Both were live in working code, and both are the kind that only show up under a test

README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ If `logs` shows nothing, that is usually the adapter's own log level rather than
113113
| `init` | Write `.iobroker-sync.json`, verify the connection, create the script folder. Asks interactively when run without flags. `--types` also sets up TypeScript definitions. |
114114
| `types` | Set up editor intellisense (`log`, `schedule`, ...). `--force`, `--offline`. |
115115
| `login` / `logout` | Save or remove the password for this instance. Never stored in the project. |
116+
| `trust` | Accept the instance's current TLS certificate. Only needed after it changes. `--yes` skips the prompt. |
116117
| `pull [pattern]` | Download scripts to disk. Never deletes or overwrites local files. |
117118
| `push [pattern]` | Upload locally modified scripts. Never deletes remote objects. |
118119
| `status` | Show what changed, locally and remotely. |
@@ -189,8 +190,9 @@ test -z "$(iob-sync --json status | jq -rc 'select(.state != "in-sync")')"
189190

190191
- **Verified against Admin 7.x only.** Older versions are likely to work — the legacy
191192
login path exists for them — but the wire protocol has not been checked against them.
192-
- **Self-signed certificates** are accepted only when `allowSelfSigned` is set, and any
193-
certificate is accepted; there is no pinning.
193+
- **Self-signed certificates** are accepted only when `allowSelfSigned` is set. The
194+
certificate is then pinned on first connection and verified on every one after that,
195+
so a change is caught — but the first connection itself is trusted blindly.
194196
- **`Blockly` and `Rules` scripts** are pulled for completeness, but their sources are
195197
generated and editing them by hand is not supported.
196198

SECURITY.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ If you are looking for somewhere to start:
3838
- **`src/sync/safe-path.ts`** — server-controlled ioBroker ids become local file paths.
3939
Guards against traversal, symlinked files and symlinked directories.
4040
- **`src/client/auth.ts`** — OAuth2 and legacy login, TLS handling.
41+
- **`src/client/tls.ts`** — certificate pinning. The check that replaces CA validation
42+
when `allowSelfSigned` is on, and the thing standing between the stored password and
43+
whatever is answering on that address.
4144
- **`src/commands/backup.ts`** — snapshots contain whatever secrets the live scripts do,
4245
and land under the gitignored `.iobroker-sync/`.
4346

@@ -52,10 +55,14 @@ These are known trade-offs, documented so they need not be re-reported:
5255
- **There is no `--password` flag.** `argv` is readable by any local process via `ps` and
5356
is recorded in shell history. Use `--password-stdin`, `IOBROKER_PASSWORD`, the saved
5457
credential, or the interactive prompt.
55-
- **`allowSelfSigned` accepts any certificate.** It exists because ioBroker refuses
56-
passwords over plain HTTP, so authenticated instances are HTTPS with a self-signed
57-
certificate. There is no certificate pinning; on an untrusted network, that is a
58-
meaningful limitation.
58+
- **`allowSelfSigned` turns off CA validation, and the first connection is trusted
59+
blindly.** The flag exists because ioBroker refuses passwords over plain HTTP, so
60+
authenticated instances are HTTPS with a self-signed certificate. Identity then comes
61+
from the pinned fingerprint in `certFingerprint` instead (`src/client/tls.ts`):
62+
recorded on first use, verified on every connection afterwards, and a change stops the
63+
command before anything is sent. The residual weakness is the same one `ssh` has —
64+
an attacker already in position for the _very first_ connection is trusted and pinned.
65+
Verify the fingerprint out of band if that matters to you.
5966
- **`push` cannot disable a script or move it between javascript instances.** It sends
6067
only `common.source` and `common.engineType`, enforced by the type of
6168
`ObjectsApi.extendScript`. This is a safety property, not an oversight.
@@ -71,3 +78,5 @@ These are known trade-offs, documented so they need not be re-reported:
7178
`common.engineType`.
7279
- Anything that lets a malicious ioBroker server cause local code execution or
7380
arbitrary file writes.
81+
- Any way to reach the network with credentials that skips the fingerprint check in
82+
`src/client/tls.ts`, or to make a mismatch continue rather than stop.

docs/AUTHENTICATION.md

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,29 @@ Admin authenticates via OAuth2.
6363
Connect to the **admin adapter** port, usually 8081 — not the socket.io adapter port
6464
(8084), which lacks the permissions this tool needs.
6565

66-
`allowSelfSigned` applies to both the HTTPS login and the websocket. It accepts any
67-
certificate; there is no way to pin a specific one. Without it, an untrusted certificate
68-
fails before any credential is sent, and the error names the setting that fixes it.
66+
`allowSelfSigned` applies to both the HTTPS login and the websocket. Without it, an
67+
untrusted certificate fails before any credential is sent, and the error names the
68+
setting that fixes it.
69+
70+
### The certificate is pinned
71+
72+
Switching off certificate validation would otherwise mean the tool sends your password
73+
to anything answering on that address. So the certificate is remembered instead:
74+
75+
1. On the first connection its SHA-256 fingerprint is written to `certFingerprint` in
76+
`.iobroker-sync.json` and reported once. Nothing to type, nothing to look up.
77+
2. Every connection after that must present the same certificate.
78+
3. If it changes, you are asked before anything is sent — and in a script or CI job,
79+
where there is nobody to ask, the command fails instead.
80+
81+
This is what `ssh` does with `known_hosts`, including the weak spot: the _first_
82+
connection is trusted blindly. On a network you do not trust, verify the fingerprint
83+
against the server before the first run.
84+
85+
A certificate normally changes only because ioBroker was reinstalled or its certificate
86+
regenerated. To accept the new one:
87+
88+
```bash
89+
iob-sync trust # shows the fingerprint and asks
90+
iob-sync trust --yes # no prompt, for unattended use
91+
```

docs/CONFIGURATION.md

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,21 @@ somewhere else.
2424

2525
## `.iobroker-sync.json`
2626

27-
| Field | Meaning |
28-
| ----------------- | ------------------------------------------------------------------ |
29-
| `url` | Admin base URL including scheme and port, e.g. `https://host:8081` |
30-
| `scriptRoot` | Folder for synced scripts, relative to the project root |
31-
| `allowSelfSigned` | Accept an untrusted TLS certificate |
32-
| `username` | Admin username, or `null` when authentication is disabled |
33-
| `defaultInstance` | javascript instance assigned to newly created scripts |
27+
| Field | Meaning |
28+
| ----------------- | --------------------------------------------------------------------- |
29+
| `url` | Admin base URL including scheme and port, e.g. `https://host:8081` |
30+
| `scriptRoot` | Folder for synced scripts, relative to the project root |
31+
| `allowSelfSigned` | Accept an untrusted TLS certificate |
32+
| `certFingerprint` | Optional. SHA-256 of the certificate to expect; recorded on first use |
33+
| `username` | Admin username, or `null` when authentication is disabled |
34+
| `defaultInstance` | javascript instance assigned to newly created scripts |
3435

3536
Commit this file — it holds no password. Credentials live outside the project entirely;
36-
see [AUTHENTICATION.md](AUTHENTICATION.md).
37+
see [AUTHENTICATION.md](AUTHENTICATION.md). A certificate fingerprint is public
38+
information, not a secret, so committing it is fine and makes the pin reviewable.
39+
40+
`certFingerprint` is written for you the first time you connect to an `allowSelfSigned`
41+
instance; you never need to type it. Configs written before it existed keep working.
3742

3843
## `scriptRoot` cannot escape the project
3944

docs/TROUBLESHOOTING.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,15 @@ credential is sent.
6969

7070
Also check you are on the **admin** adapter port (usually 8081), not the socket.io port
7171
(8084).
72+
73+
## "The TLS certificate has changed"
74+
75+
Commands stop before sending anything, and print the pinned fingerprint next to the one
76+
the server presented.
77+
78+
If you reinstalled ioBroker or regenerated its certificate, this is expected — run
79+
`iob-sync trust` to accept the new one (`--yes` when unattended). If you did **not**
80+
change anything on the server, do not accept it: something is answering in its place.
81+
82+
To start over from scratch, delete the `certFingerprint` line from
83+
`.iobroker-sync.json`; the next connection records whatever it finds.

src/cli.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { loadConfig } from './config';
1515
import { AdminSocketClient } from './client/socket';
1616
import { AdminObjectsApi } from './client/objects';
1717
import { getAuthCookie } from './client/auth';
18+
import { ensureTrustedCertificate } from './client/tls';
1819

1920
import { runInit } from './commands/init';
2021
import { pull } from './commands/pull';
@@ -32,6 +33,7 @@ import { rename } from './commands/rename';
3233
import { move } from './commands/move';
3334
import { remove } from './commands/remove';
3435
import { login, logout } from './commands/login';
36+
import { trust } from './commands/trust';
3537
import { setupTypes } from './commands/types';
3638

3739
/**
@@ -124,7 +126,17 @@ async function withContext(
124126
const startDir = globals.cwd ? path.resolve(globals.cwd) : process.cwd();
125127
const { root, config } = await loadConfig(startDir);
126128

127-
const cookie = await getAuthCookie(config.url, config.username, config.allowSelfSigned, {
129+
// Before anything is sent. `ensureTrustedCertificate` may update `config` in place
130+
// (trust on first use, or an accepted certificate change), so it must run ahead of
131+
// getAuthCookie — that is the call that carries the password.
132+
await ensureTrustedCertificate(root, config, { log: logger });
133+
134+
const tlsConfig = {
135+
allowSelfSigned: config.allowSelfSigned,
136+
certFingerprint: config.certFingerprint,
137+
};
138+
139+
const cookie = await getAuthCookie(config.url, config.username, tlsConfig, {
128140
passwordStdin: globals.passwordStdin,
129141
warn: (msg) => logger.warn(msg),
130142
info: (msg) => logger.info(msg),
@@ -133,7 +145,7 @@ async function withContext(
133145
const socket = new AdminSocketClient({
134146
url: config.url,
135147
cookie,
136-
allowSelfSigned: config.allowSelfSigned,
148+
...tlsConfig,
137149
});
138150

139151
await socket.connect();
@@ -242,7 +254,10 @@ program
242254
.action(function (this: Command) {
243255
return action(async () => {
244256
const startDir = resolveCwd();
245-
const { config } = await loadConfig(startDir);
257+
const { root, config } = await loadConfig(startDir);
258+
// `login` does not go through withContext, so the certificate check has to
259+
// happen here — before a password is collected, let alone sent.
260+
await ensureTrustedCertificate(root, config, { log: logger });
246261
await login(config, { passwordStdin: globals().passwordStdin }, logger);
247262
})();
248263
});
@@ -265,6 +280,18 @@ program
265280
})();
266281
});
267282

283+
program
284+
.command('trust')
285+
.description("record the instance's current TLS certificate as the expected one")
286+
.option('-y, --yes', 'skip the confirmation prompt')
287+
.action(function (this: Command) {
288+
const opts = this.opts();
289+
return action(async () => {
290+
const { root, config } = await loadConfig(resolveCwd());
291+
await trust(root, config, { yes: Boolean(opts.yes) }, logger);
292+
})();
293+
});
294+
268295
program
269296
.command('logout')
270297
.description('remove the stored password for this instance')

src/client/auth.ts

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
* usually also on HTTPS with a self-signed certificate — so honouring
1313
* `allowSelfSigned` here is not optional. The websocket path already honours it.
1414
*
15+
* When a certificate is pinned, these requests go through the agent from
16+
* `src/client/tls.ts`, which drops the connection before the request is written if
17+
* the certificate is not the expected one. That check matters most here: this is the
18+
* module that sends the password.
19+
*
1520
* The password is never logged, never placed in argv, and never written to the
1621
* project config. See `src/credentials.ts`.
1722
*/
@@ -22,6 +27,7 @@ import * as https from 'node:https';
2227
import { UserError } from '../types';
2328
import { readStoredPassword, saveStoredPassword } from '../credentials';
2429
import { isInteractive, promptPassword, promptYesNo, readPasswordFromStdin } from '../prompt';
30+
import { TlsConfig, createPinnedAgent, describePinFailure } from './tls';
2531

2632
export interface AuthOptions {
2733
/** Read the password from stdin (`--password-stdin`) instead of env/store/prompt. */
@@ -57,7 +63,7 @@ function trimTrailingSlash(url: string): string {
5763
*/
5864
function request(
5965
target: string,
60-
opts: { method: string; body?: string; allowSelfSigned: boolean },
66+
opts: { method: string; body?: string; tls: TlsConfig },
6167
): Promise<HttpResponse> {
6268
return new Promise((resolve, reject) => {
6369
let parsed: URL;
@@ -85,7 +91,10 @@ function request(
8591
}
8692
: {},
8793
// Only meaningful for https; ignored otherwise.
88-
rejectUnauthorized: !opts.allowSelfSigned,
94+
rejectUnauthorized: !opts.tls.allowSelfSigned,
95+
// Undefined unless a certificate is pinned, in which case this is what
96+
// enforces it. Node falls back to its global agent when it is absent.
97+
agent: createPinnedAgent(target, opts.tls),
8998
},
9099
(res) => {
91100
// The body is not needed for any of these endpoints, but it must be drained
@@ -157,18 +166,22 @@ async function resolvePassword(
157166
export async function getAuthCookie(
158167
url: string,
159168
username: string | null,
160-
allowSelfSigned: boolean,
169+
tlsConfig: TlsConfig,
161170
opts: AuthOptions = {},
162171
): Promise<string | undefined> {
163172
const base = trimTrailingSlash(url);
164173

165174
let loginProbe: HttpResponse;
166175
try {
167-
loginProbe = await request(`${base}/login`, { method: 'GET', allowSelfSigned });
176+
loginProbe = await request(`${base}/login`, { method: 'GET', tls: tlsConfig });
168177
} catch (err) {
178+
// A pin mismatch is not a reachability problem and must not be reported as one.
179+
const pinFailure = describePinFailure(err);
180+
if (pinFailure) throw pinFailure;
181+
169182
const message = (err as Error).message;
170183
const selfSignedHint =
171-
/self.signed|unable to verify|CERT_/i.test(message) && !allowSelfSigned
184+
/self.signed|unable to verify|CERT_/i.test(message) && !tlsConfig.allowSelfSigned
172185
? 'The certificate is not trusted. Set "allowSelfSigned": true in .iobroker-sync.json.'
173186
: 'Check that the URL in your config is correct and the instance is reachable.';
174187
throw new UserError(`Could not reach ioBroker Admin at ${base}: ${message}`, selfSignedHint);
@@ -196,7 +209,7 @@ export async function getAuthCookie(
196209
try {
197210
const oauthRes = await request(`${base}/oauth/token`, {
198211
method: 'POST',
199-
allowSelfSigned,
212+
tls: tlsConfig,
200213
body: new URLSearchParams({
201214
grant_type: 'password',
202215
username: user,
@@ -213,24 +226,30 @@ export async function getAuthCookie(
213226
return cookie;
214227
}
215228
}
216-
} catch {
217-
// Fall through to legacy login below.
229+
} catch (err) {
230+
// Fall through to legacy login below — unless the certificate was wrong, which
231+
// no amount of retrying on another endpoint will fix.
232+
const pinFailure = describePinFailure(err);
233+
if (pinFailure) throw pinFailure;
218234
}
219235

220236
// Fall back to legacy session-cookie login.
221237
let legacyRes: HttpResponse;
222238
try {
223239
legacyRes = await request(`${base}/login`, {
224240
method: 'POST',
225-
allowSelfSigned,
241+
tls: tlsConfig,
226242
body: new URLSearchParams({
227243
username: user,
228244
password,
229245
stayloggedin: 'on',
230246
}).toString(),
231247
});
232248
} catch (err) {
233-
throw new UserError(`Login request to ${base}/login failed: ${(err as Error).message}`);
249+
throw (
250+
describePinFailure(err) ??
251+
new UserError(`Login request to ${base}/login failed: ${(err as Error).message}`)
252+
);
234253
}
235254

236255
const legacyCookie = extractCookie(legacyRes, 'connect.sid');

src/client/socket.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
SocketOptions,
2424
UserError,
2525
} from '../types';
26+
import { createPinnedAgent, describePinFailure } from './tls';
2627

2728
const READY_MESSAGE = '___ready___';
2829
const OBJECT_CHANGE = 'objectChange';
@@ -111,8 +112,18 @@ export class AdminSocketClient implements SocketClient {
111112
if (this.options.cookie) {
112113
wsOptions.headers = { Cookie: this.options.cookie };
113114
}
114-
if (this.options.allowSelfSigned) {
115-
wsOptions.rejectUnauthorized = false;
115+
// Never a literal `false`: whether the certificate chain is checked is the user's
116+
// `allowSelfSigned` decision, and writing it as a constant would claim otherwise.
117+
// When validation is off, identity comes from the pinned fingerprint instead —
118+
// the agent below drops the connection before the Cookie header is written if the
119+
// certificate is not the expected one. See `src/client/tls.ts`.
120+
wsOptions.rejectUnauthorized = !this.options.allowSelfSigned;
121+
const agent = createPinnedAgent(this.options.url, {
122+
allowSelfSigned: Boolean(this.options.allowSelfSigned),
123+
certFingerprint: this.options.certFingerprint,
124+
});
125+
if (agent) {
126+
wsOptions.agent = agent;
116127
}
117128

118129
this.connectPromise = new Promise<void>((resolve, reject) => {
@@ -196,10 +207,11 @@ export class AdminSocketClient implements SocketClient {
196207
clearTimeout(connectTimer);
197208
this.connectPromise = null;
198209
reject(
199-
new UserError(
200-
`Could not connect to ioBroker Admin at ${this.options.url}: ${err.message}`,
201-
'Check the URL/port and that the Admin instance is reachable.',
202-
),
210+
describePinFailure(err) ??
211+
new UserError(
212+
`Could not connect to ioBroker Admin at ${this.options.url}: ${err.message}`,
213+
'Check the URL/port and that the Admin instance is reachable.',
214+
),
203215
);
204216
return;
205217
}

0 commit comments

Comments
 (0)