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

Support for traditional OpenSSL and PKCS8 RSA private key serialization #1503

Merged
merged 10 commits into from
Mar 1, 2015
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ Changelog
support loading DER encoded public keys.
* Fixed building against LibreSSL, a compile-time substitute for OpenSSL.
* FreeBSD 9.2 was removed from the continuous integration system.
* Added
:class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKeyWithSerialization`
and deprecated
:class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKeyWithNumbers`.
* Added
:meth:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKeyWithSerialization.as_bytes`
to
:class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKeyWithSerialization`.

0.7.2 - 2015-01-16
~~~~~~~~~~~~~~~~~~
Expand Down
70 changes: 70 additions & 0 deletions docs/hazmat/primitives/asymmetric/rsa.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,39 @@ password. If the key is encrypted we can pass a ``bytes`` object as the
There is also support for :func:`loading public keys in the SSH format
<cryptography.hazmat.primitives.serialization.load_ssh_public_key>`.

Key serialization
~~~~~~~~~~~~~~~~~

If you have a key that you've loaded or generated which implements the
:class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKeyWithSerialization`
interface you can use
:meth:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKeyWithSerialization.as_bytes`
to serialize the key.

.. doctest::

>>> from cryptography.hazmat.primitives import serialization
>>> pem = private_key.as_bytes(
... encoding=serialization.Encoding.PEM,
... format=serialization.Format.PKCS8,
... encryption_algorithm=serialization.BestAvailableEncryption(b'mypassword')
... )
>>> pem.splitlines()[0]
'-----BEGIN ENCRYPTED PRIVATE KEY-----'

It is also possible to serialize without encryption using
:class:`~cryptography.hazmat.primitives.serialization.NoEncryption`.

.. doctest::

>>> pem = private_key.as_bytes(
... encoding=serialization.Encoding.PEM,
... format=serialization.Format.TraditionalOpenSSL,
... encryption_algorithm=serialization.NoEncryption()
... )
>>> pem.splitlines()[0]
'-----BEGIN RSA PRIVATE KEY-----'

Signing
~~~~~~~

Expand Down Expand Up @@ -485,6 +518,43 @@ Key interfaces
instance.


.. class:: RSAPrivateKeyWithSerialization

.. versionadded:: 0.8

Extends :class:`RSAPrivateKey`.

.. method:: private_numbers()

Create a
:class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateNumbers`
object.

:returns: An
:class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateNumbers`
instance.

.. method:: as_bytes(encoding, format, encryption_algorithm)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why private_numbers but not private_bytes? This seems inconsistent.

Would private_as_numbers / private_as_bytes make sense?


Allows serialization of the key to bytes. Encoding (PEM or DER), format
(TraditionalOpenSSL or PKCS8) and encryption algorithm (such as
Copy link
Member

Choose a reason for hiding this comment

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

Can we linkify PEM, DER, TraditionalOpenSSL, and PKCS8

:class:`~cryptography.hazmat.primitives.serialization.BestAvailableEncryption`
or :class:`~cryptography.hazmat.primitives.serialization.NoEncryption`)
are chosen to define the exact serialization.

:param encoding: A value from the
:class:`~cryptography.hazmat.primitives.serialization.Encoding` enum.

:param format: A value from the
:class:`~cryptography.hazmat.primitives.serialization.Format` enum.

:param encryption_algorithm: An instance of an object conforming to the
:class:`~cryptography.hazmat.primitives.serialization.KeySerializationEncryption`
interface.

:return bytes: Serialized key.


.. class:: RSAPublicKey

.. versionadded:: 0.2
Expand Down
65 changes: 63 additions & 2 deletions docs/hazmat/primitives/asymmetric/serialization.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Key Serialization
=================

.. currentmodule:: cryptography.hazmat.primitives.serialization
.. module:: cryptography.hazmat.primitives.serialization

.. testsetup::

Expand Down Expand Up @@ -75,7 +75,7 @@ methods.
.. doctest::

>>> from cryptography.hazmat.backends import default_backend
>>> from cryptography.hazmat.primitives.asymmetric import dsa, rsa
>>> from cryptography.hazmat.primitives.asymmetric import rsa
Copy link
Member

Choose a reason for hiding this comment

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

This change should also be rolled back, DSA is needed here.

>>> from cryptography.hazmat.primitives.serialization import load_pem_private_key
>>> key = load_pem_private_key(pem_data, password=None, backend=default_backend())
>>> if isinstance(key, rsa.RSAPrivateKey):
Expand Down Expand Up @@ -282,3 +282,64 @@ DSA keys look almost identical but begin with ``ssh-dss`` rather than

:raises cryptography.exceptions.UnsupportedAlgorithm: If the serialized
key is of a type that is not supported.

Serialization Formats
~~~~~~~~~~~~~~~~~~~~~

.. class:: Format

.. versionadded:: 0.8

An enumeration for key formats.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like a little bit denser linking might be helpful for someone reading this documentation. For example, a link here back to RSAPrivateKeyWithSerialization to say that that's where this enumeration is used would be helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Probably specifically link to as_bytes, though that's going to get bloaty when we have 6 classes to link to.

(Let's include the link for now and revisit as we add more implementations)


.. attribute:: TraditionalOpenSSL

Frequently known as PKCS#1 format. Still a widely used format, but
generally considered legacy.

.. attribute:: PKCS8
Copy link
Member

Choose a reason for hiding this comment

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

More words are needed here. Should probably specifically mention that where possible PKCS8 is prefered since it allows better encryption algorithms.


A more modern format for serializing keys which allows for better
encryption. Choose this unless you have explicit legacy compatibility
requirements.

Serialization Encodings
~~~~~~~~~~~~~~~~~~~~~~~

.. class:: Encoding

.. versionadded:: 0.8

An enumeration for encoding types.

.. attribute:: PEM

For PEM format. This is a base64 format with delimiters.

.. attribute:: DER

For DER format. This is a binary format.


Serialization Encryption Types
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. class:: KeySerializationEncryption

Objects with this interface are usable as encryption types with methods
like
:meth:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKeyWithSerialization.as_bytes`.
All other classes in this section represent the available choices for
encryption and have this interface.

.. class:: BestAvailableEncryption(password)

Encrypt using the best available encryption for a given key's backend.
This is a curated encryption choice and the algorithm may change over
time.

:param bytes password: The password to use for encryption.

.. class:: NoEncryption

Do not encrypt.
2 changes: 2 additions & 0 deletions docs/spelling_wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ pseudorandom
pyOpenSSL
Schneier
scrypt
Serializers
serializer
Solaris
Tanja
testability
Expand Down
66 changes: 64 additions & 2 deletions src/cryptography/hazmat/backends/openssl/rsa.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@
from cryptography.hazmat.primitives.asymmetric.padding import (
AsymmetricPadding, MGF1, OAEP, PKCS1v15, PSS
)
from cryptography.hazmat.primitives.interfaces import (
RSAPrivateKeyWithNumbers, RSAPublicKeyWithNumbers
from cryptography.hazmat.primitives.asymmetric.rsa import (
RSAPrivateKeyWithNumbers, RSAPrivateKeyWithSerialization,
RSAPublicKeyWithNumbers
)
from cryptography.hazmat.primitives.serialization import (
BestAvailableEncryption, Encoding, Format, KeySerializationEncryption,
NoEncryption
)


Expand Down Expand Up @@ -507,6 +512,7 @@ def _verify_pss(self, evp_md):


@utils.register_interface(RSAPrivateKeyWithNumbers)
@utils.register_interface(RSAPrivateKeyWithSerialization)
class _RSAPrivateKey(object):
def __init__(self, backend, rsa_cdata):
self._backend = backend
Expand Down Expand Up @@ -559,6 +565,62 @@ def private_numbers(self):
)
)

def as_bytes(self, encoding, format, encryption_algorithm):
if not isinstance(encoding, Encoding):
raise TypeError("encoding must be an item from the Encoding enum")
Copy link
Contributor

Choose a reason for hiding this comment

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

Experience has taught me that any error message like this really ought to have an "… instead got …" portion which explains what object was received instead. Otherwise debugging can be agonizing - "I only get this exception in production, my unit tests aren't catching it, what am I doing? am I reversing the order of arguments, is something naming a variable wrong..."

Copy link
Member

Choose a reason for hiding this comment

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

We don't really do that /anywhere/ else right now. If we want to do that we
should do it everywhere IMO. (Also, this is trivial to debug with sentry,
just sayin)

On Sat, Feb 28, 2015 at 12:50 PM, Glyph notifications@github.com wrote:

In src/cryptography/hazmat/backends/openssl/rsa.py
#1503 (comment):

@@ -559,6 +565,62 @@ def private_numbers(self):
)
)

  • def as_bytes(self, encoding, format, encryption_algorithm):
  •    if not isinstance(encoding, Encoding):
    
  •        raise TypeError("encoding must be an item from the Encoding enum")
    

Experience has taught me that any error message like this really ought
to have an "… instead got …" portion which explains what object was
received instead. Otherwise debugging can be agonizing - "I only get this
exception in production, my unit tests aren't catching it, what am I doing?
am I reversing the order of arguments, is something naming a variable
wrong..."


Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/pull/1503/files#r25561072.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084


if not isinstance(format, Format):
raise TypeError("format must be an item from the Format enum")

# This is a temporary check until we land DER serialization.
if encoding is not Encoding.PEM:
raise ValueError("Only PEM encoding is supported by this backend")

if format is Format.PKCS8:
write_bio = self._backend._lib.PEM_write_bio_PKCS8PrivateKey
key = self._evp_pkey
elif format is Format.TraditionalOpenSSL:
write_bio = self._backend._lib.PEM_write_bio_RSAPrivateKey
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be clearer to write this as:

if isinstance(s, PKCS8):
    # ...
elif isinstance(s, TraditionalOpenSSL):
    # ...
else:
   raise TypeError("...")

Copy link
Member

Choose a reason for hiding this comment

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

Still applicable.

key = self._rsa_cdata

if not isinstance(encryption_algorithm, KeySerializationEncryption):
raise TypeError(
"Encryption algorithm must be a KeySerializationEncryption "
"instance"
)

if isinstance(encryption_algorithm, NoEncryption):
password = b""
passlen = 0
evp_cipher = self._backend._ffi.NULL
elif isinstance(encryption_algorithm, BestAvailableEncryption):
# This is a curated value that we will update over time.
evp_cipher = self._backend._lib.EVP_get_cipherbyname(
b"aes-256-cbc"
)
password = encryption_algorithm.password
passlen = len(password)
if passlen > 1023:
raise ValueError(
"Passwords longer than 1023 bytes are not supported by "
"this backend"
)
else:
raise ValueError("Unsupported encryption type")

bio = self._backend._create_mem_bio()
res = write_bio(
bio,
key,
evp_cipher,
password,
passlen,
self._backend._ffi.NULL,
self._backend._ffi.NULL
)
assert res == 1
return self._backend._read_mem_bio(bio)


@utils.register_interface(RSAPublicKeyWithNumbers)
class _RSAPublicKey(object):
Expand Down
19 changes: 18 additions & 1 deletion src/cryptography/hazmat/primitives/asymmetric/rsa.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,30 @@ def public_key(self):


@six.add_metaclass(abc.ABCMeta)
class RSAPrivateKeyWithNumbers(RSAPrivateKey):
class RSAPrivateKeyWithSerialization(RSAPrivateKey):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there really a reason that this needs to be a subclass? Practically speaking, things which implement key serialization will also themselves be a key, but for testing or format-translation or other esoteric use-cases, mightn't there be a serialization provider with no actual decrypt? Is there a reason to prevent that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really no. RSAPrivateKeyWithNumbers is already a subclass so I'd oppose changing that, but we could certainly choose to make its successor (WithSerialization) not a subclass and simply register both interfaces. The numbers classes are (I believe) the only place in cryptography that we utilize inheritance.

@alex, @public got opinions here? I'm happy to go either way.

Copy link
Member

Choose a reason for hiding this comment

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

Leave the subclassing orthagonal IMO; no need to change more things than
our necessary with this patch.

On Sat, Feb 28, 2015 at 4:45 PM, Paul Kehrer notifications@github.com
wrote:

In src/cryptography/hazmat/primitives/asymmetric/rsa.py
#1503 (comment):

@@ -42,13 +42,30 @@ def public_key(self):

@six.add_metaclass(abc.ABCMeta)
-class RSAPrivateKeyWithNumbers(RSAPrivateKey):
+class RSAPrivateKeyWithSerialization(RSAPrivateKey):

Not really no. RSAPrivateKeyWithNumbers is already a subclass so I'd
oppose changing that, but we could certainly choose to make its successor (
WithSerialization) not a subclass and simply register both interfaces.
The numbers classes are (I believe) the only place in cryptography that we
utilize inheritance.

@alex https://github.com/alex, @public https://github.com/public got
opinions here? I'm happy to go either way.


Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/pull/1503/files#r25562882.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough; In that case this PR is once again ready for review. I added additional cross-linking and changed the name of as_bytes to private_bytes at @glyph's suggestion.

@abc.abstractmethod
def private_numbers(self):
"""
Returns an RSAPrivateNumbers.
"""

@abc.abstractmethod
def as_bytes(self, encoding, format, encryption_algorithm):
"""
Returns the key serialized as bytes.
"""


RSAPrivateKeyWithNumbers = utils.deprecated(
RSAPrivateKeyWithSerialization,
__name__,
(
"The RSAPrivateKeyWithNumbers interface has been renamed to "
"RSAPrivateKeyWithSerialization"
),
utils.DeprecatedIn08
)


@six.add_metaclass(abc.ABCMeta)
class RSAPublicKey(object):
Expand Down
5 changes: 3 additions & 2 deletions src/cryptography/hazmat/primitives/interfaces/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,12 @@
)

RSAPrivateKeyWithNumbers = utils.deprecated(
rsa.RSAPrivateKeyWithNumbers,
rsa.RSAPrivateKeyWithSerialization,
__name__,
(
"The RSAPrivateKeyWithNumbers interface has moved to the "
"cryptography.hazmat.primitives.asymmetric.rsa module"
"cryptography.hazmat.primitives.asymmetric.rsa module and has been "
"renamed RSAPrivateKeyWithSerialization"
),
utils.DeprecatedIn08
)
Expand Down
32 changes: 32 additions & 0 deletions src/cryptography/hazmat/primitives/serialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@

from __future__ import absolute_import, division, print_function

import abc
import base64
import struct
from enum import Enum

import six

from cryptography import utils
from cryptography.exceptions import UnsupportedAlgorithm
from cryptography.hazmat.primitives.asymmetric import dsa, ec, rsa

Expand Down Expand Up @@ -164,3 +167,32 @@ def _int_from_bytes(data, byteorder, signed=False):
data = data[4:]

return result


class Encoding(Enum):
PEM = "PEM"
DER = "DER"


class Format(Enum):
PKCS8 = "PKCS8"
TraditionalOpenSSL = "TraditionalOpenSSL"


@six.add_metaclass(abc.ABCMeta)
class KeySerializationEncryption(object):
pass


@utils.register_interface(KeySerializationEncryption)
class BestAvailableEncryption(object):
def __init__(self, password):
if not isinstance(password, bytes) or len(password) == 0:
raise ValueError("Password must be 1 or more bytes.")

self.password = password


@utils.register_interface(KeySerializationEncryption)
class NoEncryption(object):
pass
Loading