8325513: Export method for Cipher

[wangweij]

Add Cipher::exportKey API.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires CSR request JDK-8370812 to be approved

Issues

• JDK-8325513: Export method for Cipher (Enhancement - P4)
• JDK-8370812: Export method for Cipher (CSR)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18409/head:pull/18409
$ git checkout pull/18409

Update a local copy of the PR:
$ git checkout pull/18409
$ git pull https://git.openjdk.org/jdk.git pull/18409/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18409

View PR using the GUI difftool:
$ git pr show -t 18409

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18409.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back weijun! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@wangweij The following label will be automatically applied to this pull request:

• security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 06: Full - Incremental (2da969c0)
• 05: Full - Incremental (9f9cd651)
• 04: Full - Incremental (cd02570c)
• 03: Full - Incremental (11701dce)
• 02: Full - Incremental (bbd97a30)
• 01: Full - Incremental (b8658a76)
• 00: Full (8834f04e)

[ascarpino]

Why is the algorithm necessary for this new method? Couldn't the new SecretKey take the algorithm from the original SecretKey stored in the Cipher object?

[wangweij]

One use case for this method is HPKE key export. Obviously, the exported key won't have algorithm name being "HPKE".

[ascarpino]

One use case for this method is HPKE key export. Obviously, the exported key won't have algorithm name being "HPKE".

If Cipher::init is passed an AES SecretKey, wouldn't the Cipher::export return an AES SecretKey? I don't see the case where it is anything but the original key's algorithm.
This concern comes from the lack of a requirements on value being an algorithm name, and our provider usage is inconsistent with it to. If my memory is correct, SunJCE requires "AES", but other algorithms we don't. So having the user define this value leaves a grey area where users may enter anything. Or in the worse case, the developer not knowing what the key algorithm is (AES or CC20) and needs to keep track of it.

[wangweij]

I don't know if any AES cipher defines an export function. As I said earlier the reason I introduce this method is for HPKE. In HPKE, the cipher algorithm name is "HPKE" and the keys used in initialization are asymmetric keys. While the 3rd step in HPKE is an AEAD cipher, I cannot guarantee the export function would only generate a key for this AEAD algorithm. The fact that a length is needed for the function means the output could be anything. Also, there is an export-only mode that does not require an AEAD cipher at all.

[wangweij]

In fact, the original definition of export function returns a byte array. I choose the SecretKey return type here simply because it could be non extractable. I am now wondering if there should be also an exportData method.

[ascarpino]

If this is an HPKE operation, shouldn't it be part of an HPKE API? Why part of Cipher?

[wangweij]

I don't think it's worth inventing a new cryptographic engine for HPKE and it is a cipher.

        var g = KeyPairGenerator.getInstance("X25519");
        var kp = g.generateKeyPair();
        var sender = Cipher.getInstance("HPKE");
        var params = HPKEParameterSpec.of()
                .info("this_info".getBytes(StandardCharsets.UTF_8));
        sender.init(Cipher.ENCRYPT_MODE, kp.getPublic(), params);
        var receiver = Cipher.getInstance("HPKE");
        receiver.init(Cipher.DECRYPT_MODE, kp.getPrivate(), params.encapsulation(sender.getIV()));
        var msg = "Hello World".getBytes(StandardCharsets.UTF_8);
        var ct = sender.doFinal(msg);
        var pt = receiver.doFinal(ct);

[jnimeh]

I see that it could work that way, but have we firmly established that HPKE will be implemented in JCE as a Cipher? I see no CSR for this and I would think we'd need one. If we have not firmly established that HPKE is being done as a Cipher, then changing Cipher in advance seems like putting the cart before the horse. I just thought I'd see CSRs in place for JDK-8325513 and/or JDK-8325548 and some kind of consensus on the direction. Maybe I missed an agreement on this approach in the past.

[wangweij]

I haven't started JDK-8325548 yet since it requires KDF. Also, I was thinking to get some consensus here on the format of the APIs first and then write the CSR.

[wangweij]

As for the cart and horse order, I think you're right. Maybe I should do this in 2 steps:

1. Basic HPKE as a cipher.
2. Introduce Cipher::exportKey API and add impl in HPKE.

[jnimeh]

That seems like a good approach. If Cipher can address all the use cases for HPKE then is seems more prudent to proceed with #2. As for the KDF impact, do you see a potential Cipher-based HPKE needing to know the details of the KDF API? Or would it just need to know a standard algorithm name for a given KDF? If the latter then you may not need to wait for KDF in order to proceed given where it is in its review cycle.

[openjdk]

@wangweij This pull request is now open

[bridgekeeper]

@wangweij This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@wangweij This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[wangweij]

/open

[openjdk]

@wangweij This pull request is now open

[bridgekeeper]

@wangweij This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@wangweij This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[wangweij]

/open

[openjdk]

@wangweij This pull request is now open

[bridgekeeper]

@wangweij This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@wangweij This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[wangweij]

/open

[openjdk]

@wangweij This pull request is now open

[bridgekeeper]

@wangweij This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!