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

Signatures / MACs: AAD generation missing? #12

Open
chrysn opened this issue Apr 15, 2021 · 4 comments
Open

Signatures / MACs: AAD generation missing? #12

chrysn opened this issue Apr 15, 2021 · 4 comments

Comments

@chrysn
Copy link
Collaborator

chrysn commented Apr 15, 2021

Looking to fix the big TODO around _verify_signature (which currently returns successfully for all cases), I'm lacking the AAD context to verify these hashes.

What's the plan around that function? Is the expectation that it has enough context to build the relevant plaintext/AAD itself, or should it grow an argument and have the callers provide that?

@chrysn
Copy link
Collaborator Author

chrysn commented Apr 15, 2021

Having worked a bit on an actual implementation (with which I can verify the MAC in message3), I found that while of peers there's always full information (ID_CRED_mine, CRED_mine, my private key), of the peer there's only ID_CRED_peer and the public key of the peer, but not the consensus encoding of the CRED_peer (which typically takes the form {1: 1, -1: 4, -2: key, "subject name": something}). Are there any plans where that should be transported yet?

CC @TimothyClaeys as I'm not sure you're subscribed here.

@chrysn
Copy link
Collaborator Author

chrysn commented Apr 15, 2021

Hm, seems ther is a distincguished place for those (_peer_cred, _remote_authkey), just that there's some paths through _parse_credentials that make them identical.

On the plus side, this is getting into a PR'able shape slowly, maybe we can talk more precisely when reviewing that.

@chrysn
Copy link
Collaborator Author

chrysn commented Apr 16, 2021

I do now have working signature_or_mac_3 verification (both versions); PR probably makes most sense after #9 is processed and my stuff is rebased onto there.

@TimothyClaeys
Copy link
Member

Okay, I think you can PR your changes. in the meantime I'll add some additional test vectors from test-vectors-05.

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