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

Include missing data about key types in keychain_items #8002

Merged

Conversation

bgirardeau-figma
Copy link
Contributor

Includes the label and associated public key hash for keys in the keychain_items table.

Before this change, the label column was blank for "public key", "symmetric key", and "private key" item types because these can use a different attribute for their label (kSecKeyPrintName).

The new column "pk_hash" is useful to identify which public key and private keys are linked to which certificates, described in the docs for kSecKeyLabel.

While the new attributes are marked deprecated, there does not seem to be a non-deprecated replacement.

Tested this change on macOS Ventura 13.3 (many entries removed to just show 1 example of each case):

select label, type, pk_hash from keychain_items;
| Developer ID Certification Authority                                   | certificate       | 5717EDA2CFDC7C98A110E0FCBE872D2CF2E31754 |
| Developer ID Certification Authority                                   | certificate       | F83A0C691176E0EDACD1EBA659FA37D5C455B01E |
| Developer ID Installer: Bradley Girareau (VRCY35483Y)                  | certificate       | C4ABFB1F1F5316E9914B24E0390364D570E85BA6 |
| Mac Developer ID Installer: Bradley Girareau                           | private key       | C4ABFB1F1F5316E9914B24E0390364D570E85BA6 |
| Spotlight Metadata Privacy (9547B18D-F428-3365-8D69-9EA526ED5F06)      | symmetric key     |                                          |
| com.apple.systemdefault                                                | public key        | DE2232008405EE16C541DEEC59C89EAD9DC56D17 |
| Spotlight Metadata Privacy                                             | password          |                                          |
| Docker Credentials                                                     | internet password |                                          |

Before this PR the same items look like:

select label, type from keychain_items;
| Developer ID Certification Authority                                    | certificate       |
| Developer ID Certification Authority                                    | certificate       |
| Developer ID Installer: Bradley Girareau (VRCY35483Y)                   | certificate       |
|                                                                         | private key       |
|                                                                         | symmetric key     |
|                                                                         | public key        |
| Spotlight Metadata Privacy                                              | password          |
| Docker Credentials                                                      | internet password |

@bgirardeau-figma bgirardeau-figma requested review from a team as code owners April 21, 2023 07:56
@directionless directionless added this to the 5.9.0 milestone May 5, 2023
@mike-myers-tob mike-myers-tob changed the title Include missing data about key types in keychain_items Include missing data about key types in keychain_items May 9, 2023
Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

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

Looks useful!

@@ -8,6 +8,7 @@ schema([
Column("created", TEXT, "Date item was created"),
Column("modified", TEXT, "Date of last modification"),
Column("type", TEXT, "Keychain item type (class)"),
Column("pk_hash", TEXT, "Hash of associated public key (SHA1 of DER PKCS#1)"),
Copy link
Member

Choose a reason for hiding this comment

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

Is this a standard hash? Can we match openssl's format for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like it is actually the SHA1 hash of the subjectPublicKey bit string defined by RFC 5280, which matches the first method outlined in section 4.2.1.2 for generating a "Subject Key Identifier". I'll update the documentation to reference that RFC instead of PKCS#1, since PKCS#1 is specifically for RSA keys.

I didn't find a particularly convenient way to output this in OpenSSL, though I might have missed it. One method I used to test is to first find the public key info section with the BIT STRING in the output of openssl asn1parse -i, which looks something like this:

  # ECDSA Example
  267:d=3  hl=2 l=  19 cons: SEQUENCE          
  269:d=4  hl=2 l=   7 prim: OBJECT            :id-ecPublicKey
  278:d=4  hl=2 l=   8 prim: OBJECT            :prime256v1
  288:d=3  hl=2 l=  66 prim: BIT STRING        

  # RSA Example
  278:d=2  hl=4 l= 290 cons:   SEQUENCE          
  282:d=3  hl=2 l=  13 cons:    SEQUENCE          
  284:d=4  hl=2 l=   9 prim:     OBJECT            :rsaEncryption
  295:d=4  hl=2 l=   0 prim:     NULL              
  297:d=3  hl=4 l= 271 prim:    BIT STRING        

The first number in the row is the byte offset into the ASN.1 DER encoded certificate data, hl is the header length of the section, and l is the length of the section content. The actual data to hash is offset by 1 byte within the BIT STRING. So the data to hash starts at (offset + header + 1) for length - 1 bytes.

For the certificate examples above, the pk_hash can then be computed like:

# ECDSA example
$ cat ecdsa.pem | sed '1d;$d' | base64 -d | dd skip=291 bs=1 count=65 | sha1sum
# RSA example
$ cat rsa.pem | sed '1d;$d' | base64 -d | dd skip=302 bs=1 count=270 | sha1sum

Copy link
Member

Choose a reason for hiding this comment

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

Works for me! Mostly I'm never sure when hash based fingerprints should be upcased or downcased of need : in the middle. Though I guess we can update if we figure out we got it wrong

@sharvilshah sharvilshah merged commit 3d3424d into osquery:master Jun 6, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants