Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws에서 security group 정보 가져오는 API #2

Merged
merged 4 commits into from
Oct 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
*.rest
# OS X
.DS_Store*
Icon?
Expand All @@ -24,4 +25,4 @@ package-lock.json
coverage

# Benchmarking
benchmarks/graphs
benchmarks/graphs
23 changes: 23 additions & 0 deletions src/entities/PromiseStatus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export enum promiseStatus {
"OK",
"FATAL",
"ERROR"
}

export interface IPromiseStatus {
result: promiseStatus;
message: string;
}

class PromiseType implements IPromiseStatus {

public result: promiseStatus;
public message: string;

constructor(result: number, message: string) {
this.result = result
this.message = message
}
}

export default PromiseType;
23 changes: 23 additions & 0 deletions src/helper/shell.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import PromiseType, { promiseStatus } from '@entities/PromiseStatus';

async function execShellCommand(cmd:string):Promise<PromiseType> {
const exec = require('child_process').exec;
return new Promise((resolve, reject) => {
exec(cmd, (error, stdout, stderr) => {
if (error) {
console.log(error)
reject(new PromiseType(promiseStatus.FATAL, error));
}
if (stdout) {
console.log(stdout)
resolve(new PromiseType(promiseStatus.OK, stdout))
}
if (stderr) {
console.log(stderr)
reject(new PromiseType(promiseStatus.ERROR, stderr))
}
});
});
}

export {execShellCommand};
3 changes: 2 additions & 1 deletion src/routes/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { Router } from 'express';
import UserRouter from './Users';

import AwsRouterV1 from './v1/Aws';
// Init router and path
const router = Router();

// Add sub-routes
router.use('/users', UserRouter);
router.use('/v1/aws', AwsRouterV1)

// Export the base-router
export default router;
28 changes: 28 additions & 0 deletions src/routes/v1/Aws.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import PromiseType from '@entities/PromiseStatus';
import { Request, Response, Router } from 'express';
import { execShellCommand } from "../../helper/shell";

// Init shared
const router = Router();


router.get('/securitygroups', async (req: Request, res: Response) => {
const { access_key, secret_key } = req.headers;
const { region } = req.query

let ret:PromiseType = await execShellCommand(`docker-compose -f helper/docker-compose.yml run \
-e AWS_DEFAULT_REGION=${region || 'us-east-1'} \
-e AWS_ACCESS_KEY_ID=${access_key} \
-e AWS_SECRET_ACCESS_KEY=${secret_key} \
--rm aws ec2 describe-security-groups \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커맨드 쉘 실행에서 값 검증 없이 그냥 넣으면 injection이 가능할 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

백엔드 프론트엔드가 같은 CORS를 적용해서 프론트쪽에서 믿을 수 있는 입력값을 보내는 경우도 injection 가능성이 있나요??

그리고 혹시 공격예시 알려주실 수 있을까요?

execShellCommand(param)

param에 인젝션 스트링을 넣어서 공격한다는걸로 이해했는데 맞나요?
제가 보안을 잘 몰라서 ㅎㅎ..
맞다면 docker 내부에서 모든 명령어가 실행되므로 안전하지 않나요?

Copy link
Member

@nnnlog nnnlog Oct 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

image

인자를 그대로 넣었기 때문에 || 로 도커의 커맨드를 자르고, 원하는 커맨드를 넣어 실행할 수 있을 것 같아요. (뒤에 남은 도커 인자들은 /dev/null로 보내서 자를 수 있고)
CORS와는 상관없이 지금은 저희만 사용하지만, 로그인/회원가입을 넣으면 로그인한 사용자는 저 요청을 날릴 수 있는 API인거 같아서요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이해했습니다 설명 감사합니다 👍

regex로 영문, 숫자, - 를 제외한 문자들이 오면 커맨드 실행 안하고 400 리턴 해주려고 하는데 효과가 있는 방법일까요?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regex로 거르고 400 Bad Request 반환해주면 좋을 것 같아요
저 access key들이 다른 API에서 공통으로 사용된다면 미들웨어로 api 라우터에 가기 전에 앞단에서 거르는게 제일 좋을 것 같아요

Copy link
Contributor Author

@KimKiHyuk KimKiHyuk Oct 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네 좋은 생각이십니다.
일단 테스트 단계에서 access_key는 거의 대부분 API가 공통으로 사용할테니 미들웨어 걸어서 regex로 체크하는 방법으로 변경하겠습니다 :)

`);
if (ret.result == 1 || ret.result == 2) {
return res.status(400).end(ret.message);
}

return res.status(200).send(JSON.parse(ret.message));
});


export default router;

2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"outDir": "dist",
"removeComments": true,
"sourceMap": true,
"strict": true,
"strict": false,
"target": "es6",
"moduleResolution": "node",
"paths": {
Expand Down