Repository navigation
fix(api): scope and expire sync tokens (GHSA-2452-4qrm-6p4w) - #16485
Closed
diegosouzapw wants to merge 3 commits into
Closed
diegosouzapw wants to merge 3 commits into
diegosouzapw wants to merge 3 commits into
Conversation
GHSA-2452-4qrm-6p4w. A sync token unlocks GET /api/sync/bundle, which returns every provider credential and every OmniRoute API key in plaintext. Any manage-scoped API key could mint one without a dashboard session, and the token never expired nor died with its issuer. - POST /api/sync/tokens now requires an authenticated dashboard session or the loopback CLI credential (or a login-disabled instance); a manage-scoped API key or oma_ access token alone gets 403. - Tokens expire (default 30 days, ttl/expiresInDays up to 365 days). - Tokens are bound to a digest of the admin password hash at mint time: any password rotation (dashboard, require-login, CLI reset) kills them. - A token whose recorded issuing API key is revoked, expired, banned or deleted is rejected. - Legacy tokens without the binding are rejected and revoked by migration 206. - While API key reveal is disabled the bundle masks OmniRoute API keys the same way GET /api/keys does. Cloud sync is unaffected. Co-authored-by: gsmarcil <gsmarcil@users.noreply.github.com>
…452-sync-token-scope
…issuer narrowing The binding digested the stored management password with a fast hash; a legacy install can still hold that password in plaintext, so it now goes through scrypt. The issuer check used a truthiness test that does not narrow the union without strictNullChecks, which broke the API route typecheck.
Contributor
CI Coverage Report
Coverage artifact was not available for this run. |
Collaborator
Author
|
Fechando como duplicada: a GHSA-2452 foi corrigida no tip pela #16478 (sessão de dashboard para emitir, expiração e revogação com o emissor). Advisory já fechado. |
Contributor
|
Hi @diegosouzapw — thanks for confirming that #16478 addresses GHSA-2452-4qrm-6p4w. I understand that #16485 was closed as a duplicate implementation PR. Could you please clarify:
My concern is preserving attribution for the original discovery and clarifying its CVE status, regardless of which PR delivers the fix. Thank you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contexto
Corrige o advisory privado GHSA-2452-4qrm-6p4w (reportado por @gsmarcil), severidade média.
GET /api/sync/bundle(rota pública, protegida só pelo sync token) devolve o bundle completo de configuração: credenciais decifradas de todas as conexões de provider (apiKey/accessToken/refreshToken/idToken) e todas as API keys do OmniRoute em texto puro. Até agora:manageemitia um sync token viaPOST /api/sync/tokens, sem sessão do dashboard e sem senha;Isso contornava controles que já existem:
GET /api/providersmascara os segredos,GET /api/keys/{id}/revealresponde 403 quando o reveal está desligado ePOST /api/cloud/authentrega só fragmentos a chavesmanage.O que muda
Quem pode emitir (
src/lib/sync/issuerAuth.ts):POST /api/sync/tokenspassa a exigir uma credencial de operador:omniroute sync tokens createcontinua funcionando;requireLogin=false), onde toda a superfície de gestão já é aberta.API key
manage/adminsozinha, ou access tokenoma_, recebe 403. Sem credencial nenhuma, continua 401.GET(listar, só metadados) eDELETE(revogar) seguem comrequireManagementAuth.Expiração: todo token novo tem
expiresAt. O padrão é 30 dias. O corpo aceitattl(30d,12h,90m, o formato que a CLI já enviava em--ttl, mas que o servidor ignorava) ouexpiresInDays(mesma convenção de/api/cli/tokens), com teto de 365 dias. Um TTL acima disso responde 400.Vínculo com o emissor:
/api/settings),/api/settings/require-login,omniroute reset-passworde setup da CLI. Comparo o hash armazenado em vez de usarsessionsValidAfterporque este só é atualizado pela rota de settings, e o reset pela CLI (o caminho de recuperação quando o admin foi comprometido) não passaria por ele.syncApiKeyId, como já acontecia. Se essa key for revogada, desativada, banida, expirada ou apagada, o token é rejeitado.Tokens legados: linhas sem vínculo (emitidas antes da correção, possivelmente por uma key
manage) são rejeitadas na validação. A migration206_sync_token_issuer_binding.sqltambém as marca como revogadas, para que a listagem mostre isso.API keys no bundle: com o reveal desligado (
ALLOW_API_KEY_REVEAL, o mesmo switch do/api/keys/{id}/reveal), a rota/api/sync/bundlemascara o campokeycomo a listagem de/api/keysjá faz (8 primeiros +****+ 4 últimos). A configuração da key continua indo no bundle. Com o reveal ligado, nada muda. As credenciais de provider continuam no bundle, porque replicar a config para outra instância é o propósito da feature. O que fecha essa exposição é a nova barreira de emissão, a expiração e a invalidação.Migration: 206 (a mais alta no tip⚠️ As PRs abertas #16350, #14665, #16439, #16339, #16087 e #14771 também usam o número 206. A que entrar depois precisa renumerar.
release/v3.8.52é 205).Consumidores verificados
omniroute sync bundle <out>(bin/cli/commands/sync.mjs): baixa o bundle com o token via--api-key. Continua funcionando com tokens novos. Tokens antigos precisam ser emitidos de novo.omniroute sync tokens create/list/revoke: a emissão usaapiFetch, que no loopback enviax-omniroute-cli-token, e esse caminho é aceito. O--ttl 30dque a CLI já mandava agora é respeitado, e a colunaExpiresda listagem passa a ter valor.src/lib/cloudSync.ts): monta o payload direto combuildConfigSyncEnvelope(), sem passar pela rota. Não é afetado: o mascaramento das keys acontece só na rota HTTP.docs/guides/FEATURES.mddizia que o bundle tinha só "settings não sensíveis". Corrigi para descrever o conteúdo real e as regras novas. Os espelhos i18n ficam para o pipeline de tradução.Prova TDD (RED → GREEN)
tests/unit/sync-token-scope-ghsa-2452.test.ts(DATA_DIR temporário,resetDbInstance()notest.after):manageemite (201 !== 403); o bundle segue 200 depois de revogar ou apagar a key emissora e depois de trocar a senha (200 !== 401); não háexpiresAt; token legado é aceito; key em texto puro no bundle. Os 2 que passam são os de regressão: sessão de admin emite e baixa o bundle; CLI local emite.Suítes existentes relacionadas, todas verdes e sem alterar asserções:
sync-routes(2),sync-bundle(2),db-sync-tokens(8),cli-sync-commands(8),error-message-sanitization(37),api-auth(40),authz/classify(52),authz/public-route-exact-match(8),hard-session-lease-bypass-inventory(3),capture-critical-db-state(7),api-key-reveal-route(8),check-migration-numbering(15),db-migration-version-uniqueness(2),db-migration-runner(26),db-portable-migration-snapshot-16035(17),db-portable-migration-gating-16035(2).npm run typecheck:corelimpo.eslint(com as supressões) eprettierlimpos nos arquivos alterados.Pontos para o dono decidir
managecomprometida.POST /api/cli/tokensainda aceita uma keymanagepara emitir access tokensoma_com escopoadmin. Isso fica fora do escopo deste advisory, mas é o mesmo padrão (keymanageescalando para credencial de operador). Por isso os tokensoma_não são aceitos para emitir sync tokens.