PublicKeyAlgorithm does not encode None parameters as ASN1 Null element for RSA Keys

[andrewhop]

PublicKeyAlgorithm has an optional parameters, RSA keys do not specify any parameters and when asn1crypto encodes an RSA key with None parameters it omits the parameters entirely. This is incorrect according to rfc4055 section 1.2 which states "the parameters field MUST contain NULL". This seems to be a known requirement but this isn't injecting the expected NULL ASN1 element.

import base64
from asn1crypto.keys import RSAPublicKey, PublicKeyAlgorithm, PublicKeyInfo
algorithm = PublicKeyAlgorithm({'algorithm': 'rsa', 'parameters': None})
print(base64.b64encode(algorithm.dump()))

Prints:

MAsGCSqGSIb3DQEBAQ==
which decodes as
SEQUENCE (1 elem)
  OBJECT IDENTIFIER 1.2.840.113549.1.1.1 rsaEncryption (PKCS #1)

Expected output:

MA0GCSqGSIb3DQEBAQUA
which decodes as
SEQUENCE (2 elem)
  OBJECT IDENTIFIER 1.2.840.113549.1.1.1 rsaEncryption (PKCS #1)
  NULL

The incorrect output can cause downstream consumers to fail to parse the output from asn1crypto.

However, if I pass in asn1crypto's Null() it does work. Is it expected that I need to pass in Null when construction the PublicKeyALgorithm?

algorithm = PublicKeyAlgorithm({'algorithm': 'rsa', 'parameters': Null()})

[geitda]

You're conflating the ASN.1 types NULL and VOID with each other.
The Python None type corresponds to ASN.1 type VOID and produces no output when dumped.
The Python Null type (unsurprisingly) corresponds to ASN.1 type NULL and has some logic built into it so that it DOES dump an actual NULL some of the time. Classes that inherit _ForceNullParameters (see algos.py) will be handled correctly.

Imagine that you really do want to force the generation of the PublicKeyAlgorithm to omit the parameters. You MUST pass Python None to get that behavior.

# Defaults to Null when omitted
>>> PublicKeyAlgorithm({'algorithm': 'rsa'})['parameters']
<asn1crypto.core.Null 3026224464848 b'\x05\x00'>
# Explicitly providing None forces generating Void
>>> PublicKeyAlgorithm({'algorithm': 'rsa', 'parameters': None})['parameters']
<asn1crypto.core.Void 3026223057680 b''>
# Your last example explicitly passing Null() works the same as when omitted
>>> PublicKeyAlgorithm({'algorithm': 'rsa', 'parameters': Null()})['parameters']
<asn1crypto.core.Null 3026224466576 b'\x05\x00'>

[geitda]

So I found where this behavior can be non-intuitive: round-tripping or copying from one object to another with native OrderedDict representation.
Although the three examples above are all correct, you can elicit some breakage if you "serialize" the object to native, and reconstruct.

>>> omitted = PublicKeyAlgorithm({'algorithm': 'rsa'})
>>> round_tripped = PublicKeyAlgorithm(omitted.native) # This should be identical, right?
>>> omitted.dump() == round_tripped.dump()
False
>>> omitted.native # Here's why
OrderedDict([('algorithm', 'rsa'), ('parameters', None)])

The native representation has to use a None to describe the lack of 'parameters' (as the ASN.1 schema says it's optional).
But if you reconstruct a PublicKeyAlgorithm object with the explicit None it will instantiate with a Void, leaving it out entirely. If you need to use the constructor, be mindful.
I'm not sure if there's anything to be fixed within asn1crypto as the three situations (omitted, None, or Null()) are behaving the way I'd expect them to, it's just a quirk that round-tripping won't be identical.

# If you rely on native representation, it WILL be lossy
>>> type(PublicKeyAlgorithm({'algorithm': 'rsa'})['parameters'].native)
<class 'NoneType'>
>>> type(PublicKeyAlgorithm({'algorithm': 'rsa', 'parameters': None})['parameters'].native)
<class 'NoneType'>
>>> type(PublicKeyAlgorithm({'algorithm': 'rsa', 'parameters': Null()})['parameters'].native)
<class 'NoneType'>

[joernheissler]

The "native" representation isn't intended to retain all information:

>>> GraphicString("Hello").dump() == VisibleString("Hello").dump()
False
>>> GraphicString("Hello").native == VisibleString("Hello").native
True

If you're writing code that needs to emit different algorithms, either leave out the parameters when not needed, or always specify them with the correct value.

>>> PublicKeyAlgorithm({"algorithm": "rsa"}).dump().hex()
'300d 0609 2a864886f70d010101 0500'
>>> PublicKeyAlgorithm({"algorithm": "ed25519"}).dump().hex()
'3005 0603 2b6570'

ASN.1 and its encodings are confusing and error-prone, the standards (like X.509) building upon it even more so.

Perhaps documentation could be improved, e.g. a "common pitfalls" page.