Skip to content

Commit e4f8ac5

Browse files
ivanstanevivanstanev
authored
Merge pull request #194 from snyk/feat/secure-config-improved
feat: secure config improved
2 parents 0b05aee + b2da3a4 commit e4f8ac5

File tree

11 files changed

+215
-17
lines changed

11 files changed

+215
-17
lines changed

‎package-lock.json‎

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"needle": "^2.4.0",
4545
"response-time": "^2.3.2",
4646
"snyk-config": "^2.2.0",
47-
"snyk-docker-plugin": "^1.33.0",
47+
"snyk-docker-plugin": "1.34.0",
4848
"source-map-support": "^0.5.9",
4949
"tslib": "^1.9.3",
5050
"ws": "^7.0.0",

‎snyk-monitor-deployment.yaml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ spec:
2525
readOnly: true
2626
mountPath: "/root/.docker"
2727
- name: temporary-storage
28-
mountPath: "/snyk-monitor"
28+
mountPath: "/var/tmp"
2929
env:
3030
- name: SNYK_INTEGRATION_ID
3131
valueFrom:
@@ -57,7 +57,11 @@ spec:
5757
limits:
5858
cpu: '1'
5959
memory: '2Gi'
60-
securityContext: {}
60+
securityContext:
61+
readOnlyRootFilesystem: true
62+
capabilities:
63+
drop:
64+
- ALL
6165
volumes:
6266
- name: docker-config
6367
secret:

‎snyk-monitor/templates/deployment.yaml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ spec:
3131
readOnly: true
3232
mountPath: "/root/.docker"
3333
- name: temporary-storage
34-
mountPath: "/snyk-monitor"
34+
mountPath: "/var/tmp"
3535
env:
3636
- name: SNYK_INTEGRATION_ID
3737
valueFrom:
@@ -51,6 +51,11 @@ spec:
5151
limits:
5252
cpu: '1'
5353
memory: '2Gi'
54+
securityContext:
55+
readOnlyRootFilesystem: true
56+
capabilities:
57+
drop:
58+
- ALL
5459
volumes:
5560
- name: docker-config
5661
secret:

‎src/common/config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ const config = require('snyk-config')(__dirname + '/../..', {
77
config.AGENT_ID = uuidv4();
88
config.INTEGRATION_ID = config.INTEGRATION_ID.trim();
99
config.CLUSTER_NAME = config.CLUSTER_NAME || 'Default cluster';
10-
config.IMAGE_STORAGE_ROOT = '/snyk-monitor';
10+
config.IMAGE_STORAGE_ROOT = '/var/tmp';
1111

1212
export = config;

‎src/kube-scanner/image-scanner.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,25 @@ import * as plugin from 'snyk-docker-plugin';
22
import logger = require('../common/logger');
33
import { IStaticAnalysisOptions, StaticAnalysisImageType } from './types';
44
import { IPullableImage } from '../images/types';
5+
import config = require('../common/config');
56

67
export interface IScanResult {
78
image: string;
89
imageWithTag: string;
910
pluginResult: any;
1011
}
1112

12-
function removeTagFromImage(imageWithTag: string): string {
13+
/**
14+
* Exported for testing
15+
*/
16+
export function removeTagFromImage(imageWithTag: string): string {
1317
return imageWithTag.split('@')[0].split(':')[0];
1418
}
1519

16-
function getImageTag(imageWithTag: string): string {
20+
/**
21+
* Exported for testing
22+
*/
23+
export function getImageTag(imageWithTag: string): string {
1724
const imageParts: string[] = imageWithTag.split(':');
1825
if (imageParts.length === 2) { // image@sha256:hash or image:tag
1926
return imageParts[1];
@@ -22,17 +29,17 @@ function getImageTag(imageWithTag: string): string {
2229
return '';
2330
}
2431

25-
function constructStaticAnalysisOptions(
26-
fileSystemPath: string | undefined,
32+
/**
33+
* Exported for testing
34+
*/
35+
export function constructStaticAnalysisOptions(
36+
fileSystemPath: string,
2737
): { staticAnalysisOptions: IStaticAnalysisOptions } {
28-
if (!fileSystemPath) {
29-
throw new Error('Missing path for image for static analysis');
30-
}
31-
3238
return {
3339
staticAnalysisOptions: {
3440
imagePath: fileSystemPath,
3541
imageType: StaticAnalysisImageType.DockerArchive,
42+
tmpDirPath: config.IMAGE_STORAGE_ROOT,
3643
},
3744
};
3845
}

‎src/kube-scanner/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export enum StaticAnalysisImageType {
2929
export interface IStaticAnalysisOptions {
3030
imagePath: string;
3131
imageType: StaticAnalysisImageType;
32+
tmpDirPath: string;
3233
}
3334

3435
export interface KubeObjectMetadata {

‎test/helpers/deployment.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import * as tap from 'tap';
2+
import { V1Deployment } from '@kubernetes/client-node';
3+
4+
export function validateSecureConfiguration(test: tap, deployment: V1Deployment) {
5+
if (
6+
!deployment.spec ||
7+
!deployment.spec.template.spec ||
8+
!deployment.spec.template.spec.containers ||
9+
deployment.spec.template.spec.containers.length === 0 ||
10+
!deployment.spec.template.spec.containers[0].securityContext
11+
) {
12+
test.fail('bad container spec or missing securityContext');
13+
return;
14+
}
15+
16+
const securityContext =
17+
deployment.spec.template.spec.containers[0].securityContext;
18+
19+
if (!securityContext.capabilities) {
20+
test.fail('missing capabilities section in pod securityContext');
21+
return;
22+
}
23+
24+
test.same(
25+
securityContext.capabilities.drop,
26+
['ALL'],
27+
'all capabilities are dropped',
28+
);
29+
30+
if (securityContext.capabilities.add) {
31+
test.false(
32+
securityContext.capabilities.add.includes('SYS_ADMIN'),
33+
'CAP_SYS_ADMIN not added',
34+
);
35+
}
36+
37+
test.ok(
38+
securityContext.readOnlyRootFilesystem === true,
39+
'readOnlyRootFilesystem is set',
40+
);
41+
}
42+
43+
export function validateVolumeMounts(test: tap, deployment: V1Deployment) {
44+
if (
45+
!deployment.spec ||
46+
!deployment.spec.template.spec ||
47+
!deployment.spec.template.spec.containers ||
48+
deployment.spec.template.spec.containers.length === 0 ||
49+
!deployment.spec.template.spec.containers[0].volumeMounts
50+
) {
51+
test.fail('bad container spec or missing volumeMounts');
52+
return;
53+
}
54+
55+
const volumeMounts = deployment.spec.template.spec.containers[0].volumeMounts;
56+
57+
const temporaryStorageMount = volumeMounts.find(
58+
(mount) => mount.name === 'temporary-storage',
59+
);
60+
if (!temporaryStorageMount) {
61+
test.fail('missing deployment mount "temporary-storage"');
62+
return;
63+
}
64+
65+
test.same(
66+
temporaryStorageMount.mountPath,
67+
'/var/tmp',
68+
'deployment file mounts temporary storage at the expected path',
69+
);
70+
71+
const dockerConfigMount = volumeMounts.find(
72+
(mount) => mount.name === 'docker-config',
73+
);
74+
if (!dockerConfigMount) {
75+
test.fail('missing deployment mount "docker-config"');
76+
return;
77+
}
78+
79+
test.same(
80+
dockerConfigMount.readOnly,
81+
true,
82+
'docker-config is a read-only mount',
83+
);
84+
85+
test.same(
86+
dockerConfigMount.mountPath,
87+
'/root/.docker',
88+
'docker-config mount path is as expected',
89+
);
90+
}

‎test/integration/kubernetes.test.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CoreV1Api, KubeConfig } from '@kubernetes/client-node';
1+
import { CoreV1Api, KubeConfig, AppsV1Api } from '@kubernetes/client-node';
22
import setup = require('../setup');
33
import * as tap from 'tap';
44
import { getKindConfigPath } from '../helpers/kind';
@@ -9,6 +9,7 @@ import {
99
validateHomebaseStoredMetadata,
1010
getHomebaseResponseBody,
1111
} from '../helpers/homebase';
12+
import { validateSecureConfiguration, validateVolumeMounts } from '../helpers/deployment';
1213

1314
let integrationId: string;
1415

@@ -162,3 +163,20 @@ tap.test(`snyk-monitor has resource limits`, async (t) => {
162163
t.ok(monitorResources.requests.cpu !== undefined, 'snyk-monitor has cpu resource request');
163164
t.ok(monitorResources.requests.memory !== undefined, 'snyk-monitor has memory resource request');
164165
});
166+
167+
tap.test('snyk-monitor secure configuration is as expected', async (t) => {
168+
const kindConfigPath = await getKindConfigPath();
169+
const kubeConfig = new KubeConfig();
170+
kubeConfig.loadFromFile(kindConfigPath);
171+
172+
const k8sApi = kubeConfig.makeApiClient(AppsV1Api);
173+
174+
const response = await k8sApi.readNamespacedDeployment(
175+
'snyk-monitor',
176+
'snyk-monitor',
177+
);
178+
const deployment = response.body;
179+
180+
validateSecureConfiguration(t, deployment);
181+
validateVolumeMounts(t, deployment);
182+
});

‎test/unit/deployment-files.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import * as tap from 'tap';
2+
import { parse } from 'yaml';
3+
import { readFileSync } from 'fs';
4+
import { V1Deployment } from '@kubernetes/client-node';
5+
import * as snykConfig from '../../src/common/config';
6+
import { validateSecureConfiguration, validateVolumeMounts } from '../helpers/deployment';
7+
8+
/**
9+
* Note that these checks are also performed at runtime on the deployed snyk-monitor, see the integration tests.
10+
*/
11+
12+
tap.test('ensure the security properties of the deployment files are unchanged', async (t) => {
13+
t.same(snykConfig.IMAGE_STORAGE_ROOT, '/var/tmp', 'the snyk-monitor points to the correct mounted path');
14+
15+
const deploymentFiles = ['./snyk-monitor/templates/deployment.yaml', './snyk-monitor-deployment.yaml'];
16+
17+
for (const filePath of deploymentFiles) {
18+
const fileContent = readFileSync(filePath, 'utf8');
19+
const deployment: V1Deployment = parse(fileContent);
20+
21+
validateSecureConfiguration(t, deployment);
22+
validateVolumeMounts(t, deployment);
23+
}
24+
});

0 commit comments

Comments
 (0)