Skip to content

Add X25519Kyber768Draft00 experimental HPKE KEM#421

Merged
armfazh merged 1 commit intomainfrom
caw/hybrid-x25519-kyber-hpke-kem
Apr 12, 2023
Merged

Add X25519Kyber768Draft00 experimental HPKE KEM#421
armfazh merged 1 commit intomainfrom
caw/hybrid-x25519-kyber-hpke-kem

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
@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

chris-wood 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)

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

chris-wood commented Apr 7, 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

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

chris-wood commented Apr 7, 2023

@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

chris-wood commented Apr 7, 2023

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
@bwesterb bwesterb requested review from armfazh and cjpatton April 7, 2023 18:47
@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
bwesterb reviewed Apr 7, 2023
View reviewed changes
@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/hpke.go

// 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

@armfazh armfazh Apr 11, 2023

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

@chris-wood chris-wood Apr 12, 2023

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?

@chris-wood
Copy link
Contributor Author

chris-wood commented Apr 12, 2023

@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

chris-wood commented Apr 12, 2023

@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

bwesterb commented Apr 12, 2023

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