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

lmp-el2go-auto-register: enable generic usage for subscriber secure objects #1307

Merged

Conversation

arnout
Copy link

@arnout arnout commented Oct 6, 2023

This PR makes various improvements to lmp-el2go-auto-register. The main goal is to make it more useful for a subscriber who wants to use EdgeLock 2GO to manage their own secure objects.

Currently only two secure objects are handled: Foundries and AWS IoT. The mechanism used for AWS IoT can however easily be applied more generically to any keypair and certificate that a subscriber wants to provision through EdgeLock 2GO.

Subscriber keypairs may use an algorithm other than P-256, so support for that is added as well. However, this part is largely untested - in the end we used P-256 after all, so I didn't get around to test all the others, and our SE050 variant doesn't support P-384 and P-521.

We happen to use OID 0x5010 in our tests, and it turns out that the processing of labels didn't work correctly with smaller OIDs. The final patch fixes that bug.

The other patches are cleanups and enablers.

The documentation is updated accordingly: foundriesio/docs#611

Arnout Vandecappelle added 5 commits October 6, 2023 19:31
The HANDLERS environment variable allows to control which keypair
handlers are actually invoked. However, adding keypair handlers requires
adding custom code to the script, and in this case there's no need for
an enviroment variable to control it. On the other hand, the aktualizr
handler is pretty much obligatory, and the AWS handler will not be
invoked if the AWS keys are not installed. Thus, the HANDLERS override
mechanism serves no useful purpose.

Remove HANDLERS and always iterate over all handlers. Since the keys of
the handlers are no longer used, convert it to a list instead of a dict.

Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@mind.be>
Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@fortrobotics.com>
lmp-el2go-auto-register has specific support for import of "AWS IoT
keys". However, exactly the same approach applies to any other key that
gets provisioned over EdgeLock2Go as well, not just for AWS.

Therefore, generalize the import of keys to any key ID. The key IDs are
given as a comma-separated list of hexadecimal IDs in the
GENERIC_KEYPAIRS environment variable. Thus, it can be set in
/etc/default/lmp-el2go-auto-register.

For simplicitly, we enforce the convention that the certificate
corresponding to the key is put in the slot just above the key itself.
Thus, only the key ID needs to be given, the certificate ID is
calculated based on it.

For flexibility, the key ID is converted to integer and back to string,
so that it is formatted correctly as expected by fio-se05x-cli.

Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@mind.be>
Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@fortrobotics.com>
Like was done earlier for import_crt, use fio-se05x-cli for import_key
as well.

This will allow us to automatically derive the key type in a later
commit.

Both import_crt and import_key can be called from FioCli instead of
Pkcs11, so they are no longer needed in Pkcs11.

Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@mind.be>
Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@fortrobotics.com>
When importing a key, the key type must be provided. Currently it is
fixed as EC:prime256v1, but it might actually be a different one.
Fortunately, the SE050 tells us the key type that is present in the
slot, so we can automatically derive it. We can use fio-se05x-cli to get
this information before calling it again to import the key.

For EC keys, this requires a lookup table because OpenSSL has a weird
naming convention for the NIST-P keys. Note that this mapping table is
currently incomplete: the brainpool and ED names are missing, because we
couldn't find how they are called in the output from fio-se05x-cli.

Ideally, this conversion would be done in fio-se05x-cli itself and the
--key-type parameter would be made redundant. That was however a bit too
big of a change to be performed by us.

Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@mind.be>
Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@fortrobotics.com>
The SE05x keys and certificates are manipulated through their Object IDs
(OIDs), which are 32-bit hexadecimal numbers. Once imported into
PKCS#11, a label is used which is derived from the OID.

There is however a problem for OIDs with leading zeroes, i.e. smaller
than 0x10000000. In some places, an 8-character number with leading
zeroes is expected (in particular in the PKCS#11 labels). In other
places, leading zeroes are not allowed (e.g. in the argument to
fio-se05x-cli --list-objects).

To make it easier to use leading zeroes exactly as needed, keep track of
the OIDs as int instead of as str. Convert them to the expected string
formatting in every place, as appropriate. Since there are several
places where the SE_ label is used, add a function SE_label that
converts the OID int to the label string.

Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@mind.be>
Signed-off-by: Arnout Vandecappelle <arnout.vandecappelle@fortrobotics.com>
Copy link
Member

@doanac doanac left a comment

Choose a reason for hiding this comment

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

this looks nice. We should probably have @mwasilew give it a quick test and then I think its good to merge.

@mwasilew
Copy link
Contributor

mwasilew commented Oct 6, 2023

I'll try to test it next week.

@arnout
Copy link
Author

arnout commented Oct 26, 2023

The documentation for this feature (foundriesio/docs#611) was already merged...

Copy link
Member

@ricardosalveti ricardosalveti left a comment

Choose a reason for hiding this comment

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

LGTM, great set of changes, thanks for the contribution!

@ricardosalveti
Copy link
Member

@ldts one interesting one for you: 382219a

@ricardosalveti ricardosalveti merged commit b5a78cf into foundriesio:main Oct 27, 2023
@ldts
Copy link
Contributor

ldts commented Oct 27, 2023

@ldts one interesting one for you: 382219a

yeah, I can do the conversion

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 this pull request may close these issues.

None yet

5 participants