Skip to content

Commit

Permalink
🚨 Require allowlisting KASes on decrypt and upsert (#195)
Browse files Browse the repository at this point in the history
- Creates helper function for validating input KAS urls are secure
   - This only logs at 'error', as there are many sitations where we want http KAS urls in practice
- Creates helper function to validate a URL is appropriately prefixed (or 'safe')
- Fail on `upsert` and `rewrap` against 'unsafe' URL before emitting the request
- 1.3.0 minor autobump
- Adds new error type, `UnsafeUrlError`, with a `url` proprerty to allow applications to update the client `allowKases` list and retry
  • Loading branch information
dmihalcik-virtru authored Aug 11, 2023
1 parent 74c8995 commit 893788d
Show file tree
Hide file tree
Showing 14 changed files with 1,126 additions and 1,435 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

version=1.2.2
version=1.3.0
pkgs=lib web-app

.PHONY: all audit license-check lint test ci i start format clean
Expand Down
4 changes: 2 additions & 2 deletions lib/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{

Check notice on line 1 in lib/package.json

View workflow job for this annotation

GitHub Actions / deliver-ghp

Will be published to [GitHub Packages](https://github.com/opentdf/client-web/pkgs/npm/client) as beta with version=[1.3.0-beta.1069+5836133520.893788]
"name": "@opentdf/client",
"version": "1.2.2",
"version": "1.3.0",
"description": "Access and generate tdf protected content",
"homepage": "https://github.com/opentdf/client-web",
"bugs": {
Expand Down
6 changes: 6 additions & 0 deletions lib/src/nanotdf/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import getHkdfSalt from './helpers/getHkdfSalt.js';
import DefaultParams from './models/DefaultParams.js';
import { fetchWrappedKey } from '../kas.js';
import { AuthProvider, reqSignature } from '../auth/providers.js';
import { safeUrlCheck, validateSecureUrl } from '../utils.js';

const { KeyUsageType, AlgorithmName, NamedCurve } = cryptoEnums;

Expand Down Expand Up @@ -52,6 +53,7 @@ export default class Client {
static readonly INITIAL_RELEASE_IV_SIZE = 3;
static readonly IV_SIZE = 12;

allowedKases: string[];
/*
These variables are expected to be either assigned during initialization or within the methods.
This is needed as the flow is very specific. Errors should be thrown if the necessary step is not completed.
Expand Down Expand Up @@ -80,7 +82,9 @@ export default class Client {
dpopEnabled = false
) {
this.authProvider = authProvider;
validateSecureUrl(kasUrl);
this.kasUrl = kasUrl;
this.allowedKases = [kasUrl];
this.kasPubKey = '';
this.dpopEnabled = dpopEnabled;

Expand Down Expand Up @@ -186,6 +190,8 @@ export default class Client {
clientVersion: string,
authTagLength: number
): Promise<CryptoKey> {
safeUrlCheck(this.allowedKases, kasRewrapUrl);

// Ensure the ephemeral key pair has been set or generated (see createOidcServiceProvider)
await this.fetchOIDCToken();

Expand Down
55 changes: 55 additions & 0 deletions lib/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,59 @@
import { type AxiosResponseHeaders, type RawAxiosResponseHeaders } from 'axios';
import { UnsafeUrlError } from '../tdf3/src/errors.js';

/**
* Check to see if the given URL is 'secure'. This assumes:
*
* - `https` URLs are always secure
* - `http` URLS are allowed for localhost
* - And also for '`svc.cluster.local` and `.internal` URLs
*
* Note that this does not resolve the URL, so it is possible this could
* resolve to some other internal URL, and may return `false` on non-fully
* qualified internal URLs.
*
* @param url remote service to validate
* @returns the url is local or `https`
*/
export function validateSecureUrl(url: string): boolean {
const httpsRegex = /^https:/;
if (/^http:\/\/(localhost|127\.0\.0\.1)(:[0-9]{1,5})?($|\/)/.test(url)) {
console.warn(`Development URL detected: [${url}]`);
} else if (
/^http:\/\/([a-zA-Z.-]*[.])?svc\.cluster\.local($|\/)/.test(url) ||
/^http:\/\/([a-zA-Z.-]*[.])?internal(:[0-9]{1,5})?($|\/)/.test(url)
) {
console.info(`Internal URL detected: [${url}]`);
} else if (!httpsRegex.test(url)) {
console.error(`Insecure KAS URL loaded. Are you running in a secure environment? [${url}]`);
return false;
}
return true;
}

export function padSlashToUrl(u: string): string {
if (u.endsWith('/')) {
return u;
}
return `${u}/`;
}

const someStartsWith = (prefixes: string[], requestUrl: string): boolean =>
prefixes.some((prixfixe) => requestUrl.startsWith(padSlashToUrl(prixfixe)));

/**
* Checks that `testUrl` is prefixed with one of the given origin + path fragment URIs in urlPrefixes.
*
* Note this doesn't do anything special to queries or fragments and will fail to work properly if those are present on the prefixes
* @param urlPrefixes a list of origin parts of urls, possibly including some path fragment as well
* @param testUrl a url to see if it is prefixed by one or more of the `urlPrefixes` values
* @throws Error when testUrl is not present
*/
export const safeUrlCheck = (urlPrefixes: string[], testUrl: string): void | never => {
if (!someStartsWith(urlPrefixes, testUrl)) {
throw new UnsafeUrlError(`Invalid request URL: [${testUrl}] ∉ [${urlPrefixes}];`, testUrl);
}
};

export function isBrowser() {
return typeof window !== 'undefined'; // eslint-disable-line
Expand Down
2 changes: 1 addition & 1 deletion lib/src/version.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Exposes the released version number of the `@opentdf/client` package
*/
export const version = '1.2.2';
export const version = '1.3.0';

/**
* A string name used to label requests as coming from this library client.
Expand Down
38 changes: 31 additions & 7 deletions lib/tdf3/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { OIDCExternalJwtProvider } from '../../../src/auth/oidc-externaljwt-prov
import { CryptoService, PemKeyPair } from '../crypto/declarations.js';
import { AuthProvider, AppIdAuthProvider, HttpRequest } from '../../../src/auth/auth.js';
import EAS from '../../../src/auth/Eas.js';
import { validateSecureUrl } from '../../../src/utils.js';

import { EncryptParams, DecryptParams, type Scope } from './builders.js';
import { DecoratedReadableStream } from './DecoratedReadableStream.js';
Expand Down Expand Up @@ -98,6 +99,11 @@ export interface ClientConfig {
clientId?: string;
dpopEnabled?: boolean;
kasEndpoint?: string;
/**
* List of allowed KASes to connect to for rewrap requests.
* Defaults to `[kasEndpoint]`.
*/
allowedKases?: string[];
easEndpoint?: string;
// DEPRECATED Ignored
keyRewrapEndpoint?: string;
Expand Down Expand Up @@ -151,18 +157,19 @@ export async function createSessionKeys({
return { keypair: k2, signingKeys };
}

/*
* If we have KAS url but not public key we can fetch it from KAS
/**
* If we have KAS url but not public key we can fetch it from KAS, fetching
* the value from `${kas}/kas_public_key`.
*/
export async function fetchKasPubKey(kasEndpoint: string): Promise<string> {
if (!kasEndpoint) {
export async function fetchKasPubKey(kas: string): Promise<string> {
if (!kas) {
throw new TdfError('KAS definition not found');
}
try {
return await TDF.getPublicKeyFromKeyAccessServer(kasEndpoint);
return await TDF.getPublicKeyFromKeyAccessServer(kas);
} catch (cause) {
throw new TdfError(
`Retrieving KAS public key [${kasEndpoint}] failed [${cause.name}] [${cause.message}]`,
`Retrieving KAS public key [${kas}] failed [${cause.name}] [${cause.message}]`,
cause
);
}
Expand All @@ -181,6 +188,12 @@ export class Client {
*/
readonly kasEndpoint: string;

/**
* List of allowed KASes to connect to for rewrap requests.
* Defaults to `[this.kasEndpoint]`.
*/
readonly allowedKases: string[];

readonly kasPublicKey: Promise<string>;

readonly easEndpoint?: string;
Expand Down Expand Up @@ -231,6 +244,13 @@ export class Client {
this.kasEndpoint = clientConfig.keyRewrapEndpoint.replace(/\/rewrap$/, '');
}

if (clientConfig.allowedKases) {
this.allowedKases = [...clientConfig.allowedKases];
} else {
this.allowedKases = [this.kasEndpoint];
}
this.allowedKases.forEach(validateSecureUrl);

this.authProvider = config.authProvider;
this.clientConfig = clientConfig;

Expand Down Expand Up @@ -354,7 +374,10 @@ export class Client {

// TODO: Refactor underlying builder to remove some of this unnecessary config.

const tdf = TDF.create({ cryptoService: this.cryptoService })
const tdf = TDF.create({
allowedKases: this.allowedKases,
cryptoService: this.cryptoService,
})
.setPrivateKey(sessionKeys.keypair.privateKey)
.setPublicKey(sessionKeys.keypair.publicKey)
.setEncryption({
Expand Down Expand Up @@ -436,6 +459,7 @@ export class Client {
const tdf = TDF.create({ cryptoService: this.cryptoService })
.setPrivateKey(sessionKeys.keypair.privateKey)
.setPublicKey(sessionKeys.keypair.publicKey)
.setAllowedKases(this.allowedKases)
.setAuthProvider(this.authProvider);
if (entityObject) {
tdf.setEntity(entityObject);
Expand Down
11 changes: 11 additions & 0 deletions lib/tdf3/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ export class TdfError extends Error {
}
}

export class UnsafeUrlError extends Error {
override name = 'UnsafeUrlError';
readonly url: string;

constructor(message: string, url: string) {
super(message);
Object.setPrototypeOf(this, new.target.prototype);
this.url = url;
}
}

export class AttributeValidationError extends TdfError {
override name = 'AttributeValidationError';
}
Expand Down
34 changes: 21 additions & 13 deletions lib/tdf3/src/tdf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { v4 } from 'uuid';
import { exportSPKI, importPKCS8, importX509 } from 'jose';
import { DecoratedReadableStream } from './client/DecoratedReadableStream.js';
import { EntityObject } from '../../src/tdf/EntityObject.js';
import { validateSecureUrl } from '../../src/utils.js';

import {
AttributeSet,
Expand Down Expand Up @@ -118,6 +119,7 @@ type Chunk = {
};

type TDFConfiguration = {
allowedKases?: string[];
cryptoService: CryptoService;
};

Expand All @@ -138,10 +140,15 @@ export class TDF extends EventEmitter {
segmentSizeDefault: number;
chunkMap: Map<string, Chunk>;
cryptoService: CryptoService;
allowedKases: string[];

constructor(configuration: TDFConfiguration) {
super();

if (configuration.allowedKases) {
this.allowedKases = [...configuration.allowedKases];
this.allowedKases.forEach(validateSecureUrl);
}
this.attributeSet = new AttributeSet();
this.cryptoService = configuration.cryptoService;
this.publicKey = '';
Expand Down Expand Up @@ -211,19 +218,7 @@ export class TDF extends EventEmitter {

// return a PEM-encoded string from the provided KAS server
static async getPublicKeyFromKeyAccessServer(url: string): Promise<string> {
const httpsRegex = /^https:/;
if (url.startsWith('http://localhost') || url.startsWith('http://127.0.0.1')) {
console.warn(`Development KAS URL detected: [${url}]`);
} else if (
/^http:\/\/[a-zA-Z.-]*[.]?svc\.cluster\.local($|\/)/.test(url) ||
/^http:\/\/[a-zA-Z.-]*[.]?internal($|\/)/.test(url)
) {
console.info(`Internal KAS URL detected: [${url}]`);
} else if (!httpsRegex.test(url)) {
console.error(
`Public key must be requested over a secure channel. Are you running in a secure environment? [${url}]`
);
}
validateSecureUrl(url);
const kasPublicKeyRequest: { data: string } = await axios.get(`${url}/kas_public_key`);
return TDF.extractPemFromKeyString(kasPublicKeyRequest.data);
}
Expand Down Expand Up @@ -373,6 +368,12 @@ export class TDF extends EventEmitter {
throw new KeyAccessError('TDF.addKeyAccess: No source for kasUrl or pubKey');
}

setAllowedKases(kases: string[]) {
this.allowedKases = [...kases];
this.allowedKases.forEach(validateSecureUrl);
return this;
}

setPolicy(policy: Policy) {
this.validatePolicyObject(policy);
this.policy = policy;
Expand Down Expand Up @@ -516,6 +517,10 @@ export class TDF extends EventEmitter {
return;
}

if (!this.allowedKases.includes(keyAccessObject.url)) {
throw new KasUpsertError(`Unexpected KAS url: [${keyAccessObject.url}]`);
}

const url = `${keyAccessObject.url}/${isAppIdProvider ? '' : 'v2/'}upsert`;

//TODO I dont' think we need a body at all for KAS requests
Expand Down Expand Up @@ -844,6 +849,9 @@ export class TDF extends EventEmitter {
if (this.authProvider === undefined) {
throw new Error('Upsert can be done without auth provider');
}
if (!this.allowedKases.includes(keySplitInfo.url)) {
throw new KasUpsertError(`Unexpected KAS url: [${keySplitInfo.url}]`);
}
const url = `${keySplitInfo.url}/${isAppIdProvider ? '' : 'v2'}/rewrap`;

const requestBodyStr = JSON.stringify({
Expand Down
2 changes: 1 addition & 1 deletion lib/tdf3/src/version.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export const version = '1.2.2';
export const version = '1.3.0';
export const clientType = 'tdf3-js-client';
5 changes: 5 additions & 0 deletions lib/tests/mocha/unit/tdf.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ describe('TDF', () => {
expect(actual).to.be.an.instanceof(TDF);
});

it('allowedKases', () => {
const actual = TDF.create({ allowedKases: ['https://local.virtru.com'], cryptoService });
expect(actual.allowedKases).to.contain('https://local.virtru.com');
});

it('Encodes the postMessage origin properly in wrapHtml', () => {
const cipherText = 'abcezas123';
const transferUrl = 'https://local.virtru.com/start?htmlProtocol=1';
Expand Down
Loading

0 comments on commit 893788d

Please sign in to comment.