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.

Sign up for GitHub

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

Jump to bottom

Add X25519Kyber768Draft00 experimental HPKE KEM #421

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Add X25519Kyber768Draft00 experimental HPKE KEM #421

merged 1 commit into from
Apr 12, 2023

Conversation

chris-wood
Copy link
Contributor

@chris-wood chris-wood commented Apr 6, 2023 •
edited
Loading

See the draft. Some tasks still left to do:

  • Add multiple encryption vectors to match RFC9180
  • Add multiple export vectors to match RFC9180

@bwesterb bwesterb self-requested a review April 6, 2023 14:46
bwesterb
bwesterb requested changes Apr 6, 2023
View reviewed changes
hpke/hybridkem.go Show resolved Hide resolved
hpke/hybridkem.go Outdated Show resolved Hide resolved
hpke/hybridkem.go Show resolved Hide resolved
hpke/hybridkem.go Outdated Show resolved Hide resolved
hpke/hybridkem.go Outdated Show resolved Hide resolved
hpke/hybridkem.go Outdated Show resolved Hide resolved
hpke/hybridkem.go Show resolved Hide resolved
@bwesterb
Copy link
Member

bwesterb commented Apr 6, 2023

Thanks @chris-wood !

@bwesterb
Copy link
Member

bwesterb commented Apr 6, 2023

For your convenience:

  Running [/home/runner/golangci-lint-1.51.2-linux-amd64/golangci-lint run --out-format=github-actions --config=./.etc/golangci.yml ./...] in [] ...
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `gofumpt`-ed (gofumpt)
  Error: File is not `gofumpt`-ed (gofumpt)
  Error: `(kemBase).labeledExtract` - `salt` always receives `nil` (unparam)

@chris-wood
Copy link
Contributor Author

For your convenience:

  Running [/home/runner/golangci-lint-1.51.2-linux-amd64/golangci-lint run --out-format=github-actions --config=./.etc/golangci.yml ./...] in [] ...
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `gofumpt`-ed (gofumpt)
  Error: File is not `gofumpt`-ed (gofumpt)
  Error: `(kemBase).labeledExtract` - `salt` always receives `nil` (unparam)

What incantation do I need to invoke to make these changes locally? Can we get that set up in a make target?

@bwesterb
Copy link
Member

bwesterb commented Apr 6, 2023

What incantation do I need to invoke to make these changes locally? Can we get that set up in a make target?

make lint

@chris-wood
Copy link
Contributor Author

What incantation do I need to invoke to make these changes locally? Can we get that set up in a make target?

make lint

This is sort of unhelpful in that it just runs the linter without actually applying changes. I'll send a PR to update the Makefile so that there's a target for actually doing the formatting.

@chris-wood
Copy link
Contributor Author

@bwesterb I pushed some preliminary test vector generation code.

@chris-wood chris-wood marked this pull request as ready for review April 7, 2023 18:33
@chris-wood
Copy link
Contributor Author

Marking as ready for review now. I updated when keys are validated (only when we receive them over the wire through deserialization functions). Fail early and fail often!

@bwesterb bwesterb self-requested a review April 7, 2023 18:46
@gitguardian
Copy link

gitguardian bot commented Apr 7, 2023 •
edited
Loading

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@bwesterb
Copy link
Member

bwesterb commented Apr 8, 2023

@armfazh @cjpatton Please check whether this makes sense to you.

@bwesterb bwesterb added the new feature New functionality or module label Apr 10, 2023
armfazh
armfazh reviewed Apr 11, 2023
View reviewed changes
hpke/hybridkem.go Outdated Show resolved Hide resolved
hpke/xkem.go Outdated Show resolved Hide resolved
hpke/algs.go Outdated Show resolved Hide resolved
hpke/hpke.go
@@ -105,10 +105,6 @@ type Sender struct {

// NewSender creates a Sender with knowledge of the receiver's public-key.
func (suite Suite) NewSender(pkR kem.PublicKey, info []byte) (*Sender, error) {
if !suite.kemID.validatePublicKey(pkR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As part of this checking, the validation also checks whether the KEM in the suite is compatible with the KEM key.

The same applies to other checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's address that in a separate PR?

hpke/vectors_test.go Show resolved Hide resolved
hpke/aead.go Show resolved Hide resolved
@chris-wood
Copy link
Contributor Author

@armfazh can you please re-review? I'd like to merge this and address KEM-specific validation steps in a separate PR, if possible.

armfazh
armfazh approved these changes Apr 12, 2023
View reviewed changes
Copy link
Contributor

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

it looks good, could you please squash the commits.

@chris-wood chris-wood force-pushed the caw/hybrid-x25519-kyber-hpke-kem branch from 2424c4f to 13bd304 Compare April 12, 2023 18:12
@chris-wood
Copy link
Contributor Author

@armfazh squashed and ready to go!

@chris-wood
Add X25519Kyber768Draft00 experimental HPKE KEM
161705b 
This change also adds the ability to produce test vectors for the
draft specification:

  https://datatracker.ietf.org/doc/draft-westerbaan-cfrg-hpke-xyber768d00/

This change also updates the known answer test vectors from RFC9180.
@chris-wood chris-wood force-pushed the caw/hybrid-x25519-kyber-hpke-kem branch from 13bd304 to 161705b Compare April 12, 2023 18:13
@armfazh armfazh merged commit eaec71f into main Apr 12, 2023
@armfazh armfazh deleted the caw/hybrid-x25519-kyber-hpke-kem branch April 12, 2023 18:43
@bwesterb
Copy link
Member

Cool 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwesterb bwesterb bwesterb approved these changes

@armfazh armfazh armfazh approved these changes

@cjpatton cjpatton Awaiting requested review from cjpatton

Assignees
No one assigned
Labels
new feature New functionality or module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chris-wood @bwesterb @armfazh