Support otherName in subAltName in CSR for UTF8 strings

[jborean93]

SUMMARY

Add support for specifying an otherName: subject alt name in the UTF8String type for a CSR. Other types could be supported in the future but this covers one of the major ones for this field.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

openssl_csr

[felixfontein]

@jborean93 I now merged #9 which implements a hex version of otherName. I guess the best would be if both ways to specify otherName would be possible (hex and UTF8 string). Do you want to rebase your PR accordingly, or should I do that?

[jborean93]

Sorry I completely forgot I had this, will look at rebasing and fixing the tests tomorrow.

[jborean93]

Found some time this morning, let me know what you think of the changes.

[MarkusTeufelberger]

Do we really have to implement an ASN1 parser for this? I would expect this stuff to be part of cryptography. From a quick glance, https://cryptography.io/en/latest/x509/reference/?highlight=csr#cryptography.x509.OtherName seems to be the class implementing this field.

[jborean93]

The code is using that OtherName class but the trouble is you need to specify the bytes as an ASN.1 encoded value. If you do find the logic to do this in cryptography I’ll definitely prefer to use that but I haven’t seen one yet.

[jborean93]

Also the current code that was recently added requires a user to specify the ASN.1 value as a hex string which isn’t really that easy for an end user to do. This PR allows people to create a CSR using a human readable value that is in the same format that OpenSSL supports

[MarkusTeufelberger]

https://github.com/pyca/cryptography/blob/a849f40556bd022c7478a44e935359c5fac83193/src/cryptography/hazmat/backends/openssl/decode_asn1.py sounds a bit like that, but I'm unsure how to use it...

The more I look into Cryptography, the more I wonder if we should start to work on improving things there as well... heck, they still can't even verify a certificate chain! :-(

[jborean93]

It could be an option, I would have to see if there is a public function available that exposes this and not something that seems to be a bit more private.

I'm also not sure how backends deal with availability on different distros. Is it guaranteed to be available or only when openssl is there? Is that something we should be concerned about.

I originally wanted to implement this in a way that called OpenSSL and use some form of interface from cryptography to call the proper conversion methods but even that seemed quite hairy so just went with the Python route.

[felixfontein]

cryptography does not support this. From the discussions on Freenode in #cryptography-dev, I don't think they want to support that in the future either. The focus of the library is what is needed for the WebPKI.

So for this, we have three choices:

1. implement this ourselves (as @jborean93 did);
2. use OpenSSL methods via cryptography internals (like here: https://github.com/ansible-collections/community.crypto/blob/master/plugins/module_utils/crypto/_obj2txt.py#L23-L43) - this is something I'd very much like to avoid;
3. use yet another library (some pure-Python ASN.1 parse/serialize library) for this. This additional dependency would only be needed when using this feature I guess, but I'm not really happy about adding another dependency either.

So I'm happy with doing a certain amount of ASN.1 serialization manually. (In this case, serializing an UTF-8 string.)

[jborean93]

That's pretty much what I ended up with, the other 2 options either seemed to tie us into more of the cryptography/OpenSSL internals which can cause even more gnarly issues that are hard to debug when these versions change. The option of taking another Python dep was also another consideration but considering it only took ~150 lines of code to implement the actual ASN.1 taking work I thought that added more complexity than what was needed. This could potentially become even larger in the future if we ever want to start encoding integers or sequences of elements but that might be the point where an external dep is used.

[MarkusTeufelberger]

Random quick googling shows e.g. https://github.com/andrivet/python-asn1 which we might get away with vendoring (it is only ~640 lines + MIT licensed). That way we wouldn't have to tell users to install a library only for some edge cases

[jborean93]

¯\_(ツ)_/¯ honestly it is up to you (the maintainers here). The current code is quite small, you only really have to deal with the actual tagging part and not the logic for encoding different types so I don't see too much value in vendoring a library. It might be a different story if you want to start encoding an INTEGER (especially with negative numbers) or some of the other more complex types but even then it's not too hard once you have a working implementation. You would still even need something like https://github.com/ansible-collections/community.crypto/pull/53/files#diff-1a464fbddec314f789b916776746a6d0R69-R103 to be able to derive what ASN.1 tags and value you need to use.

Ultimately I'm not the maintainer here, just found that this use case was not possible to achieve until recently with #9 and even then you needed to deal with the encoding yourself outside the module. I probably won't have time to really work on this but feel free to use whatever code you have here if someone wishes to go that route.

[MarkusTeufelberger]

I guess there's no clear way towards an "ideal" solution anyways (probably integrating this into cryptography and waiting a decade until we can require that version on all distros...) and I don't want to stand in the way of "good enough", so I'm also fine with merging this as-is.

I'm maybe just getting a bit sensitive when I read "ASN1 parser" after working with fuzzers for a while. ;-)

After all the most important part is that something works and can be used, we can take care about making the "how" prettier in a future iteration.

[felixfontein]

@MarkusTeufelberger this isn't really a ASN.1 parser, it's more a ASN.1 encoder. So I think this should be safe :)

[felixfontein]

@MarkusTeufelberger any last thoughts (or vetos) before I merge this?

[MarkusTeufelberger]

Nah, it's fine by me and thanks a lot to @jborean93 for writing this!

[felixfontein]

@jborean93 tanks a lot for implementing this!

[jborean93]

No worries, thanks for the reviews and guidance.