-
Notifications
You must be signed in to change notification settings - Fork 151
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.
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
Bring over the WWDC 2023 CryptoKit API #181
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments inline. A lot of them are around formatting and captilization of method names
/// An Advanced Encryption Standard cipher in Galois/Counter Mode with a key length of 128 bits. | ||
case AES_GCM_128 | ||
/// An Advanced Encryption Standard cipher in Galois/Counter Mode with a key length of 256 bits. | ||
case AES_GCM_256 | ||
/// A ChaCha20 stream cipher with the Poly1305 message authentication code. | ||
case chaChaPoly | ||
/// An export-only mode. | ||
/// | ||
/// In export-only mode, HPKE negotiates key derivation, but you can't use it to encrypt or decrypt data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc comments are wrongly indented here
case .chaChaPoly: | ||
return try ChaChaPoly.seal(message, using: key, nonce: ChaChaPoly.Nonce(data: nonce), authenticating: aad).combined.suffix(from: nonce.count) | ||
default: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use a default here or exhaustively switch to cover new cases added in the future?
|
||
|
||
extension HPKE { | ||
/// Cipher suites to use in hybrid public key encryption. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Cipher suites to use in hybrid public key encryption. | |
/// Cipher suites to use in hybrid public key encryption. |
/// mechanism (KEM) for sharing the symmetric key. The sender and recipient of encrypted messages need to use the | ||
/// same cipher suite. | ||
public struct Ciphersuite { | ||
/// A cipher suite for HPKE that uses NIST P-256 elliptic curve key agreement, SHA-2 key derivation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// A cipher suite for HPKE that uses NIST P-256 elliptic curve key agreement, SHA-2 key derivation | |
/// A cipher suite for HPKE that uses NIST P-256 elliptic curve key agreement, SHA-2 key derivation |
|
||
fileprivate static let ciphersuiteLabel = Data("HPKE".utf8) | ||
|
||
/// The key encapsulation mechanism (KEM) for encapsulating the symmetric key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// The key encapsulation mechanism (KEM) for encapsulating the symmetric key. | |
/// The key encapsulation mechanism (KEM) for encapsulating the symmetric key. |
#else | ||
import Foundation | ||
|
||
internal func I2OSP(value: Int, outputByteCount: Int) -> Data { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
let key: DHPK | ||
|
||
init(_ publicKey: DHPK, kem: HPKE.KEM) throws { | ||
// TODO: Validate Ciphersuite Mismatches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this TODO?
return Crypto.KEM.EncapsulationResult(sharedSecret: HPKE.KexUtils.ExtractAndExpand(dh: dh, | ||
enc: enc, | ||
pkRm: selfRepresentation, | ||
kem: kem, | ||
kdf: kem.kdf), encapsulated: enc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formatting is off here
|
||
/// A type that ``HPKE`` uses to encode the public key. | ||
public protocol HPKEPublicKeySerialization { | ||
/// Creates a public key from an encoded representation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments here are also wrongly indented
let key: DHSK | ||
|
||
init(_ privateKey: DHSK, kem: HPKE.KEM) throws { | ||
// TODO: Validate Ciphersuite Mismatches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this one
@FranzBusch Will it be acceptable for you if I apply those proposals in a new PR? |
Totally fine by me! |
Motivation WWDC has arrived! 🎉 As part of the celebration, let's bring Crypto up to speed with the new CryptoKit API surface. Modifications Substantial new docstrings HPKE support Result WWDC 2023 support.
Motivation
WWDC has arrived! 🎉 As part of the celebration, let's bring Crypto up to speed with the new CryptoKit API surface.
Modifications
Substantial new docstrings
HPKE support
Result
WWDC 2023 support.