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

Unable to express all security conditions for key usage. #66

Open
dmercer-google opened this issue Sep 13, 2023 · 3 comments
Open

Unable to express all security conditions for key usage. #66

dmercer-google opened this issue Sep 13, 2023 · 3 comments

Comments

@dmercer-google
Copy link
Collaborator

The Access Mode Enumeration described here does not allow for a complete expression of all access modes described in 800-73-4.

You define:

AccessMode ::= ENUMERATED {
	never  (0),
	pin  (1),  
	pinAlways  (2),
        occ  (4),  
	userAdmin  (16),
	always  (127)
}

If you look at 800-73-4 Part 1 Table b4 you will see that there is a need to describe VCI and PIN, VCI and OCC, VCI and PIN Always and VCI and OCC Always

Looking at your PutDataCreateObjectRequest and PutDataCreateKeyRequest in the PUT DATA ADMIN schema I see that you have only defined modeContact and modeContactless. I think that the right way to fix this is to add modeVci to each of these and add a tag to the pre-perso APDUs to indicate conditions specific only to the VCI interface.

BTW: We will probably be implementing VCI and will push it up to you if and when it gets done. Likewise with OCC.

@makinako
Copy link
Owner

This is by design, since 'VCI' is essentially about the conditions where the contactless interface may be treated the same as the contact interface. So instead of defining a separate 'vci' condition we just apply this logic:

  • If [contact interface], use the modeContact field to determine permissions
  • If [contactless interface], use the modeContactless field to determine permissions
  • If [contactless interface] AND [vci conditions met], use the modeContact field to determine permissions

Now I would have kissed you if you had told me about implementing VCI a few months ago, but aside from some response wrapping cases it's all done!

I made a possibly questionable decision at the start of the FIPS140 process to bring the changes to a private repo with the intention of merging it back in once reasonably secure about the necessary changes. This came out of a general uncertainty about just how big the changes would need to be to comply and the presumption of some chaos at the start.
To put it short, I intend to merge the changes in to this repo soon and then I will have to manually deal with any commits that have happened since (not much currently, just a fix by @mistial-dev).

So my suggestion is to prioritise occ first and I will prioritise getting the code back into a FIPS branch like it should be so we can then go forward cohesively. Regarding VCI implementation specifically, some early design decisions around the ChainBuffer and the separation of responsbilities between OpenFIPS01.java and PIV.java didn't work very well for PIV-SM and SCP03 together, so I have changed the code quite a bit.
There is now a PIVBuffer which contains the internal memory buffer (what used to be called scratch) and this is passed from OpenFIPS201 to PIV. It is responsible for unwrapping/wrapping and command/response chaining as well as providing access to the buffer. I'll document this a bit more in the Design page.

@dmercer-google
Copy link
Collaborator Author

I dontb think that this works for something like key 9B which is always for contact and never for contactless, VCI or no.

@makinako
Copy link
Owner

This is true, though 800-73 has a general rule in 5.5 that prohibits VCI for administrative operations altogether. This logic is reflected in the Options.restrictContactlessAdmin parameter, though the default is false and in retrospect should probably have been named permitContactlessAdmin with a default of false to require explicit enabling.

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