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

OID for Attestation extension incorrect #90

Closed
jcspencer opened this issue Dec 23, 2024 · 4 comments
Closed

OID for Attestation extension incorrect #90

jcspencer opened this issue Dec 23, 2024 · 4 comments

Comments

@jcspencer
Copy link

Hi there @virot!

First off, absolutely love this library/module! Makes things so much nicer.

I'd been looking for a way to do TPM-style attestation with Yubikey-signed CSRs for ages, and the combination of this library with TameMyCerts is perfect!

One thing I noticed is that the OID used to staple the attestation to the CSR, 1.3.6.1.4.1.41482.3.11 is incorrect; it should likely be 1.3.6.1.4.1.41482.3.11, as referenced in the Yubikey OID reference

The .1 OID is for "Attestation data and signature", and the .2 OID is for "Attestation certificate"; the .11 is for identifying that the card is "CSPN" certified, which doesn't seem to match up.

Likely just a typo, and happy to make a PR to tweak it if needed, just thought I'd check first!

Thanks again for the great module - it's a great tool to have around!

@virot
Copy link
Owner

virot commented Dec 23, 2024

Hi.

That is also why I have done code together with Uwe to allow TameMyCerts to have included YubiKey attestion :)
That make it easier to check.

This is where it becomes a bit murky.. First you wrote the same OID twice. I know that the documentation says .1. But the Yubico reference implementation yubico-piv-tool, says:
add_ext(exts, YKPIV_ATTESTATION_OID ".11", "ykpiv attestation", "Yubico PIV X.509 Attestation", buf, buflen);

The question do we want the PowerShell module to behave as the reference implementation or do we want to follow the standard :/

virot added a commit that referenced this issue Dec 23, 2024
Making sure Confirm-YubikeyAttestion, allows for both YubiKey documentation .1 and yubico-piv-tool implementation .11 location of Attestation Data.
Added data out put to say where Attestation Data was located.
Update Build-YubikeyPIVCertificateSigningRequest to give better error if KeyStatus is wrong.

In regard to #90, while I try to figure out where to store the Attestation Data should be in a new CSR.
@virot
Copy link
Owner

virot commented Dec 23, 2024

I did a fix that allows for both in the verification Cmdlet. I will try get more information about yubico-piv-tool implementation.

@jcspencer
Copy link
Author

Huh, that’s odd - it seems they’re using their own OIDs differently to what’s in the spec!

Makes sense to follow what’s happening in yubikey-piv-tool, but agree that it’s probably worth seeing what they think upstream - looks like a typo in their code may have just snuck through…

Thanks for the quick reply! :^)

@jcspencer
Copy link
Author

@virot I've raised this upstream in Yubico/yubico-piv-tool#526 🙂

Thanks for the adding the flags in the cmdlets!

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

No branches or pull requests

2 participants