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

Validate rsa private key and certificate content format #971

Conversation

DevDengChao
Copy link
Contributor

Fix #969.

const { green, red } = require('colors');
const { processApiParameters } = require('./deploy-support-api');
const { getCloudApiClient, getSlsClient, getMnsClient } = require('../client');
const {green, red} = require('colors');
Copy link
Collaborator

Choose a reason for hiding this comment

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

eslint 是如何配置的?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我是在 Windows 上用 WebStorm 开发的, 未修改 .eslintrc 的内容, 对 deploy-support.js 使用 .eslintrc 的格式进行格式化的.
测试后发现有没有空格 eslint 都没有报错.
TIM截图20200714105210
TIM截图20200714105225

需要我回滚格式化代码的 commit 么?

let p = path.resolve(__dirname, privateKey);
// private key is provided by local file
if (fs.pathExistsSync(p)) {
certConfig.PrivateKey = fs.readFileSync(p, 'utf-8');
Copy link
Collaborator

@ChanDaoH ChanDaoH Jul 14, 2020

Choose a reason for hiding this comment

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

fs.readFileSync(p, 'utf-8') => await fs.readFile(p, 'utf-8')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

let p = path.resolve(__dirname, certificate);
// certificate is provided by local file
if (fs.pathExistsSync(p)) {
certConfig.Certificate = fs.readFileSync(p, 'utf-8');
Copy link
Collaborator

Choose a reason for hiding this comment

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

fs.readFileSync(p, 'utf-8') => await fs.readFile(p, 'utf-8')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

//endregion

//region validate RSA private key content
let expectedPrefix = '-----BEGIN RSA PRIVATE KEY-----', expectedSuffix = '-----END RSA PRIVATE KEY-----';
Copy link
Collaborator

Choose a reason for hiding this comment

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

写在 const 里面吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

//endregion

//region validate certificate content
let expectedPrefix = '-----BEGIN CERTIFICATE-----', expectedSuffix = '-----END CERTIFICATE-----';
Copy link
Collaborator

Choose a reason for hiding this comment

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

写在 const 里面吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

let path = require('path');
const expect = require('expect.js');

describe('fs-extra module Tests', function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个测试集的作用是?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • 为了确认 fs-extra 的 pathExists 方法对相对路径的支持方式, 目前确认到的情况是需要使用 path.resolve 先对路径进行解析才行.
  • 为了确认 fs-extra 的 pathExists 方法对 PrivateKey 以及 Certificate 是硬编码的 PEM 内容时的返回结果.

需要添加注释? 还是删掉这个测试集?

@DevDengChao DevDengChao requested a review from ChanDaoH July 14, 2020 03:27
@DevDengChao
Copy link
Contributor Author

🤔

1 similar comment
@DevDengChao
Copy link
Contributor Author

🤔

@CLAassistant
Copy link

CLAassistant commented Jan 26, 2022

CLA assistant check
All committers have signed the CLA.

@DevDengChao
Copy link
Contributor Author

Closed as #1146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

建议变更 PEM 证书的校验规则
3 participants