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

x509:get_extension("subjectKeyIdentifier"):text() is in weird format #24

Closed
Evengard opened this issue Apr 23, 2021 · 6 comments · Fixed by #28
Closed

x509:get_extension("subjectKeyIdentifier"):text() is in weird format #24

Evengard opened this issue Apr 23, 2021 · 6 comments · Fixed by #28

Comments

@Evengard
Copy link

Evengard commented Apr 23, 2021

For some reason, when doing x509:get_extension("subjectKeyIdentifier"):text(), I am getting a "keyid:" prefix, the actual keyid and a line return at the end.
When doing x509:get_extension("authorityKeyIdentifier"):text() I am getting only the keyid without any prefixes and line returns at the end.
I am using it to detect if a certificate is self-signed, so I filter theese weirdnesses out with gsub, but it is still strange that the keyid outputting is different for the similar use case.

@fffonion
Copy link
Owner

fffonion commented Apr 23, 2021

@Evengard Could you share a sample cert you are seeing this error? If not possible could you share output of openssl x509 -in youcert.crt -noout -text regarding subjectkeyidentifier and authoritykeyidentifier.

extension:text() uses the X509V3_EXT_print function, which is mainly focused on human readable output so the output could be not unified. You can probably use to_der() instead, which gives you the raw ASN.1 data and is better for machine to read.

@Evengard
Copy link
Author

Evengard commented Apr 23, 2021

I might had mistaken and subjectKeyIdentifier gives a line return, and authorityKeyIdentifier gives the "keyid:" prefix. Anyway, I actually noticed kind of similar behaviour (regarding the prefix) when checking the cert in the Windows cert viewer dialog. So that may be an ill-formed certificate...
Anyway, the cert itself is not a secret, attaching it. And also I found a better way to check for self-signedness (aka x509:verify(x509:get_pubkey()))
buggy.zip

@Evengard
Copy link
Author

Also I couldn't find documentation for the to_der method...

@fffonion
Copy link
Owner

fffonion commented Apr 24, 2021

The output format for authorityKeyIdentifier is decided by openssl and it's not actually an cert issue. By rfc5280, AuthorityKeyIdentifier is a sequence and it's element at position 0 decribes the KeyIdentifier (which is an octect string); while SubjectKeyIdentifier is just a octect string. That makes the text() function behave differently.

x509:verify(x509:get_pubkey()) might not be reliable depending on if you trust the ceritificate or not, as you can't distinguish "if the cert's signature is valid" from "if the cert is not self-signed".

So there're couple of ways to archieve your goal:

  1. Parse the text representation from :text() as you decribed before
  2. The ideal or most reliable way is to use to_der() and then parse the ASN.1 data by yourself. ASN.1 parser is not implemented yet in this project, but it's on the roadmap. For now you could use some existing tooling like https://github.com/nmap/nmap/blob/master/nselib/asn1.lua. Or if you could also parse it on your own:
local x509 = require("resty.openssl.x509")
local c1 = assert(x509.new(io.open("buggy.crt"):read("*a")))

local function to_hex(str)
    return (str:gsub('.', function (c)
        return string.format('%02X', string.byte(c))
    end))
end

print(to_hex(c1:get_extension("subjectKeyIdentifier"):to_der()))
print(to_hex(c1:get_extension("authorityKeyIdentifier"):to_der()))

gives you

04141F35BAA027617E1DFB54BEF2E24849C5C2A9
(which represents)
^ 04: it's a octect string
  ^ 14: string length = 20
    ^ 1F35.... : keyId
 
301680141F35BAA027617E1DFB54BEF2E24849C5C2A9
(which represents)
^ 30: it's a sequence
  ^ 16: sequence length = 16
    ^ 80: context specific, number [0]
      ^ 14: length = 20
        ^ 1F35.... : keyId

This can be made into a helper function x509:is_self_signed() once asn.1 parser is completed. I will also add doc
for to_der shortly.

  1. Last approach is to use x509.store , only a self-signed cert will verify on itself (with a depth of 0).
local store = require("resty.openssl.x509.store")
local x509 = require("resty.openssl.x509")
local c1 = assert(x509.new(io.open("buggy.crt"):read("*a")))

local s1 = store.new()
assert(s1:add(c1))

print(s1:verify(c1))

gives you true, while a non self-signed cert will fails in verify with unable to get local issuer certificate.

@Evengard
Copy link
Author

Oh ok. I actually searched a method for binary comparison, but the to_der method wasn't documented, so I haven't found it =).
Anyway, that is a bit strange that x509:verify(x509:get_pubkey()) could fail, I am actually checking that way untrusted certs (the one I sent is an untrusted one, for example), it seems to be just checking if it was signed by the same key which is the pubkey of the cert, which is the very definition of self-signed. Works fine for now though. What could be the other cause being "signature invalid" in this way? Means it's signed by some other key, hence not self-signed...?
If I'll stumble upon the problem though I'll consider the store method.

@fffonion
Copy link
Owner

@Evengard Per my understanding, usually x509:verify is to make sure if the cert is being tempered,
or simply the signature is not legit. So you will need to exclude those possibilities if you say a false is
returned by x509:verify.

fffonion added a commit that referenced this issue Jul 20, 2021
where X509V3_EXT_print is not available

Fix #24 #27
fffonion added a commit that referenced this issue Jul 20, 2021
where X509V3_EXT_print is not available

Fix #24 #27
fffonion added a commit that referenced this issue Jul 21, 2021
where X509V3_EXT_print is not available

Fix #24 #27
fffonion added a commit that referenced this issue Aug 2, 2021
where X509V3_EXT_print is not available

Fix #24 #27
fffonion added a commit that referenced this issue Aug 2, 2021
where X509V3_EXT_print is not available

Fix #24 #27
fffonion pushed a commit that referenced this issue May 28, 2024
fix #24
Co-authored-by: Callum Loh <cloh@squiz.net>
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 a pull request may close this issue.

2 participants