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

CRLs generated by PKI engine convert issuer name from UTF8String to PrintableString in the ASN.1 structure #10550

Closed
rubendv opened this issue Dec 14, 2020 · 11 comments

Comments

@rubendv
Copy link
Contributor

rubendv commented Dec 14, 2020

Describe the bug
I have an intermediate CA setup in a vault PKI secrets engine. The commonName of this CA certificate is encoded as UTF8String (as required by the ISIS-MTT standard, see https://mskb.pkisolutions.com/kb/890772), but the CRL exposed by vault on /v1/<mount path>/crl has the issuer commonName encoded as PrintableString. This causes the client certificates signed by this CA (that refer to this CRL) to be rejected by Windows because there is no binary match on the issuer commonName of the CRL and the issuer commonName of the client certificate (which does have its issuer commonName properly encoded as UTF8String).

I think I have tracked this down to the following bug in the Go ASN.1 implementation: golang/go#3791
They have a "fix" that upgrades the encoding to UTF8String but only if the string contains non-ASCII characters (https://codereview.appspot.com/6348074). For my usecase the encoding should always match the encoding of the commonName of the CA certificate, regardless of content.

Since the specific encoding to be used should be decided by vault I opened an issue for vault instead of for the ASN.1 implementation in Go.

To Reproduce

  1. Create a CA cert where commonName is encoded in ASN.1 as UTF8String (check with openssl x509 -in certificate.pem -outform der | openssl asn1parse -inform der -i)
  2. Setup a vault PKI secrets engine with this CA
  3. curl http://<vault host>:<vault port>/v1/<mount path>/crl | openssl asn1parse -inform der -i shows PrintableString for issuer name instead of UTF8String

Expected behavior
When the commonName of the CA cert is UTF8String, the issuer commonName in the CRL should also be encoded as UTF8String.
When the commonName of the CA cert is PrintableString, the issuer commonName in the CRL should also be encoded as PrintableString.

Environment:

  • Vault Server Version (retrieve with vault status): 1.1.3
  • Vault CLI Version (retrieve with vault version): 1.1.3
  • Server Operating System/Architecture: Ubuntu 16.04 amd64
@swayne275
Copy link
Contributor

Hey, thanks for the comment and the details! Have you checked if this issue is still present in more recent versions of Vault? I looked at the issue that you linked, and it appears it was fixed in Go back in 2012. We upgrade to newer versions of Go frequently so if this is a language bug it should have been fixed a long time ago.

With that, I'm going to close the issue. If this bug is still present in more recent versions of Vault please re-open and let us know! And thanks again!

@rubendv
Copy link
Contributor Author

rubendv commented Dec 18, 2020

I was wrong to say that the issue was caused by that bug in Go, but it is closely related to it. The fix for that bug was to promote the field to UTF8String as needed according to the content of the string that is being marshalled to ASN.1.
However in my case the encoding does not depend on the contents of the string but on the encoding used in the original certificate.

I will check again on the latest version, but I expect it will still have the same problem. I'll reopen this issue if that is the case.
I'll also try to create a CA cert with UTF8String commonName and key file and write out exact reproduce steps for your convenience.

@rubendv
Copy link
Contributor Author

rubendv commented Dec 21, 2020

Hi @swayne275, I confirmed that the issue is still present in vault 1.6.1.
Could you reopen the issue, since I don't have permission to do so?

I used a self signed certificate in UTF-8 encoding, generated like this:

openssl req -x509 -newkey rsa:4096 -keyout key.pem -out cert.pem -days 365 -sha256 -subj "/C=US/ST=Oregon/L=Portland/O=Company Name/OU=Org/CN=www.example.com" -utf8 -nodes

The command openssl x509 -in cert.pem -outform der | openssl asn1parse -i -inform der will show UTF8String for commonName and other fields.

I then setup a PKI secrets engine:

  1. HTTP POST to /sys/mounts/my/path/: {'type': 'pki'}
  2. HTTP POST to /my/path/config/ca: {'pem_bundle': '<contents of cert.pem>' + '\n' + '<contents of key.pem>'}

You can now curl http://<vault host>:<vault port>/my/path/crl' | openssl asn1parse -i -inform der and see that the UTF8String fields from the original cert have been replaced with PrintableString fields.

@jefferai jefferai reopened this Jan 4, 2021
@jefferai
Copy link
Member

jefferai commented Jan 4, 2021

I'll reopen for discussion but I don't think that this is an issue in Vault. Not that we can't consider a workaround if this is indeed default Windows behavior, but using the name on the cert for revocation is not a part of the CRL spec. It is supposed to be a combination of the issuer's key and the serial number of the certificate. Given that, converting from PrintableString to UTF8String or vice versa depending on whether there is a need for UTF8 is completely valid, because it's only supporting information and not security-sensitive information.

Is this a particular program on Windows? Or Windows itself (and if so, which version(s))?

@rubendv
Copy link
Contributor Author

rubendv commented Jan 4, 2021

Thanks for reopening.

The part of the spec you are referring to is about matching a certificate with an entry in the certificate revocation list, to determine if it is revoked or not.

The problem I am having however is about the issuer of the CRL itself. Section 6 of the spec covers path validation of the certification path used to verify the signature on the CRL itself. If I understand it correctly, in 6.3.3 (b)(1) it is mentioned that the issuer of the CRL should match the issuer of the certificate that is being checked for revocation.
Furthermore, Section 7.1 covers the use of different string encodings and how they are matched and it is defined that for issuers to match they need to have the same attribute type (i.e. UTF8String or PrintableString) (see comment below regarding string matching algorithm).

This is done by windows itself. More specifically, we are encountering the issue in the context of public key based Windows logon using Kerberos PKINIT. As far as I know this behaviour is present in all Windows versions since at least server 2008 and probably even before that.

This issue may be under the radar a bit because by default, Active Directory Certificate Services only creates (CA) certificates with PrintableString, and you have to change a setting (https://mskb.pkisolutions.com/kb/890772) to get a UTF8String issuer name that will trigger the issue. However, it is still a valid thing to do according to the spec (and the German PKI standard mentioned in the link). You could also encounter this issue if you use a different PKI than the default Windows one.

@rubendv
Copy link
Contributor Author

rubendv commented Jan 4, 2021

This section which defines the term “issuer” is also of interest: https://tools.ietf.org/html/rfc5280#section-4.1.2.4

@rubendv
Copy link
Contributor Author

rubendv commented Jan 5, 2021

I tried to reproduce the issue with openssl, but there it seems to work, ignoring the encoding difference.
I did some more digging in the spec and found a mention of RFC 4518 (string matching algorithm) in Section 7.1 of the CRL spec, and in there (https://tools.ietf.org/html/rfc4518#section-2.1) it is mentioned that PrintableString should be converted to UTF8String before matching them.

So it seems that you are correct that this is not a bug in vault but instead a bug in Windows.

However, the proposed solution to keep the original encoding that was in the CA certificate for the issuer in the CRL would solve this issue for Windows, and should not cause issues for other implementations such as openssl, since they would then simply be comparing UTF8String vs UTF8String without having to do the conversion first.

@rubendv
Copy link
Contributor Author

rubendv commented Jan 18, 2021

Would you accept a PR to keep the same encoding for the CRL issuer name as in the original certificate? If so, I'd be willing to work on that this week.

@wdoekes
Copy link

wdoekes commented Oct 6, 2021

Here's some funny related stuff.

Just today, I created a root CA with openssl (with the default setting of string_mask = utf8only), which got UTF8String values for the stateOrProvince and organizationName fields (but a PRINTABLESTRING for countryName):

$ openssl asn1parse -in dostno-rootca.crt -inform pem -i
...
   54:d=4  hl=2 l=   9 cons:     SEQUENCE          
   56:d=5  hl=2 l=   3 prim:      OBJECT            :countryName
   61:d=5  hl=2 l=   2 prim:      PRINTABLESTRING   :NL
   65:d=3  hl=2 l=  18 cons:    SET               
   67:d=4  hl=2 l=  16 cons:     SEQUENCE          
   69:d=5  hl=2 l=   3 prim:      OBJECT            :stateOrProvinceName
   74:d=5  hl=2 l=   9 prim:      UTF8STRING        :Groningen

and for the Vault-generated intermediate -- generated with vault write pki intermediate/generate/internal -- I got a PRINTABLESTRING for both:

$ openssl asn1parse -in dostno-intca.csr -inform pem -i
...
   15:d=4  hl=2 l=   9 cons:     SEQUENCE          
   17:d=5  hl=2 l=   3 prim:      OBJECT            :countryName
   22:d=5  hl=2 l=   2 prim:      PRINTABLESTRING   :NL
   26:d=3  hl=2 l=  18 cons:    SET               
   28:d=4  hl=2 l=  16 cons:     SEQUENCE          
   30:d=5  hl=2 l=   3 prim:      OBJECT            :stateOrProvinceName
   35:d=5  hl=2 l=   9 prim:      PRINTABLESTRING   :Groningen

This resulted in this madness:

$ openssl ca ... 
Enter pass phrase for dostno-rootca.key:
Check that the request matches the signature
Signature ok
The stateOrProvinceName field is different between
CA certificate (Groningen) and the request (Groningen)

🤯 Groningen != Groningen

So, the cause turned out to be that openssl ca appears to mark the strings as unequal because of the different type.

I can't tell if defaulting to UTF8 is better than moving to UTF8 only when strings don't fit in a regular printablestring, but I can tell you that this was confusing.

Carry on with your original ticket 🚶

@cipherboy
Copy link
Contributor

cipherboy commented May 12, 2022

JFTR, this is a limitation of Go's CRL building: it round-trips via pkix.Name which has some other limitations on round-tripping (not just this typing issue but also RDN grouping not being preserved). As of yet, there's no way to build the CRL with Go's types correctly, so it might be worth filing an upstream Go ticket for this behavior, if there isn't one already.

https://github.com/golang/go/blob/fdb640b7a1324c2a4fc579389c4bc287ea90f1db//src/crypto/x509/x509.go#L2152-L2209

@cipherboy
Copy link
Contributor

Note that as the corresponding Go issue has now been completed, this will be picked up when Go 1.20 is released. Therefore since there's nothing else to do on the Vault side (other than wait for a pending Go version bump when it is released), I'm closing this issue as completed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants