Skip to content

Conversation

@rodesai
Copy link
Contributor

@rodesai rodesai commented Nov 15, 2024

This patch adds some infra to support license verification:

The main change is the addition of LicenseAuthenticator and LicenseChecker. LicenseAuthenticator is responsible for taking a license document, verifying that it is signed correctly, and then returning the contained signed LicenseInfo. LicenseChecker takes a LicenseInfo and validates that the license it describes can be used.

These new classes operate on the pojo types defined under the model package that define the structure of the license documents, and the key registry (SigningKeys) that's to be packaged into the jar.

Finally, this patch also includes a utility class for parsing pem files, which we'll use to store public keys. I opted to write our own parser (rather than using something like bouncycastle) because the format it's simple and I'd rather not include a dependency on a foundational artifact like bouncycastle that is likely to conflict with a user's dependencies.

This patch adds some infra to support license verification:

The main change is the addition of LicenseAuthenticator and
LicenseChecker. LicenseAuthenticator is responsible for taking
a license document, verifying that it is signed correctly, and
then returning the contained signed LicenseInfo. LicenseChecker
takes a LicenseInfo and validates that the license it describes
can be used.

These new classes operate on the pojo types defined under the model
package that define the structure of the license documents, and the
key registry (SigningKeys) that's to be packaged into the jar.

Finally, this patch also includes a utility class for parsing pem
files, which we'll use to store public keys. I opted to write our
own parser (rather than using something like bouncycastle) because
the format it's simple and I'd rather not include a dependency on
a foundational artifact like bouncycastle that is likely to
conflict with a user's dependencies.
// Given:
final var store = new ResponsiveKeyValueStore(
ResponsiveKeyValueParams.keyValue("test"),
ResponsiveKeyValueParams.keyValue("license-test"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

damn it intellij you're not as smart as you think

import java.util.Base64;
import java.util.List;

public class PublicKeyPemFileParser {
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 could have used bouncycastle to parse these, but I didn't want to bring it in as a dependency just to do that because it's likely the user will also depend on bouncycastle (possibly at a conflicting version).

Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

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

LGTM

}
}

private void verifyTimedTrialV1(final TimedTrialV1 timedTrial) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking about what attack vectors there might be. I'm not at all worried about any attack vector for timed trial (should I be?), but when we start adding more complicated ones (i.e. usage based) the bad types of attacks open up (like an organization trying to use up credits for another organization by spoofing some part of this).

For that, doing any kind of client-side validation won't be possible since you could always attach a debugger and change things after the signature is verified.

I don't think this has any bearing on this PR, but we'll probably need to attach some kind of cryptographic license identifier to any metric we send home -- so just something to keep in mind to see if that changes the design here for anything.

@rodesai rodesai merged commit 604aba4 into main Nov 15, 2024
1 check passed
@rodesai rodesai deleted the verify-license branch November 15, 2024 21:55
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