Skip to content

Conversation

@prbprbprb
Copy link
Contributor

@prbprbprb prbprbprb commented Oct 10, 2023

Decouples API from implementation.

Client API finds implementation instances using JCA Provider model and uses them via astable SPI using only primitives and classes available since Android API level 19, possibly via duck typing reflection if they are in different packages.

Verified that this works end-to-end on Android where the API from a standalone Conscrypt library (org.conscrypt) could find and use the implementation in the platform (com.android.org.conscrypt).

Also re-aligns client API and thrown exceptions to more closely match other JCA services, e.g. Cipher

@prbprbprb prbprbprb marked this pull request as draft October 10, 2023 16:23
@prbprbprb prbprbprb marked this pull request as ready for review October 20, 2023 14:19
@prbprbprb prbprbprb requested review from davidben and jorgesaldivar and removed request for jorgesaldivar October 20, 2023 14:19
@prbprbprb prbprbprb requested a review from Yqwed October 24, 2023 15:12
throw new NoSuchMethodException(sourceMethod + " return value (" + sourceReturnType
+ ") incompatible with target return value (" + targetReturnType + ")");
}
methods.put(sourceMethod.getName(), sourceMethod);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I don't know how likely it is, but if a method from HpkeSpi is overloaded, this will be overridden. Should we check that put returns null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's covered, as we do

 Method sourceMethod =
          sourceClass.getMethod(targetMethod.getName(), targetMethod.getParameterTypes());

and (as I found during refactorings :) that won't match unless the parameters match exactly.

I am a bit concerned about differing checked exceptions though....

Also, I'm going to move that getClass() outside the loop though seeing as there's another fix needs to go in

private final HpkeSuite hpkeSuite;
private EVP_HPKE_CTX ctx;
private byte[] enc;
public class HpkeContextSender extends HpkeContext{
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: space

Pete Bentley added 15 commits October 24, 2023 17:19
Java 9+ makes poor decisions about which constructor
to look for when passing in an arg. Workaround adds
un-needed complexity to the Provider, so go back
to no-arg for now and if we actually need to track
the Provider we'll add a setter to the SPI.
Also rename `enc` params to `encapsulated` for clarity.
Adds API code to initialise all modes, although only base
mode is still implemented.
Also somehow I forgot to update the test expectations
for decrypt errors in the last commit. *sigh*
@prbprbprb prbprbprb merged commit bc1a34b into google:master Oct 24, 2023
@prbprbprb prbprbprb deleted the hpke branch December 17, 2023 19:48
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.

3 participants