Use hash size as KSF output size, expose KSF parameters & salt in configuration #73

DJAndries · 2024-11-05T00:09:04Z

A couple of changes to achieve interoperability with facebook/opaque-ke:

Use the configured hash size as the KSF output size (it seems that the Rust library does this, not sure if it's mentioned in the draft)
Allow configuration of KSF cost parameters & salt value

…figuration

bytemare · 2024-12-29T02:57:10Z

examples_test.go

+		OPRF: opaque.RistrettoSha512,
+		KDF:  crypto.SHA512,
+		MAC:  crypto.SHA512,
+		Hash: crypto.SHA512,
+		KSF: opaque.KSFConfiguration{
+			Identifier: ksf.Argon2id,
+		},


Suggested change

OPRF: opaque.RistrettoSha512,

KDF: crypto.SHA512,

MAC: crypto.SHA512,

Hash: crypto.SHA512,

KSF: opaque.KSFConfiguration{

Identifier: ksf.Argon2id,

},

OPRF: opaque.RistrettoSha512,

KDF: crypto.SHA512,

MAC: crypto.SHA512,

Hash: crypto.SHA512,

KSF: opaque.KSFConfiguration{

Identifier: ksf.Argon2id,

},

bytemare · 2024-12-29T02:59:22Z

opaque.go

+type KSFConfiguration struct {
+	Identifier ksf.Identifier `json:"identifier"`
+	Parameters []int          `json:"parameters"`
+	Salt       []byte         `json:"salt"`
+}
+


Suggested change

type KSFConfiguration struct {

Identifier ksf.Identifier `json:"identifier"`

Parameters []int `json:"parameters"`

Salt []byte `json:"salt"`

}

type KSFConfiguration struct {

Parameters []int `json:"parameters"`

Salt []byte `json:"salt"`

Identifier ksf.Identifier `json:"identifier"`

}

aligns better in memory

bytemare · 2024-12-29T02:59:43Z

opaque.go

+	Parameters []int          `json:"parameters"`
+	Salt       []byte         `json:"salt"`
+}
+
 // Configuration represents an OPAQUE configuration. Note that OprfGroup and AKEGroup are recommended to be the same,
 // as well as KDF, MAC, Hash should be the same.
 type Configuration struct {
 	Context []byte


Suggested change

Context []byte

bytemare · 2024-12-29T02:59:59Z

opaque.go

+	KDF     crypto.Hash      `json:"kdf"`
+	MAC     crypto.Hash      `json:"mac"`
+	Hash    crypto.Hash      `json:"hash"`
+	OPRF    Group            `json:"oprf"`
+	KSF     KSFConfiguration `json:"ksf"`
+	AKE     Group            `json:"group"`


Suggested change

KDF crypto.Hash `json:"kdf"`

MAC crypto.Hash `json:"mac"`

Hash crypto.Hash `json:"hash"`

OPRF Group `json:"oprf"`

KSF KSFConfiguration `json:"ksf"`

AKE Group `json:"group"`

KSF KSFConfiguration `json:"ksf"`

Context []byte

KDF crypto.Hash `json:"kdf"`

MAC crypto.Hash `json:"mac"`

Hash crypto.Hash `json:"hash"`

OPRF Group `json:"oprf"`

KSF KSFConfiguration `json:"ksf"`

AKE Group `json:"group"`

memory alignement

bytemare · 2024-12-29T03:01:19Z

opaque.go

+	var ksfEncodedParams []byte
+	for _, param := range c.KSF.Parameters {
+		ksfEncodedParams = append(ksfEncodedParams, encoding.I2OSP(param, 4)...)
+	}


Suggested change

}

}

bytemare · 2024-12-29T03:01:55Z

opaque.go

@@ -228,21 +248,50 @@ func (c *Configuration) GetFakeRecord(credentialIdentifier []byte) (*ClientRecor

 // DeserializeConfiguration decodes the input and returns a Parameter structure.
 func DeserializeConfiguration(encoded []byte) (*Configuration, error) {
-	if len(encoded) < confLength+2 { // corresponds to the configuration length + 2-byte encoding of empty context
+	if len(encoded) < confIdsLength+2 { // corresponds to the configuration length + 2-byte encoding of empty context


Suggested change

if len(encoded) < confIdsLength+2 { // corresponds to the configuration length + 2-byte encoding of empty context

if len(encoded) < confIdsLength+6 { // corresponds to the configuration length + 3*2-byte encoding of empty context and KSF parameters

bytemare · 2024-12-29T03:02:27Z

opaque.go

 		return nil, internal.ErrConfigurationInvalidLength
 	}

-	ctx, _, err := encoding.DecodeVector(encoded[confLength:])
+	ctx, _, err := encoding.DecodeVector(encoded[confIdsLength:])


Suggested change

ctx, _, err := encoding.DecodeVector(encoded[confIdsLength:])

ctx, offset, err := encoding.DecodeVector(encoded[confIdsLength:])

use the returned offset

bytemare · 2024-12-29T03:04:34Z

opaque.go

+	var ksfParams []int
+	ksfParamOffset := confIdsLength + 2 + len(ctx)
+
+	if len(encoded) >= ksfParamOffset+2 {
+		ksfEncodedParams, _, err := encoding.DecodeVector(encoded[ksfParamOffset:])
+		if err != nil {
+			return nil, fmt.Errorf("decoding the ksf configuration parameters: %w", err)
+		}
+		for i := 0; i < len(ksfEncodedParams); i += 4 {
+			ksfParams = append(ksfParams, encoding.OS2IP(ksfEncodedParams[i:i+4]))
+		}
+	}
+
+	var ksfSalt []byte
+	ksfSaltOffset := ksfParamOffset + 2 + (len(ksfParams) * 4)
+	if len(encoded) >= ksfSaltOffset+2 {
+		ksfSalt, _, err = encoding.DecodeVector(encoded[ksfSaltOffset:])
+		if err != nil {
+			return nil, fmt.Errorf("decoding the ksf salt: %w", err)
+		}
+		if len(ksfSalt) == 0 {
+			ksfSalt = nil
+		}
+	}


Suggested change

var ksfParams []int

ksfParamOffset := confIdsLength + 2 + len(ctx)

if len(encoded) >= ksfParamOffset+2 {

ksfEncodedParams, _, err := encoding.DecodeVector(encoded[ksfParamOffset:])

if err != nil {

return nil, fmt.Errorf("decoding the ksf configuration parameters: %w", err)

}

for i := 0; i < len(ksfEncodedParams); i += 4 {

ksfParams = append(ksfParams, encoding.OS2IP(ksfEncodedParams[i:i+4]))

}

}

var ksfSalt []byte

ksfSaltOffset := ksfParamOffset + 2 + (len(ksfParams) * 4)

if len(encoded) >= ksfSaltOffset+2 {

ksfSalt, _, err = encoding.DecodeVector(encoded[ksfSaltOffset:])

if err != nil {

return nil, fmt.Errorf("decoding the ksf salt: %w", err)

}

if len(ksfSalt) == 0 {

ksfSalt = nil

}

}

offset += confIdsLength

ksfEncodedParams, offsetKSF, err := encoding.DecodeVector(encoded[offset:])

if err != nil {

return nil, fmt.Errorf("decoding the ksf configuration parameters: %w", err)

}

offset += offsetKSF

var ksfParams []int

for i := 0; i < len(ksfEncodedParams); i += 4 {

ksfParams = append(ksfParams, encoding.OS2IP(ksfEncodedParams[i:i+4]))

}

ksfSalt, _, err := encoding.DecodeVector(encoded[offset:])

if err != nil {

return nil, fmt.Errorf("decoding the ksf salt: %w", err)

}

if len(ksfSalt) == 0 {

ksfSalt = nil

}

we can use the offset argument returned by DecodeVector

bytemare · 2024-12-29T03:05:36Z

tests/opaque_test.go

-func TestDeserializeConfiguration_InvalidContextHeader(t *testing.T) {
-	d := opaque.DefaultConfiguration().Serialize()
-	d[7] = 3
-
-	expected := "decoding the configuration context: "
-	if _, err := opaque.DeserializeConfiguration(d); err == nil || !strings.HasPrefix(err.Error(), expected) {
-		t.Errorf(
-			"DeserializeConfiguration did not return the appropriate error for vector invalid header. want %q, got %q",
-			expected,
-			err,
-		)
-	}
-}
-


I believe we should keep this error detection test.

bytemare · 2024-12-29T03:09:29Z

Hi @DJAndries ! Thank you for this PR, this is an excellent contribution :)
I'm sorry the delay, I haven't had time to get to this earlier.

I left some comments for some minor code changes.
I also believe we should keep the TestDeserializeConfiguration_InvalidContextHeader test function, but it would need some adaptation.

bytemare · 2024-12-29T03:10:29Z

Have you noticed other differences in Kevin's implementation that would be interesting to add here?

Use hash size as KSF output size, expose KSF parameters & salt in con…

9ecb8a5

…figuration

DJAndries requested a review from bytemare as a code owner November 5, 2024 00:09

bytemare requested changes Dec 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use hash size as KSF output size, expose KSF parameters & salt in configuration #73

Use hash size as KSF output size, expose KSF parameters & salt in configuration #73

DJAndries commented Nov 5, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare Dec 29, 2024

bytemare commented Dec 29, 2024

bytemare commented Dec 29, 2024

	if len(encoded) < confIdsLength+2 { // corresponds to the configuration length + 2-byte encoding of empty context
	if len(encoded) < confIdsLength+6 { // corresponds to the configuration length + 3*2-byte encoding of empty context and KSF parameters

	ctx, _, err := encoding.DecodeVector(encoded[confIdsLength:])
	ctx, offset, err := encoding.DecodeVector(encoded[confIdsLength:])

Use hash size as KSF output size, expose KSF parameters & salt in configuration #73

Are you sure you want to change the base?

Use hash size as KSF output size, expose KSF parameters & salt in configuration #73

Conversation

DJAndries commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bytemare commented Dec 29, 2024

bytemare commented Dec 29, 2024