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

SM2-PEM密钥读取写入 #4

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

SM2-PEM密钥读取写入 #4

wants to merge 14 commits into from

Conversation

rainune
Copy link

@rainune rainune commented Mar 12, 2022

  1. 增加 PEM密钥读取与生成PEM密钥,密钥格式兼容OpenSSL.
  2. 增加了der.py, 参考了 ecdsa.
  3. 增加了test_sm2_der.py, 对新增功能进行测试.
  4. encrypt 和 decrypt 的API删除了公私钥的参数,不在需要明文输入,如果确实需要,可以直接赋值。这部分如果不符合软件整体框架的话可以在修改一下。

@wptoux wptoux added the enhancement New feature or request label Mar 12, 2022
@wptoux
Copy link
Owner

wptoux commented Mar 12, 2022

感谢贡献~

PyLint 报错,需要修改下。

SM2的接口感觉还是保持原样比较好,主要是出于兼容性的考虑,目前已经有人在用这个库了,升级之后需要不能break。所以也需要修改下。

.gitignore Outdated
@@ -113,3 +113,8 @@ dmypy.json

# Pyre type checker
.pyre/

Copy link
Owner

Choose a reason for hiding this comment

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

感觉这些gitingore似乎不太必要?

Copy link
Author

Choose a reason for hiding this comment

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

sorry, 这里面包含了我测试的一些东西

return decrypt(self._sk, data, self._mode)

def pk_from_pem(self, pem):
Copy link
Owner

Choose a reason for hiding this comment

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

我觉得这里弄成这样的两个方法比较直观,当然还要加一些边界检查啥的。之所以把sk和pk暴露在外面,主要是出于防呆的考虑,因为有时候只需要其中的一个key,如果传错了还挺麻烦的,如果方法里边判断抛异常也不太友好。

def load_pem(self, pem, need_sk=False):
    if need_sk:
        return sm2_pk_from_pem(pem)
    else:
        return sm2_sk_from_pem(pem)

def dump_pem(self, sk, pk):
    if sk:
        sm2_pk_to_der(pk)
    else:
        sm2_sk_to_der(sk, pk)

Copy link
Author

Choose a reason for hiding this comment

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

按照你的建议,重新设计了一下API:

@classmethod
    def load_pem(cls, pem,  is_sk = False):
@classmethod
    def dump_pem(cls, sk, pk, is_sk = False):

将两个函数设计成classmethod, 分离与SM2类的关系。
加载PEM的时候,需要用户明确该PEM是私钥或公钥;
输出PEM的时候,需要用户明确该次输出是私钥或公钥。函数内部的话,做参数的检查,来看是否满足输出的条件。
这样就不会影响原有的所有API接口,你看是否可行?

Copy link
Owner

Choose a reason for hiding this comment

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

classmethod挺好的。但是感觉is_sk有点奇怪,load_pem在is_sk的情况下,会同时读出来sk和pk,这会不会不符合一般常识。建议改成contains_sk。

dump_pem的is_sk似乎也有点冗余,看起来只有两种情况,同时有sk和pk,以及只有pk,是否不需要传is_sk这个参数呢?直接按下面这个逻辑:

def dump_pem(cls, sk=None, pk=None):
    if pk is not None:
        if sk is not None:
            sm2_sk_to_der(sk, pk)
        else:
            sm2_pk_to_der(pk)
    else:
        raise ValueError('Pem must contains pk')

Copy link
Author

Choose a reason for hiding this comment

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

  1. is_sk的目的是想要用户明确导入的密钥是私钥或者公钥,以便于接下来进行处理. contain_sk 好像也没表达出想要的意思,这里是否用 pem_type参数代替is_sk?
  2. PEM私钥是同时包含私钥和公钥信息的。嗯...我考虑的是如果只返回私钥,可能会和OpenSSL这样的主流密码库的操作逻辑不太一样,容易引发一些别的误解。这里可以处理为只返回私钥。
  3. 对于dump_dem函数。 通过sk是可以推导出pk的,所以,输出私钥PEM的时候是可以只传入sk.
    现在有两个思路,

(1)通过*type参数由用户明确PEM或key的类型

@classmethod
    def load_pem(cls, pem, pemtype = 'pk'):
         ...
         return key #只返回一个key,sk 或 pk
@classmethod
    def dump_pem(cls, key, keytype = 'pk'):

(2)当然,如果彻底去掉参数 *type也是可以的,可以在函数内部进行判断。私钥公钥及其PEM都是比较好区分的。

@classmethod
    def load_pem(cls, pem):
         ...
         return key #只返回一个key,sk 或 pk
@classmethod
    def dump_pem(cls, key):

但是第二种方法感觉有点奇怪 😄 我等讨论完API,再去修改一下代码...

openssl ecparam -genkey -name SM2 -out sk.pem
openssl ec -in sk.pem -pubout -out pk.pem
'''
with open('sk.pem') as f:
Copy link
Owner

Choose a reason for hiding this comment

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

这里能否考虑动态生成pem呢?另外请确认下这个pem里面是否包含您个人或单位的信息。

Copy link
Author

Choose a reason for hiding this comment

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

怎么样动态生成pem啊?

@rainune rainune requested a review from wptoux March 12, 2022 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants