Skip to content

Conversation

munagekar
Copy link
Contributor

>>> import ecdsa
>>> private_key = ecdsa.SigningKey.generate(curve=ecdsa.NIST256p)
>>> pub_key_pem_bytes = private_key.get_verifying_key().to_pem()
>>> type(pub_key_pem_bytes)
<class 'bytes'>

@munagekar munagekar requested a review from tomato42 January 17, 2020 06:45
@coveralls
Copy link

coveralls commented Jan 17, 2020

Coverage Status

Coverage decreased (-0.05%) to 96.989% when pulling ac431c5 on munagekar:master into e24c027 on warner:master.

@tomato42
Copy link
Member

yes, the documentation indeed is incorrect

that being said, the PEM encoding should be str on python3 as both the headers as well as the base64 encoding inside are supposed to use human-readable characters (i.e. when PEM specifies "A" it means Unicode 0x0041 LATIN CAPITAL LETTER A, not a single byte with value 65)

fixing it would break API so it's not something I'd like to do, so I guess we need to live with it, but I'd prefer to see a warning in the doc string that the PEM byte string returned is already encoded to US-ASCII so it needs to be re-encoded if the system or protocol in which it is used in is not compatible with US-ASCII (e.g. uses UTF-16 or EBCDIC)

Copy link
Member

@tomato42 tomato42 left a comment

Choose a reason for hiding this comment

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

please add a warning about the bytes being returned

@munagekar
Copy link
Contributor Author

Thank you for the review. I have added a warning.

@munagekar munagekar requested a review from tomato42 January 17, 2020 15:16
@tomato42 tomato42 merged commit c00a19a into tlsfuzzer:master Jan 17, 2020
@tomato42
Copy link
Member

thanks!

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.

3 participants