strobe-discuss — STROBE general discussion

Re: [Strobe-discuss] x25519_sign_p2 in python

[David W. <dav...@gm...>]

Hey Evert,

If you're re-writing strobe in python, I guess you could just ignore the montgomery form and find an implementation that doesn't use this optimization? Why do you need to be able to handle montgomery form in python?

David

> On Jul 5, 2021, at 7:46 PM, Evert van Rossum <Eve...@qo...> wrote:
> 
> Classification | PRIVATE
>  
> Hi all,
>  
> Hope someone can help me with the following.
>  
> I’m trying to find a python equivalent of  the sc_montmul function, located in x25519.c
>  
> AFAICT, the sc_montmul performs a scalar multiplication on the Montgomery form of the Curve25519.
>  
> A naïve person would then go and look for some Curve25519_scalar_mult function in Python and sure enough, I could find one in pycrypto, pysodium, pyNaCl etc.
> Also I found some scalar multiplication routines in C (e.g. libsodium,https://nacl.cr.yp.to/scalarmult.html <https://nacl.cr.yp.to/scalarmult.html>)
> But they all seem to work with 2 inputs (call it scalar a * scalar b) and an output.
>  
> Now, the x25519_sign_p2 function not only takes into account scalar a and scalar b, but also the `out` argument!
>  
> I think I miss some theoretical knowledge about this, but could someone clarify how this sc_montmul works and point me to relevant or alternative (C code?) examples that perform the same operation on the 3 arguments?
>  
> Thanks in advance!
>  
> Regards,
> Evert
>  
> This email may contain highly sensitive, confidential information that may be privileged and is meant only for the use of the specific intended addressee(s). Your receipt is not intended to waive any applicable privilege. If you have received this email in error, please delete it and immediately notify the sender by separate email. Unauthorized disclosure of confidential information may cause irreparable harm to Qorvo.
> _______________________________________________
> Strobe-discuss mailing list
> Str...@li... <mailto:Str...@li...>
> https://lists.sourceforge.net/lists/listinfo/strobe-discuss <https://lists.sourceforge.net/lists/listinfo/strobe-discuss>

[Strobe-discuss] x25519_sign_p2 in python

[Evert v. R. <Eve...@qo...>]

Classification | PRIVATE

Hi all,

Hope someone can help me with the following.

I'm trying to find a python equivalent of  the sc_montmul function, located in x25519.c

AFAICT, the sc_montmul performs a scalar multiplication on the Montgomery form of the Curve25519.

A naïve person would then go and look for some Curve25519_scalar_mult function in Python and sure enough, I could find one in pycrypto, pysodium, pyNaCl etc.
Also I found some scalar multiplication routines in C (e.g. libsodium, https://nacl.cr.yp.to/scalarmult.html)
But they all seem to work with 2 inputs (call it scalar a * scalar b) and an output.

Now, the x25519_sign_p2 function not only takes into account scalar a and scalar b, but also the `out` argument!

I think I miss some theoretical knowledge about this, but could someone clarify how this sc_montmul works and point me to relevant or alternative (C code?) examples that perform the same operation on the 3 arguments?

Thanks in advance!

Regards,
Evert

This email may contain highly sensitive, confidential information that may be privileged and is meant only for the use of the specific intended addressee(s). Your receipt is not intended to waive any applicable privilege. If you have received this email in error, please delete it and immediately notify the sender by separate email. Unauthorized disclosure of confidential information may cause irreparable harm to Qorvo.

Re: [Strobe-discuss] About disco, a merge of Strobe with the Noise protocol framework

[Mike H. <mi...@sh...>]

Great work David!

I need to sit down and study this once end-of-year chaos is over.

Thanks,
— Mike

> On Dec 10, 2017, at 12:10 PM, David Wong <dav...@gm...> wrote:
> 
> Hello stroboscopes,
> 
> I've been working lately on merging the Noise protocol framework with
> the Strobe protocol framework. I've decided to write it down as an
> extension of the Noise protocol framework using the Strobe protocol
> framework as a tool/primitive inside of it (I thought it was making
> more sense like this), this means that most discussions around this
> work have been taking place in the noise mailing list.
> 
> Anyway, I was at BH EU a few days ago to present the work. I also
> wrote a blogpost about it:
> https://www.cryptologie.net/article/432/disco/
> 
> I use Strobe to simplify a lot of the handshakes in Noise, and since I
> have access to all the other functions of Strobe I also wrote simple
> APIs for a cryptographic library.
> 
> You can see more about it here: www.discocrypto.com
> 
> I'd appreciate any feedback on the spec, library, documentation, etc... :)
> 
> Cheers,
> David
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Strobe-discuss mailing list
> Str...@li...
> https://lists.sourceforge.net/lists/listinfo/strobe-discuss

[Strobe-discuss] About disco, a merge of Strobe with the Noise protocol framework

[David W. <dav...@gm...>]

Hello stroboscopes,

I've been working lately on merging the Noise protocol framework with
the Strobe protocol framework. I've decided to write it down as an
extension of the Noise protocol framework using the Strobe protocol
framework as a tool/primitive inside of it (I thought it was making
more sense like this), this means that most discussions around this
work have been taking place in the noise mailing list.

Anyway, I was at BH EU a few days ago to present the work. I also
wrote a blogpost about it:
https://www.cryptologie.net/article/432/disco/

I use Strobe to simplify a lot of the handshakes in Noise, and since I
have access to all the other functions of Strobe I also wrote simple
APIs for a cryptographic library.

You can see more about it here: www.discocrypto.com

I'd appreciate any feedback on the spec, library, documentation, etc... :)

Cheers,
David

Re: [Strobe-discuss] test vectors

[David W. <dav...@gm...>]

I've gone ahead and made some tests that should hit a lot of different
alignments:

https://github.com/mimoo/StrobeGo/blob/master/strobe/test_vectors/test_vectors.json

this is the file I've used with the reference strobe-python to test
these vectors: https://gist.github.com/mimoo/64e5054f4927a0df0033c8f5f29f4029

I've uncovered two bug alignments in my implementation thanks to them:

* one from the fact that a strobe-rate is 166 bytes vs a duplex-rate
which is 168 bytes. Functions I was using require 8-byte aligned
buffers to produce correct results
* one from the fact that I had a bad optimization for when we were
absorbing more or exactly 166 bytes

So that's a success ^.^ (traded for some past failures)

What else could I test?

* better alignment tests via always using forceF with different input sizes
* very large inputs

David

PS: I know there are already tests via the SHAKE test vectors but I
thought this would be nicer to have and setup

[Strobe-discuss] test vectors

[David W. <dav...@gm...>]

Hello!

I've successfully tested interoperability with Strobe's python library.

I'm trying to create some test vectors to facilitate future
implementations, I was wondering if anyone here wou'd be keen on
having a discussion about a good format for test vectors.

I've chosen JSON because it's easy to parse in every language. For the
moment I have something that looks like that:

{
  "test_vectors": [
    {
      "name": "run1",
      "operations": [
        {
          "name": "init",
          "input_data": "custom string",
          "input_length": 128,
          "state_after":
"9c7f75fa8b893ab70ad448513c0b5263dc0c475c551526f6733bea22f16cb57cd31f682e660ee912824a772201ee1394226f4afcb62d331293cc92e8a624acf6e1b60095e322bbfbc845e5b26995fe7d7c841374d1ff5898c92ee0636b06727321c92a603907035349ccbb1b92b7b0057e8fa87fcebc7e88656fcb45ae04bc34cabeaebe79d91750c0e8bf13b966504d1343597265dd8865adf91409cc9b20d5f47444041f97b699ddfbdee91ea87bd09bf8b02da75a96e947f07f5b65bb4e6efefaa16abfd9fbf6"
        },
        {
          "name": "KEY",
          "input_data": "303130313031",
          "state_after":
"3031303130310e6be02d3677fb5a70dfc1f3a6f889f561699f053f97f705d671383c613b13b3e4184397827ad73b9c26e13af703f0bf70ff10d71516bb8a05a5b69775f1722daf9178b71cfca96cf36cd0fab68cac22f3f35afec0bf5066656d142761e3200b9e76a9ddd6ad74932110afd4a776fd82515edfe028dd4858abfda1c3b6bff17e9b82aad3b519e51e704cba48a6e383c8504cd1f7b13d7a6ac6e9690ed05ae90744c5c32a97af08d66f15c9357bf20ea9276cab5ac736b8a68f25b095c409a1c48fd4"
        },
        {
          "name": "AD",
          "input_data":
"68656c6c6f2c20686f772061726520796f7520676f6f64207369723f",
          "state_after":
"3031303130310e6988485a1b947650b7ae848699fb904110f0701ff0986ab2514b55130413b3e4184397827ad73b9c26e13af703f0bf70ff10d71516bb8a05a5b69775f1722daf9178b71cfca96cf36cd0fab68cac22f3f35afec0bf5066656d142761e3200b9e76a9ddd6ad74932110afd4a776fd82515edfe028dd4858abfda1c3b6bff17e9b82aad3b519e51e704cba48a6e383c8504cd1f7b13d7a6ac6e9690ed05ae90744c5c32a97af08d66f15c9357bf20ea9276cab5ac736b8a68f25b095c409a1c48fd4"
        },
        {
          "name": "PRF",
          "input_length": 16,
          "output": "5ce86d0815c02a27d8bdd923f2cb0bd8",
          "state_after":
"000000000000000000000000000000006380b09c7f711ca1c97eb6612cf25c2ab2abfb9cba447b27e72134629c3947281a0be4044d4e03c60564c66885407a2e6982603b07390ab718c0fa70f25e857c189caf2bdf42786669bc2f5d43440869e8feb7d47780383bb82c84705220c4d08b5df18081cb4939f3a38d059ee18529e85c6dade82e3c8b3c6cd0b56203299bb7311dfcbb68d1b5be5df6d2b8697dd60e89ca4fd78ffbd1c22b939311775d913a8bf324b8ebac782e1f99605eccf03c0e59770c177812d1"
        }
      ]
    }
  ]
}

What do you think?

If this works, what kind of test vectors should we create? Here are few ideas:

* the strobe padding reaches the end of a rate
* the strobe padding starts on a new block
* the strobe padding is split in between two states

David

Re: [Strobe-discuss] Protocol naming and sane API

[Mike H. <mi...@sh...>]

> On Jul 24, 2017, at 3:02 PM, David Wong <dav...@gm...> wrote:
> 
>> Right, there’s just a potential problem if two people choose the same protocol name for different protocols.
> 
> Maybe you could have some hints on how to avoid this, but I wouldn't
> like it to be enforced.

OK.  You seem not to like URLs though.  Any idea that’s not a URL?  I’m thinking
something like:

Protocol designers SHOULD make an attempt to make the protocol name globally unique.
The simplest approach is to set the diversifier string to a URL (such as the web page of the
product or the protocol specification) or RFC number that has been allocated for that protocol.

Protocol designers SHOULD NOT (MUST NOT?) specify a string whose lower-case form
begins with http://, https://, ftp://, git://, or rfc\s?[\d+], unless the protocol is described at that url or RFC.

>> Thoughts?  I’m thinking length=R might actually be better...
> 
> I have to admit I only thought it was a good idea because it doesn't
> require you to know what R is needed.

Hm, OK.  I’ll suggest it as length=0, but note that you can use length=R if you really need that.

— Mike

Re: [Strobe-discuss] Protocol naming and sane API

[David W. <dav...@gm...>]

> Right, there’s just a potential problem if two people choose the same protocol name for different protocols.

Maybe you could have some hints on how to avoid this, but I wouldn't
like it to be enforced.

> Thoughts?  I’m thinking length=R might actually be better...

I have to admit I only thought it was a good idea because it doesn't
require you to know what R is needed.

Re: [Strobe-discuss] Protocol naming and sane API

[Mike H. <mi...@sh...>]

> On Jul 24, 2017, at 10:58 AM, David Wong <dav...@gm...> wrote:
>> META-RACHET
> 
> I think this is a good idea.

One other variant of this idea before I commit it to the spec:

We could suggest META-RATCHET(length=R) instead of length=0.  This has two
advantages and two disadvantages:

Advantage: It’s actually a ratchet, unlike length=0.  Prevents a possible screwup.
Advantage: You get a choice between precomputing the state before F (for size) or
after F (for speed).

Disadvantage: It’s slower by exactly one call to F.
Disadvantage: It requires knowing R, which makes it a little more variant-dependent.

Thoughts?  I’m thinking length=R might actually be better...

— Mike

Re: [Strobe-discuss] Protocol naming and sane API

[Mike H. <mi...@sh...>]

> On Jul 24, 2017, at 10:58 AM, David Wong <dav...@gm...> wrote:
> 
>> So STROBE has domain separation internally, which is that the first
>> operation should be META-AD(protocol name).  But there’s no spec for
>> the format of that string, so people might cause collisions by accident.
>> It should probably be a URL or something
> 
> The padding should allow you to avoid these kind of collisions no?
> Unless your protocol name includes the padding in itself somehow,
> which I don't think should happen (is that what you're thinking
> about?)
> URLs make me think of the way java do things and I don't like it.

Right, there’s just a potential problem if two people choose the same
protocol name for different protocols.

>> META-RACHET
> 
> I think this is a good idea.

OK, I’ll add this.  It doesn’t even need a version bump, since it’s just a usage hint.

>> Was it actually removed, or just never in the STROBE spec?
> 
> I guess I was confused, I was referencing this which is still in the spec:
> 
>> The parameters below define the protocol framework called "Strobe-Keccak-sec/b-v1.0.2", which is abbreviated to "Strobe-sec/b". In general, the name would be "Strobe-F-sec/b-vX.Y.Z”.

> I think Noise uses naming to also know when two peers are expected to
> be able to interact with each other. If two peers use Strobev1.0.2
> they are supposed to be able to communicate since the naming uniquely
> identifies the protocol.

> from Noise:
> 
>> Protocol names: The protocol name used with Initialize() must uniquely identify the combination of handshake pattern and crypto functions for every key it's used with (whether ephemeral key pair, static key pair, or PSK). If the same secret key was reused with the same protocol name but a different set of cryptographic operations then bad interactions could occur.
> 
> Which I think is applicable to Strobe as well.
> 
> David

Hmm, I see.

STROBE currently has two levels of domain separation: one that separates 
STROBEvX.Y.Z from other cSHAKE uses, and one that separates protocols
built on STROBEvX.Y.Z from each other.

In the current version, the first domain separation string isn’t the full name of the
protocol framework, just the part that isn’t already determined by other parts of
initialization.  So the full name might be STROBE-Keccak-128/1600-v1.0.2, but
the initialization string will be “STROBEv1.0.2” with the Keccak-128/1600 part
implicit in the permutation calls and padding.

We could change this to make the cSHAKE domain separation the full name of
the protocol, as you were suggesting, and then scrap the second layer.
But we’d have to be careful to avoid polluting the cSHAKE namespace.  Again
I’m concerned that this might require using URLs or a registry or something.  I’d
prefer to leave this as is, but I’m open to being convinced otherwise.

Alternatively, we can leave it at STROBEvX.Y.Z, and then you can call strobe_init()
with whatever protocol name you want so long as you take reasonable steps
to make sure it’s unique.  If you want that to be the complete name of the protocol
(including interop details like “Keccak”), that’s fine by me.

Thoughts?

— Mike

Re: [Strobe-discuss] Protocol naming and sane API

[David W. <dav...@gm...>]

> There are some uses, but you could always discourage use of *_ENC
> by calling them *_ENC_UNAUTHENTICATED or something.

I like the idea.

> So STROBE has domain separation internally, which is that the first
> operation should be META-AD(protocol name).  But there’s no spec for
> the format of that string, so people might cause collisions by accident.
> It should probably be a URL or something

The padding should allow you to avoid these kind of collisions no?
Unless your protocol name includes the padding in itself somehow,
which I don't think should happen (is that what you're thinking
about?)
URLs make me think of the way java do things and I don't like it.

> META-RACHET

I think this is a good idea.

> Was it actually removed, or just never in the STROBE spec?

I guess I was confused, I was referencing this which is still in the spec:

>  The parameters below define the protocol framework called "Strobe-Keccak-sec/b-v1.0.2", which is abbreviated to "Strobe-sec/b". In general, the name would be "Strobe-F-sec/b-vX.Y.Z".

I think Noise uses naming to also know when two peers are expected to
be able to interact with each other. If two peers use Strobev1.0.2
they are supposed to be able to communicate since the naming uniquely
identifies the protocol.

from Noise:

> Protocol names: The protocol name used with Initialize() must uniquely identify the combination of handshake pattern and crypto functions for every key it's used with (whether ephemeral key pair, static key pair, or PSK). If the same secret key was reused with the same protocol name but a different set of cryptographic operations then bad interactions could occur.

Which I think is applicable to Strobe as well.

David

Re: [Strobe-discuss] Protocol naming and sane API

[Mike H. <mi...@sh...>]

> On Jul 22, 2017, at 9:11 PM, David Wong <dav...@gm...> wrote:
> 
> Hello there!

Hi David!

> I've been working on an extension of Noise based on Strobe. The
> current draft (2) is available here:
> https://github.com/mimoo/NoiseGo/blob/master/disco/specification.md
> and a readable implementation of the extension is readable here:
> https://github.com/mimoo/NoiseGo/blob/master/disco/disco.go
> 
> What it does is that it basically remove any symmetric crypto in Noise
> and replaces it with Strobe calls, which simplifies the complicated
> continuous hash and chaining (as well as the spec any implementation).

This is pretty neat!

> Now I'm not here to talk about that, but rather about things I've been
> thinking as I work on these.
> 
> 1) Are there any use cases for using Send_ENC without it being
> followed by a Send_MAC? I've actually been defining a new pair of
> functions: Send_AEAD and Recv_AEAD (combining *_ENC and *_MAC) and
> I've been thinking of making *_ENC private functions.

There are some uses, but you could always discourage use of *_ENC
by calling them *_ENC_UNAUTHENTICATED or something.

You could, for example, SEND_ENC and then sign instead of MACing.
Or you could send multiple things in one packet and then MAC or sign
at the end, eg send_META_ENC(packet header); send_ENC(data); send_MAC()
for steg.  Or after an ephemeral DH key exchange, you could send_ENC(cert);
some DH stuff with the cert; send_MAC().  But there you could always send a
MAC anyway, it’d just use more bandwidth.

SEND_ENC is also used in the signature itself, at least in the STROBE
example software, to avoid a corner case.  If you were to SEND_CLR
the Schnorr response, then a zero signature with a zero public key might
be a valid sig for all messages, which is not really in the threat model for
signatures but probably isn’t what you want either.

> 2) I'm currently adding the version of Strobe to the Noise's naming
> way, which looks like this:
> 
> Noise_[PATTERN]_[KEYEXCHANGE]_STROBEvX.Y.Z

Makes sense.

This actually brings up a pair of minor issues in STROBE, which might be
worth looking at for the next spec revision.

So STROBE has domain separation internally, which is that the first
operation should be META-AD(protocol name).  But there’s no spec for
the format of that string, so people might cause collisions by accident.
It should probably be a URL or something.  Maybe that should be a SHOULD
in the spec and not a MUST, not sure.

The second issue is that strobe_init() doesn’t end a block, so the META-AD
will end halfway through a block.  This is related to the issue you brought
up with KEY operations not ending a block, the way that they do in KMAC.
This is just a minor efficiency issue: it means that if you precompute the
state after strobe_init(your favorite proto) or after keying a sponge, it will be
partway through a block, so you may have to call Keccak() sooner than if it
were at the beginning of a block.

A possible solution to this is to define an operation for “I just want to end the
block for efficiency reasons or whatever”, and possibly to spec that it be used
in strobe_init() in future revisions of STROBE.  Maybe
META-RATCHET(empty string) could be reserved for this?  RECV-RATCHET
(flags = CI) doesn’t have a defined use either yet, or we could define some
kind of META-AD+RATCHET() pair, but META-RACHET is probably a more
natural choice than either of these.

As for your domain separator, that works and you can do as you like, but it’d
be more portable to existing and future STROBE libraries if it just did
strobe_init(“Noise_[PATTERN]_[KEYEXCHANGE]”) or a URL-ized version
of that.

> This does not include the permutation being used in Strobe and I was
> wondering why it had been removed in the Strobe spec?

Was it actually removed, or just never in the STROBE spec?

The purpose of domain separation is to separate STROBEvX.Y.Z from
other primitives that use Keccak or whatever other permutation you’re
using, so that you won’t hose yourself by using STROBE and KMAC
with the same key, or something like that (not that this is recommended,
but it shooooouuuuuld be secure).

But STROBE with Keccak is inherently separated from STROBE with
any other permutation, because one is calling Keccak and the other is
calling NORX or whatever.  The same is true for different sizes of Keccak.
It isn’t inherently true for different rates with the same permutation, but
cSHAKE’s domain separation includes the rate, as does STROBE’s padding.

The version is important, because if STROBEv2 changes the way the
padding works, then inputs to Keccak could collide with STROBEv1.  The
inclusion of version in the domain separation prevents this.

So that’s why it includes the mode name (STROBE) and the version, but
not the rate/security level or the permutation.

> Thanks!
> David

Cheers,
— Mike

[Strobe-discuss] Protocol naming and sane API

[David W. <dav...@gm...>]

Hello there!

I've been working on an extension of Noise based on Strobe. The
current draft (2) is available here:
https://github.com/mimoo/NoiseGo/blob/master/disco/specification.md
and a readable implementation of the extension is readable here:
https://github.com/mimoo/NoiseGo/blob/master/disco/disco.go

What it does is that it basically remove any symmetric crypto in Noise
and replaces it with Strobe calls, which simplifies the complicated
continuous hash and chaining (as well as the spec any implementation).

Now I'm not here to talk about that, but rather about things I've been
thinking as I work on these.

1) Are there any use cases for using Send_ENC without it being
followed by a Send_MAC? I've actually been defining a new pair of
functions: Send_AEAD and Recv_AEAD (combining *_ENC and *_MAC) and
I've been thinking of making *_ENC private functions.

2) I'm currently adding the version of Strobe to the Noise's naming
way, which looks like this:

Noise_[PATTERN]_[KEYEXCHANGE]_STROBEvX.Y.Z

This does not include the permutation being used in Strobe and I was
wondering why it had been removed in the Strobe spec?

Thanks!
David

Re: [Strobe-discuss] Some feedback on the specification

[David W. <dav...@gm...>]

> Right, you can’t because these use Keccak-f[not 1600], so they can never be cSHAKE.
> But I wanted it to be cSHAKE for Keccak-f[1600] so that you could claim some degree
> of standards compliance.

I see what you are saying. I think I'm starting to agree with you, but
not for variants
with b < 1600. What about initializing these with how cSHAKE would
have been defined for
these smaller Keccak-f?

Strobe-128/400:

  bytepad(encode_string(N) || encode_string(S), 18)

Strobe-256/800 -> 36

  bytepad(encode_string(N) || encode_string(S), 36)

Strobe-128/800 -> 68

  bytepad(encode_string(N) || encode_string(S), 68)

(by the way, the rate is so small for Strobe-128/400, won't that be uber slow?)

> True.  But you can still use a const for R in a practical low-level implementation, and initialize > with R+2 which is also constant.  The initialization array is just going to be in memory as a literal either way.

True! I blindly trusted your reference snippets.

> Ah, right.  It’s R<254 so that R+2 fits in a byte in initialization. So there’s another cost to the +2, except that in practice R is probably at most 168 in all cases.

That makes more sense! Then let me point to you that you wrote R<=254
in the spec, not "<" ^^

I would also suggest modifying 7.2's snippet. Right now you init R as:

    self.R  = F.nbytes - sec//4

which looks weird if you read that after reading how R is supposed to
be calculated (missing a -2),
and then it is permamently changed:

    self.R -= 2

it's a detail but it tripped me on my first read of the spec. I would
find this to be clearer:

    self.R  = F.nbytes - sec//4 - 2
    ...
    domain = bytearray([1,self.R+2,1,0,1,12*8])
    ...

> Actually, I’m wrong: in practice you can set it to anything you want because
> the first operation is the protocol domain separator, and that will initialize flags for you.

I'm not sure I follow here. The protocol domain separator is an AM operation:

    self.operate(A|M, proto)

so it is still a valid flag (as well as != 0)

> Well, there are reasons, but they reflect at least partially on how I wanted the high-level operation implemented.

I see what you're saying. Instead of creating the operations first you
started with rules and properties (A, I, T, ...), then match the
relevant ones to each operations (KEY, RATCHET, ...) so that
operations makes sense.
As a normal reader of the spec you do not really see that though, you
rather see an obfuscated
algorithm :D
I was also looking for problems with your design, and it was really
hard to audit the flag magic.
For example I spent way too much time making sure that this one did
what you wanted it to do:

    if (flags & (I|T) != (I|T)) and (flags & (I|A) != A):

while on the other hand I believe something like this is clearer:

    if operation == "PRF" || operation == "send_MAC" || operation == "RATCHET" {

Now that you've figured out what operations you want to have in Strobe
I think it would make
sense to de-obfuscate these operations a bit. Just my 2 cents :)


> Yes, I need to improve API friendliness.  Adding a meta argument is a good idea.
> As for length, I don’t know.  In high-level languages at least, it’s nice to be able to
> send strings or bytearrays without doing eg send_enc(data, len(data)).

This is not what I'm doing, see:
https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L403:L411

when sending real data, length must be zero. If a length is needed
(PRF, send_MAC or RATCHET),
then use an empty buffer as the data argument. In the end:

1. this can be hidden by high level operations anyway
2. I did this to optimize one operation: RATCHET. Because it
theoreticaly doesn't need to allocate
a buffer. But now I'm thinking that you probably wouldn't call RATCHET
a lot in the first place and
that optimizing it would just complexify the code for little time gained.

So I'm not sure what I'm doing is that important (having two different
arguments: data and length),
but I still think this bytearray trick is confusing to have in the spec:

  def _duplex(self, data, cbefore=False, cafter=False, forceF=False):
    assert not (cbefore and cafter)
    data = bytearray(data)


Thanks!
David

Re: [Strobe-discuss] Some feedback on the specification

[Mike H. <mi...@sh...>]

> On May 30, 2017, at 5:56 AM, David Wong <dav...@gm...> wrote:
> 
> Hey Mike!
> 
> Thanks for the answer, some responses inline:
> 
>> It’s late and on a weekend, so please correct me if I missed or misunderstood something you said, or if my response or patch on the website isn’t clear.
> 
> Is it possible to see older versions of the spec? I like to just diff
> specs when there is a change to see where.

I just added the website to the git repository, so now you can diff it.

>> I don’t mind you changing the APIs, but of course changing the spec will lead to compatibility problems.
> 
> I should have made myself clearer: this implementation was here to
> kick around ideas and understand your design decisions. I'm not
> planning to fork your spec! rest assured!

Glad to hear it :-)

>> In particular, I don’t know why you want to initialize with R.  The whole point of initializing with
>> (1, R+2, 1, 0, len(S)*8, S, 0…) is to match cSHAKE.  If you aren’t going to follow cSHAKE, there’s no
>> point in adding (1,0) at least, since that’s the empty NIST string, and there’s no point in multiplying
>> len(S) by 8 but not multiplying R by 8.
> 
> So, I think you have a valid point here. One problem I see though, is
> that cSHAKE specifies only two variants with rate = 168 bytes or rate
> = 136 bytes, how would you adapt that to Strobe-128/400,
> Strobe-128/800 and Strobe-256/800 which have 18, 68 and 36 bytes of
> rate respectively?

Right, you can’t because these use Keccak-f[not 1600], so they can never be cSHAKE.
But I wanted it to be cSHAKE for Keccak-f[1600] so that you could claim some degree
of standards compliance.

Also, the Python code still works, you just put R+2 in the init field.

> One way of adapting that is to take cSHAKE's logic and extending their
> spec by altering the rate value used in the padding, which is what I'm
> proposing and I like that it allows you to just use a const for R
> (https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L91).

True.  But you can still use a const for R in a practical low-level implementation, and initialize with R+2 which is also constant.  The initialization array is just going to be in memory as a literal either way.

>> This is true.  That’s why the spec requires that R < 254.
> 
> I think it should be fine if R = 255 as well, the final index is R-1=
> 254 so pos_begin would be 255 which is still writable in a full byte.

Ah, right.  It’s R<254 so that R+2 fits in a byte in initialization. So there’s another cost to the +2, except that in practice R is probably at most 168 in all cases.

>> Technically flags=0 isn’t invalid though.
> 
> What kind of operation would it be? Wouldn't it be more sensitive to
> forbid such flags/non-defined operations and set cur_flags = 0 during
> initialization? This would simplify the type of the variable as well
> in implementations.

Actually, I’m wrong: in practice you can set it to anything you want because
the first operation is the protocol domain separator, and that will initialize flags for you.

Flags=0 would be a time-wasting operation, kind of like pbkdf2 or something.
Probably not useful.

>> I mean, sort of?  The idea is to categorize the operations in such a
>> way that the implementation is systematic.  So that’s why it’s based
>> on flags.
> 
> ekr has an interesting take on this here:
> https://www.ietf.org/mail-archive/web/tls/current/msg22204.html
> 
> and here is the quote:
> 
>> Most IETF specs leave the choice of how to do things up
>> to the implementor. As someone who has implemented a lot of
>> specs, I actually find it pretty frustrating when the author
>> gets all prescriptive about implementation. If there are
>> points that are unclear, I think it's fine to make them clear,
>> but I'd rather stick to the PDUs.
> 
> I would agree with you though. I like biased specs (Noise do that
> also) because I think they are safer to implement in the end. That
> being said, I don't think it's orthogonal to having clear explanations
> of what the algorithm is doing under the surface.

I meant “implemented” in a mathematical sense more than in a code sense.  Like,
send_ENC is supposed to encrypt, but with what formula?

I found it simpler to say, this is outbound, uses app data and transport data, and uses
the cipher, so you must do the things associated with each of those properties, no
more and no less.

That approach does use a little bit of its luster because of some of the arbitrary decisions
though.  Like, why is KEY AC and not just A?  And why do KEY=AC and RATCHET=C
overwrite state, but ENC=ACT doesn’t?

Well, there are reasons, but they reflect at least partially on how I wanted the high-level operation implemented.

>> Also, technically the other combinations of flags are not (currently)
>> forbidden.  In particular, you can use flags=0 to waste time, even
>> though you can also waste time by calling RATCHET.
> 
> I'm not sure I see the point in that. In my implementation I actually
> assert operations:
> https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L382:L387 <https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L382:L387>

Yeah, fair enough.

>> But I’ve added outer functions that call operate() with the right flags.
> 
> I think this is a good idea, see:
> https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L25:L80
> 
>> Yeah.  My C implementation takes a length and a nullable array of bytes.
>> I’ve tried to clarify this, but perhaps I should rewrite it as length * [0] or something.
> 
> I'm doing the same thing right now but it is sometimes not optimal,
> for example RATCHETing could benefit from not creating this zero
> buffer (but at the cost of complexifying the code).
> 
> See: https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L394:L401
> 
> I also have this "length" argument that I use for PRF, RATCHET and
> send_MAC. You're using a `bytearray()` hack in the spec, but I think
> it would be better to have a length in your API for lower level
> languages :)
> 
> Talking about ideas I toyed with, I also use a meta argument in the
> `Operate` function:
> 
> https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L389:L392
> 
> Which I think is nicer to developers

Yes, I need to improve API friendliness.  Adding a meta argument is a good idea.
As for length, I don’t know.  In high-level languages at least, it’s nice to be able to
send strings or bytearrays without doing eg send_enc(data, len(data)).

>> Simplicity, code size and memory
> 
> I think the strong point of any library is also
> availability/portability,

A strong point anyway.  Making it easy to use the library securely is also really,
really important … and is an area where the current API needs testing and
improvement.

> your library can only be compiled with ARM
> at the moment :]

Fixed, at least partially, in 158c58b.  Thanks.

It was never just for ARM.  Actually “make all” used to build for native and for
ARM, and there’s no ARM-specific code in the repo.  I was just including an
ARM build to make sure it would still build for ARM, and to benchmark size
and stack usage.

But I really do need to learn autoconf one of these days so that I can make
proper build scripts.  Also, I should make it build for multiple sponges, and then
re-integrate strobe-keccak-f[1600] into libdecaf.  Work work...

>> Also, if you’re cooking a custom protocol,
>> STROBE is supposed to make it easier to get it right, but we need real-world
>> testing (and probably better documentation) to demonstrate that.
> 
> Any project in mind :) ?

Hm.  I sometimes have IoT toy projects, but I’m more likely to use it first in a CRI-proprietary thing since I get paid to do that :-)

>> Speed is not really an advantage.
> 
> Have you thought of using 12 rounds a la KangarooTwelve instead of 24
> for the underlying Keccak function?

Reduced-round is probably workable.  It wouldn’t be NIST compliant anymore, but it would probably be secure with half the rounds.

> Thanks for all the answers! It all makes a lof of sense :)
> 
> David

Thanks for hammering on this,
— Mike

Re: [Strobe-discuss] Some feedback on the specification

[Mike H. <mi...@sh...>]

Thanks so much for looking this over, David!

Here’s a rough response, and I’m also pushing changes to strobe.sourceforge.io.

It’s late and on a weekend, so please correct me if I missed or misunderstood something you said, or if my response or patch on the website isn’t clear.

> On May 16, 2017, at 2:33 AM, David Wong <dav...@gm...> wrote:
> 
> Heyo all and Mike!
> 
> First, thank-you Mike for coming-up with STROBE! It is really awesome
> research and I've had a lot of fun reading the paper/spec and
> implementing it :)

I’m glad that at least someone found it interesting.

> I have a not well tested, and very biased implementation, here:
> https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go

I still need to take a careful look at this.

By the way, the line
“”"
if operation == "PRF" || operation != "send_MAC" || operation != “RATCHET” {
“”"
probably doesn’t do what you want.

> It's more of a "readable" implementation than an efficient one. I've
> taken some decisions and made the code longer in places, changed the
> APIs and broke the spec in some other places (for example I initialize
> with R, not with R+2).

I don’t mind you changing the APIs, but of course changing the spec will lead to compatibility problems.

In particular, I don’t know why you want to initialize with R.  The whole point of initializing with
(1, R+2, 1, 0, len(S)*8, S, 0…) is to match cSHAKE.  If you aren’t going to follow cSHAKE, there’s no
point in adding (1,0) at least, since that’s the empty NIST string, and there’s no point in multiplying
len(S) by 8 but not multiplying R by 8.

> Here is some feedback. This is a pretty long post so I apologize in
> advance. A more "readable" version is available here:
> https://gist.github.com/mimoo/96906d703f133b7f6aeab68c090ed59a
> 
> ## Sponge
> 
> I don't think a Duplex construction is a sponge construction, it does
> not rely on a sponge either, they just share the permutation. I think
> any mention of the word "sponge" should be removed from the
> specification as it produces confusion. For example "sponge state"
> could be changed to "duplex state" or better "strobe state”.

Noted and changed (in most places anyway).

> ## Protocol name and its length
> 
> In section 4:
> 
>> The parameters below define the protocol framework called "Strobe-Keccak-sec/b-v1.0.2", which is abbreviated to "Strobe-sec/b". In general, the name would be "Strobe-F-sec/b-vX.Y.Z".
> 
> then this is the string you use to initialize the state in section 5.1:
> 
>> "STROBEvX.Y.Z
> 
> Why not using the specified "STROBE-Keccak-sec/b-vX.Y.Z”?

I wanted to shorten the string for simplicity, and because I didn’t want to
go over one block for initialization, even for Keccak-f[400].

The goal of initialization is that you can determine what’s going on from
the first few calls (in this case, first 1 call) to the permutation.  You can
tell it’s Keccak-f[n] because that’s the permutation you’re calling.  You
can tell the rate (and thus, the security level) because it’s earlier in the
initialization string.  But you can’t tell the framework name (STROBE
as opposed to, I dunno, BLINKER) or version.

So STROBEvX.Y.Z is enough to separate it.

> ## cSHAKE
> 
> cSHAKE is mentioned many times. I think it should not. It's good to
> mention it in the paper and to explain how the padding of STROBE was
> created. But I think it adds confusion more than anything in the spec.

I’ll try to reduce the mentions of cSHAKE.  A bunch of the padding things
are purely “because cSHAKE does this”, but maybe it does no good to
mention that.

> Furthermore, the initialization in section 5.1. is not using the
> correct cSHAKE formatting. It should be `bytepad(encode_string(N =
> empty) || encode_string(S), 168)` whereas right now it is just
> `left_encode(S) || S = "STROBEvX.Y.Z\00"`. I also see no reason to
> count a null terminator character at the end (not everything is C :D).

Yeah, oops, this was full of typos, and it was from the draft cSHAKE
which was left_encode(S) || left_encode(N).  In the earlier version,
left_encode(“") was "\0", not "\1\0".  It really should be “\0" anyway,
but NIST special-cased it for some reason.

> In spite of this, the initialization in section 7.2 is correct!
> 
> ```py
> domain = bytearray([1,self.R,1,0,1,12*8]) + bytearray("STROBEv1.0.2")
> self._duplex(domain, forceF=True)
> ```
> 
> which is indeed
> 
> ```
> [encode length rate | encode N | N | encode STROBE string in bytes |
> STROBEv1.0.2]
> ```

Yeah, I fixed that one when NIST updated the spec, but I missed the
other one.

> ## cSHAKE test vectors
> 
> section 3:
> 
>> Strobe-128/1600 and Strobe-256/1600 are based on the draft draft cSHAKE specification from NIST. If that specification changes, then the initialization and padding of Strobe might need to be adjusted. It is also possible that I have misinterpreted the cSHAKE specification, since **there are no test vectors yet**.
> 
> [there are test vectors
> now](http://csrc.nist.gov/groups/ST/toolkit/documents/Examples/cSHAKE_samples.pdf)!

Fixed.

> ## Byte Convention
> 
> In section 2:
> 
>> Strobe operates almost exclusively on bytes, which are elements of the set [0,...,255]. When data types larger or smaller than a byte are used, it follows a little-endian convention.
> 
> I recommend against writing a spec for things smaller than a byte.
> Just because it unnecessarily complicate things.

Strobe is for encrypting/decrypting/whatever bytes, and not smaller or larger things.  But larger is needed for length fields, and smaller is needed for key tree.  I’ve clarified this.

> ## Strobe's padding
> 
> I wrote up what I understood of the padding from the snippets:
> https://cryptologie.net/article/392/strobes-padding/
> 
> One thing I find weird is that if you start an operation in a new
> block, you will still have an `old_begin = 0` starting the block,
> which is kind of redundant with the previous block `pos_begin` (by the
> way, I think you should rename that to `op_begin`).
> 
> This could be avoided by the following algorithm when starting a new operation:
> 
> * Use `0xff` to mean "operation started in a previous block" instead of `0`.
> * If I’m at `pos != 0`, then append `old_begin`, then start the new
> operation (nothing changed).
> * If I’m at `pos = 0`, do not add the `old_Begin`; add the new
> operation with `pos_begin = 0`.

As we discussed on slack, switching 0 -> 0xFF is basically just writing
pos_begin-1 instead of pos_begin.  Skipping pos=0 doesn’t save much
because there’s not a condition that forcibly causes that, and anyway it’s
tricky to make that parseable.

> ## Potential weird pos_begin value
> 
> section 7.3 in the snippet:
> 
> ```py
> oldbegin, self.posbegin = self.posbegin, self.pos+1
> ```
> 
> if we start an operation at the last byte of a block (R-1),
> `self.posbegin` will be out of the block size (=R) boundaries.
> Example:
> 
> current buffer:
> 
> ```
> [data, ...]
> ```
> 
> new operation starts:
> 
> ```
> [data, ..., old_begin, pos_begin=R, 0x04, 0, ..., 0, 0x80]
> [flag, data, ..., pos_begin = 0, 0x04, ..., 0x80]
> ```
> 
> Which is parseable but weird?

This is true.  That’s why the spec requires that R < 254.

I dunno, I could subtract 1 from everything, but I think that 0xFF for
“began last block” is also a little weird in an otherwise unsigned field.
So I’m not really inclined to change it.

> ## 7.2. Initialization
> 
> ```py
> self.cur_flags = None
> ```
> 
> I think this could be set to 0 instead of having a None value.

Technically flags=0 isn’t invalid though.

> ```py
> self.R  = F.nbytes - sec//4
> # ...
> # initialization happens
> # ...
> self.R -= 2
> ```
> 
> Is it a problem if the initialization spans over multiple blocks? In
> my implementation I'm using a constant R instead of having an extra 2
> bytes in the initialization.

Not really.  But the point of the weird initialization sequence (at least for
/1600) is to match cSHAKE, so if you’re going to change it it might as
well be something much simpler.

> Strobe-128/400 is the smallest instance, this implies `R = N -
> (2*sec)/8 - 2 = 400/8 - (2*128)/8 - 2 = 16`. The snippet's input is
> `[1, strobe_R, 1, 0, 1, 12*8, "STROBEv1.0.2"]` → 18 bytes. This will
> work with `R + 2`, but not if you use a longer string anyway.
> 
> So I propose not having this varying R and just accept the fact that
> the initialization might span over several blocks. (In my
> implementation I do not use the `R+2` trick.)
> 
> 
> ## Test Vectors?
> 
> That would be useful to test one's implementation :]

Noted.  TODO though.

> ## Things that are not clear in the spec
> 
> A lot of my critiques here is that a lot of the spec is understandable
> by looking at the code snippets, whereas I think it should be written
> in words. Here's a non-exhaustive list of things that bugged me:

If you find other things, please ping me about them.

> **operations per block?**
> 
> It is not clear from the spec without looking at the code if the
> permutation has to be run after each operation, or if two operations
> can take place before one permutation does.

Added, thanks.

> **_duplex**
> 
> It would be nice if every operation section (`KEY`, `CLR`, ...)
> described exactly what it did. For example here is how you could
> define the `KEY` section (6.1.2):
> 
> ```
> KEY: Provide cipher key
> flag: AC
> description: <!-- same one -->
> If the operation does not start in a new block, apply the permutation.
> After this, replace the strobe state with the key.
> ```

Added.

> **customization string**
> 
> The nice feature of STROBE is that you can have a single primitive in
> code, and use it for your symmetric protocol, for a PRNG, as a hash
> function, etc... But this feature is not clearly documented, the only
> mentions of a "customization string" are through the `proto` variable
> and the comment: "Turn on Strobe padding and do per-proto separation".
> I think this would benefit from more explanations.

I’ve added at least a little bit of explanation about this.  Let me know if
you think it needs more.

> **Streaming mode**
> 
> Streaming is not clearly explained in my opinion. It needs an example!
> Here's one:
> 
> ```py
> operate(A, "this is a long")
> operate(A, "message", more=True)
> ```
> 
> is equal to
> 
> ```py
> operate(A, "this is a long message")
> ```

Added.

> 
> **Initiator**
> 
> From section 5:
> 
>> A variable I0 which is 0, 1 or None. This variable marks whether the current party's role is the initiator (I0 = 0), responder (I0 = 1), or undecided (I0 = None)
> 
> Then in section 7.3:
> 
> ```py
> def _beginOp(self, flags):
>    # Adjust direction information so that sender and receiver agree
>    if flags & T:
>        if self.I0 is None: self.I0 = flags & I
>        flags ^= self.I0
> ```
> 
> Again, I think this should be put into words. My attempt:
> 
> The role is chosen when the first transport operation (T) is used:
> 
> * if the message is inbound: we're the responder
> * if the message is outbound: we're the initiator
> 
> After that, if we're the receiver of a transport operation (T), the
> received flags are toggled so that the responder can use the same flag
> in the duplex call.

This is also mentioned in Section 2, but I’ve added some more info about this.

> **As a hash**
> 
> What if I want to use the STROBE code as a hash function?
> The 6.1.6: PRF section doesn't really detail that. My take:
> 
> ```py
> strobe = new strobe(proto="hash")
> strobe.Operate(A, data="message to be hashed")
> strobe.Operate(IAC, output_length)
> ```

Added

> ## Code Clarity
> 
> **Main Operation**
> 
> ```py
>    if (flags & (I|T) != (I|T)) and (flags & (I|A) != A):
>        # Operation takes no input, only a length
>        assert isinstance(data,int)
> ```
> 
> The use of `isinstance` does not help the implementer for low level languages.

Clarified.

> **cbefore & cafter in _duplex**
> 
> In section 7.1.:
> 
> ```py
> for i in range(len(data)):
>  if cbefore: data[i] ^= self.st[self.pos]
>  self.st[self.pos]   ^= data[i]
>  if cafter:  data[i]  = self.st[self.pos]
> ```
> 
> I don't think optimization should be in a spec. This is clearer:
> 
> ```py
> for i in range(len(data)):
>  if cbefore:
>    state_temp = self.st[self.pos]
>    data[i] ^= self.st[self.pos]
>    self.st[self.pos] = data[i] ^ state_temp
>  if cafter:
>    self.st[self.pos] ^= data[i]
>    data[i] = self.st[self.pos]
> ```

I don’t see this as clearer, and I don’t see the other as an optimization.

You either apply the state as a cipher (“C”) before or after absorbing
the data, or neither.  It seems to me that this translates most naturally
to the existing code.

> **Flags**
> 
> My understanding of the flags in the logic (not talking about using
> them in the state): they're here to reduce lines of code. I think this
> is a major barrier in facilitating understanding of the spec. I'd
> prefer to see an actual explanation of the flow (instead of snippets
> of code), and in snippets a clearer logic. What I'm doing in my
> implementation: `switch(operation) { case "KEY": ... case ...`.
> 
> Should the specification talk about optimizations? Or should it be the
> role of a different document/reference implementations?

I mean, sort of?  The idea is to categorize the operations in such a
way that the implementation is systematic.  So that’s why it’s based
on flags.

Also, technically the other combinations of flags are not (currently)
forbidden.  In particular, you can use flags=0 to waste time, even
though you can also waste time by calling RATCHET.

But I’ve added outer functions that call operate() with the right flags.

> **_function()**
> 
> Correct me if I'm wrong, but you're using underscore (`_`) prior to a
> function name to mean that it should not be considered part of the
> public API. If this is the reason you are using it, I think you should
> add that to the specification, not everyone might know this :)

Added.

> **Python as pseudo-code**
> 
> Just my own opinion: I like Python and the snippets of code in the
> specification. It made it really easy to understand the spec. But I'd
> rather have pseudo-code in the spec and a reference implementation in
> python. This would remove a lot of the ugly `self`, or python-centric
> functions like `isinstance()`.

True.  But then I’d have to figure out what semantics I want from
my pseudocode.  Maybe I’ll change to this later, but for now it’s easier
to use Python.

> **_duplex for PRF/MAC/...**
> 
> ```py
> def _duplex(self, data, cbefore=False, cafter=False, forceF=False):
>    assert not (cbefore and cafter)
> 
>    # Copy data, and convert string or int to bytearray
>    # This converts an integer n to an array of n zeros
>    data = bytearray(data)
> ```
> 
> This is clear if you know python, but it doesn't help you for lower
> level languages. What needs to be done here: if the operation is
> MAC/PRF/... sends a data = [0...0] of length data to _duplex
> 
> In my implementation I accept an length parameter in the `Operation` function.

Yeah.  My C implementation takes a length and a nullable array of bytes.
I’ve tried to clarify this, but perhaps I should rewrite it as length * [0] or something.

> ## Noise + STROBE
> 
> Just a thought: I think it should be possible to use the STROBE object
> to replace the symmetric object in NOISE. This would be really
> interesting to see and could greatly reduce the complexity of the
> noise spec :]
> 
> Strobe, like Noise is a framework to create protocols. What would be
> cool to see is an actual Noise-Strobe (or Noibe :D) protocol
> specified, implemented and ready to be used :)

That’s a great idea, though I don’t really want to step on Trevor’s feet.

> ## Noise on embedded devices
> 
> I'm wondering how advisable STROBE will become. What would you say are
> the advantages of STROBE for embedded devices? The number of buffers
> it uses? The maximum length of these? The speed? The number of
> mallocs? The lines of codes (both for space and ease of code review)?
> The calls to keccak that could be implemented in hardware in the
> future?

Simplicity, code size and memory.  Also, if you’re cooking a custom protocol,
STROBE is supposed to make it easier to get it right, but we need real-world
testing (and probably better documentation) to demonstrate that.

That you could offload it to hardware is also kind of neat, but it’s a little bit
niche for most developers since I don’t think most vendors will add hardware
support for Keccak.

Speed is not really an advantage.  On a small MCU, STROBE is probably
slower than chacha+poly1305 or BLAKE2, and about the same speed as
AES-CCM.  But it’s only a little slower, it’s not horrible.  Also you might find
hardware acceleration for AES.

> Regards,
> David

Thanks again,
— Mike

Re: [Strobe-discuss] Some feedback on the specification

[David W. <dav...@gm...>]

Hey Mike!

Thanks for the answer, some responses inline:

> It’s late and on a weekend, so please correct me if I missed or misunderstood something you said, or if my response or patch on the website isn’t clear.

Is it possible to see older versions of the spec? I like to just diff
specs when there is a change to see where.

> I don’t mind you changing the APIs, but of course changing the spec will lead to compatibility problems.

I should have made myself clearer: this implementation was here to
kick around ideas and understand your design decisions. I'm not
planning to fork your spec! rest assured!

> In particular, I don’t know why you want to initialize with R.  The whole point of initializing with
> (1, R+2, 1, 0, len(S)*8, S, 0…) is to match cSHAKE.  If you aren’t going to follow cSHAKE, there’s no
> point in adding (1,0) at least, since that’s the empty NIST string, and there’s no point in multiplying
> len(S) by 8 but not multiplying R by 8.

So, I think you have a valid point here. One problem I see though, is
that cSHAKE specifies only two variants with rate = 168 bytes or rate
= 136 bytes, how would you adapt that to Strobe-128/400,
Strobe-128/800 and Strobe-256/800 which have 18, 68 and 36 bytes of
rate respectively?

One way of adapting that is to take cSHAKE's logic and extending their
spec by altering the rate value used in the padding, which is what I'm
proposing and I like that it allows you to just use a const for R
(https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L91).

> As we discussed on slack, switching 0 -> 0xFF is basically just writing
> pos_begin-1 instead of pos_begin.  Skipping pos=0 doesn’t save much

Not sure I understand what you mean by writing pos_begin-1 instead of
pos_begin. This would only happen for the value 0 which right now
doesn't mean "position 0" but "last block" instead.
It's also true that a fix is not saving much in the end :] (a byte
when you start an op on a new block)

> This is true.  That’s why the spec requires that R < 254.

I think it should be fine if R = 255 as well, the final index is R-1=
254 so pos_begin would be 255 which is still writable in a full byte.

> I dunno, I could subtract 1 from everything, but I think that 0xFF for
> “began last block” is also a little weird in an otherwise unsigned field.
> So I’m not really inclined to change it.

I'm not sure this would be a fix here (unless you meant to answer to
the previous question on changing the meaning of pos_begin = 0). I
think accepting the weirdness is fine here. I don't really see a fix.

> Technically flags=0 isn’t invalid though.

What kind of operation would it be? Wouldn't it be more sensitive to
forbid such flags/non-defined operations and set cur_flags = 0 during
initialization? This would simplify the type of the variable as well
in implementations.

> I mean, sort of?  The idea is to categorize the operations in such a
> way that the implementation is systematic.  So that’s why it’s based
> on flags.

ekr has an interesting take on this here:
https://www.ietf.org/mail-archive/web/tls/current/msg22204.html

and here is the quote:

> Most IETF specs leave the choice of how to do things up
> to the implementor. As someone who has implemented a lot of
> specs, I actually find it pretty frustrating when the author
> gets all prescriptive about implementation. If there are
> points that are unclear, I think it's fine to make them clear,
> but I'd rather stick to the PDUs.

I would agree with you though. I like biased specs (Noise do that
also) because I think they are safer to implement in the end. That
being said, I don't think it's orthogonal to having clear explanations
of what the algorithm is doing under the surface.

> Also, technically the other combinations of flags are not (currently)
> forbidden.  In particular, you can use flags=0 to waste time, even
> though you can also waste time by calling RATCHET.

I'm not sure I see the point in that. In my implementation I actually
assert operations:
https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L382:L387

> But I’ve added outer functions that call operate() with the right flags.

I think this is a good idea, see:
https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L25:L80

> Yeah.  My C implementation takes a length and a nullable array of bytes.
> I’ve tried to clarify this, but perhaps I should rewrite it as length * [0] or something.

I'm doing the same thing right now but it is sometimes not optimal,
for example RATCHETing could benefit from not creating this zero
buffer (but at the cost of complexifying the code).

See: https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L394:L401

I also have this "length" argument that I use for PRF, RATCHET and
send_MAC. You're using a `bytearray()` hack in the spec, but I think
it would be better to have a length in your API for lower level
languages :)

Talking about ideas I toyed with, I also use a meta argument in the
`Operate` function:

https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go#L389:L392

Which I think is nicer to developers

> Simplicity, code size and memory

I think the strong point of any library is also
availability/portability, your library can only be compiled with ARM
at the moment :]

> Also, if you’re cooking a custom protocol,
> STROBE is supposed to make it easier to get it right, but we need real-world
> testing (and probably better documentation) to demonstrate that.

Any project in mind :) ?

> Speed is not really an advantage.

Have you thought of using 12 rounds a la KangarooTwelve instead of 24
for the underlying Keccak function?

Thanks for all the answers! It all makes a lof of sense :)

David


On Mon, May 29, 2017 at 7:41 AM, Mike Hamburg <mi...@sh...> wrote:
> Thanks so much for looking this over, David!
>
> Here’s a rough response, and I’m also pushing changes to strobe.sourceforge.io.
>
> It’s late and on a weekend, so please correct me if I missed or misunderstood something you said, or if my response or patch on the website isn’t clear.
>
>> On May 16, 2017, at 2:33 AM, David Wong <dav...@gm...> wrote:
>>
>> Heyo all and Mike!
>>
>> First, thank-you Mike for coming-up with STROBE! It is really awesome
>> research and I've had a lot of fun reading the paper/spec and
>> implementing it :)
>
> I’m glad that at least someone found it interesting.
>
>> I have a not well tested, and very biased implementation, here:
>> https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go
>
> I still need to take a careful look at this.
>
> By the way, the line
> “”"
> if operation == "PRF" || operation != "send_MAC" || operation != “RATCHET” {
> “”"
> probably doesn’t do what you want.
>
>> It's more of a "readable" implementation than an efficient one. I've
>> taken some decisions and made the code longer in places, changed the
>> APIs and broke the spec in some other places (for example I initialize
>> with R, not with R+2).
>
> I don’t mind you changing the APIs, but of course changing the spec will lead to compatibility problems.
>
> In particular, I don’t know why you want to initialize with R.  The whole point of initializing with
> (1, R+2, 1, 0, len(S)*8, S, 0…) is to match cSHAKE.  If you aren’t going to follow cSHAKE, there’s no
> point in adding (1,0) at least, since that’s the empty NIST string, and there’s no point in multiplying
> len(S) by 8 but not multiplying R by 8.
>
>> Here is some feedback. This is a pretty long post so I apologize in
>> advance. A more "readable" version is available here:
>> https://gist.github.com/mimoo/96906d703f133b7f6aeab68c090ed59a
>>
>> ## Sponge
>>
>> I don't think a Duplex construction is a sponge construction, it does
>> not rely on a sponge either, they just share the permutation. I think
>> any mention of the word "sponge" should be removed from the
>> specification as it produces confusion. For example "sponge state"
>> could be changed to "duplex state" or better "strobe state”.
>
> Noted and changed (in most places anyway).
>
>> ## Protocol name and its length
>>
>> In section 4:
>>
>>> The parameters below define the protocol framework called "Strobe-Keccak-sec/b-v1.0.2", which is abbreviated to "Strobe-sec/b". In general, the name would be "Strobe-F-sec/b-vX.Y.Z".
>>
>> then this is the string you use to initialize the state in section 5.1:
>>
>>> "STROBEvX.Y.Z
>>
>> Why not using the specified "STROBE-Keccak-sec/b-vX.Y.Z”?
>
> I wanted to shorten the string for simplicity, and because I didn’t want to
> go over one block for initialization, even for Keccak-f[400].
>
> The goal of initialization is that you can determine what’s going on from
> the first few calls (in this case, first 1 call) to the permutation.  You can
> tell it’s Keccak-f[n] because that’s the permutation you’re calling.  You
> can tell the rate (and thus, the security level) because it’s earlier in the
> initialization string.  But you can’t tell the framework name (STROBE
> as opposed to, I dunno, BLINKER) or version.
>
> So STROBEvX.Y.Z is enough to separate it.
>
>> ## cSHAKE
>>
>> cSHAKE is mentioned many times. I think it should not. It's good to
>> mention it in the paper and to explain how the padding of STROBE was
>> created. But I think it adds confusion more than anything in the spec.
>
> I’ll try to reduce the mentions of cSHAKE.  A bunch of the padding things
> are purely “because cSHAKE does this”, but maybe it does no good to
> mention that.
>
>> Furthermore, the initialization in section 5.1. is not using the
>> correct cSHAKE formatting. It should be `bytepad(encode_string(N =
>> empty) || encode_string(S), 168)` whereas right now it is just
>> `left_encode(S) || S = "STROBEvX.Y.Z\00"`. I also see no reason to
>> count a null terminator character at the end (not everything is C :D).
>
> Yeah, oops, this was full of typos, and it was from the draft cSHAKE
> which was left_encode(S) || left_encode(N).  In the earlier version,
> left_encode(“") was "\0", not "\1\0".  It really should be “\0" anyway,
> but NIST special-cased it for some reason.
>
>> In spite of this, the initialization in section 7.2 is correct!
>>
>> ```py
>> domain = bytearray([1,self.R,1,0,1,12*8]) + bytearray("STROBEv1.0.2")
>> self._duplex(domain, forceF=True)
>> ```
>>
>> which is indeed
>>
>> ```
>> [encode length rate | encode N | N | encode STROBE string in bytes |
>> STROBEv1.0.2]
>> ```
>
> Yeah, I fixed that one when NIST updated the spec, but I missed the
> other one.
>
>> ## cSHAKE test vectors
>>
>> section 3:
>>
>>> Strobe-128/1600 and Strobe-256/1600 are based on the draft draft cSHAKE specification from NIST. If that specification changes, then the initialization and padding of Strobe might need to be adjusted. It is also possible that I have misinterpreted the cSHAKE specification, since **there are no test vectors yet**.
>>
>> [there are test vectors
>> now](http://csrc.nist.gov/groups/ST/toolkit/documents/Examples/cSHAKE_samples.pdf)!
>
> Fixed.
>
>> ## Byte Convention
>>
>> In section 2:
>>
>>> Strobe operates almost exclusively on bytes, which are elements of the set [0,...,255]. When data types larger or smaller than a byte are used, it follows a little-endian convention.
>>
>> I recommend against writing a spec for things smaller than a byte.
>> Just because it unnecessarily complicate things.
>
> Strobe is for encrypting/decrypting/whatever bytes, and not smaller or larger things.  But larger is needed for length fields, and smaller is needed for key tree.  I’ve clarified this.
>
>> ## Strobe's padding
>>
>> I wrote up what I understood of the padding from the snippets:
>> https://cryptologie.net/article/392/strobes-padding/
>>
>> One thing I find weird is that if you start an operation in a new
>> block, you will still have an `old_begin = 0` starting the block,
>> which is kind of redundant with the previous block `pos_begin` (by the
>> way, I think you should rename that to `op_begin`).
>>
>> This could be avoided by the following algorithm when starting a new operation:
>>
>> * Use `0xff` to mean "operation started in a previous block" instead of `0`.
>> * If I’m at `pos != 0`, then append `old_begin`, then start the new
>> operation (nothing changed).
>> * If I’m at `pos = 0`, do not add the `old_Begin`; add the new
>> operation with `pos_begin = 0`.
>
> As we discussed on slack, switching 0 -> 0xFF is basically just writing
> pos_begin-1 instead of pos_begin.  Skipping pos=0 doesn’t save much
> because there’s not a condition that forcibly causes that, and anyway it’s
> tricky to make that parseable.
>
>> ## Potential weird pos_begin value
>>
>> section 7.3 in the snippet:
>>
>> ```py
>> oldbegin, self.posbegin = self.posbegin, self.pos+1
>> ```
>>
>> if we start an operation at the last byte of a block (R-1),
>> `self.posbegin` will be out of the block size (=R) boundaries.
>> Example:
>>
>> current buffer:
>>
>> ```
>> [data, ...]
>> ```
>>
>> new operation starts:
>>
>> ```
>> [data, ..., old_begin, pos_begin=R, 0x04, 0, ..., 0, 0x80]
>> [flag, data, ..., pos_begin = 0, 0x04, ..., 0x80]
>> ```
>>
>> Which is parseable but weird?
>
> This is true.  That’s why the spec requires that R < 254.
>
> I dunno, I could subtract 1 from everything, but I think that 0xFF for
> “began last block” is also a little weird in an otherwise unsigned field.
> So I’m not really inclined to change it.
>
>> ## 7.2. Initialization
>>
>> ```py
>> self.cur_flags = None
>> ```
>>
>> I think this could be set to 0 instead of having a None value.
>
> Technically flags=0 isn’t invalid though.
>
>> ```py
>> self.R  = F.nbytes - sec//4
>> # ...
>> # initialization happens
>> # ...
>> self.R -= 2
>> ```
>>
>> Is it a problem if the initialization spans over multiple blocks? In
>> my implementation I'm using a constant R instead of having an extra 2
>> bytes in the initialization.
>
> Not really.  But the point of the weird initialization sequence (at least for
> /1600) is to match cSHAKE, so if you’re going to change it it might as
> well be something much simpler.
>
>> Strobe-128/400 is the smallest instance, this implies `R = N -
>> (2*sec)/8 - 2 = 400/8 - (2*128)/8 - 2 = 16`. The snippet's input is
>> `[1, strobe_R, 1, 0, 1, 12*8, "STROBEv1.0.2"]` → 18 bytes. This will
>> work with `R + 2`, but not if you use a longer string anyway.
>>
>> So I propose not having this varying R and just accept the fact that
>> the initialization might span over several blocks. (In my
>> implementation I do not use the `R+2` trick.)
>>
>>
>> ## Test Vectors?
>>
>> That would be useful to test one's implementation :]
>
> Noted.  TODO though.
>
>> ## Things that are not clear in the spec
>>
>> A lot of my critiques here is that a lot of the spec is understandable
>> by looking at the code snippets, whereas I think it should be written
>> in words. Here's a non-exhaustive list of things that bugged me:
>
> If you find other things, please ping me about them.
>
>> **operations per block?**
>>
>> It is not clear from the spec without looking at the code if the
>> permutation has to be run after each operation, or if two operations
>> can take place before one permutation does.
>
> Added, thanks.
>
>> **_duplex**
>>
>> It would be nice if every operation section (`KEY`, `CLR`, ...)
>> described exactly what it did. For example here is how you could
>> define the `KEY` section (6.1.2):
>>
>> ```
>> KEY: Provide cipher key
>> flag: AC
>> description: <!-- same one -->
>> If the operation does not start in a new block, apply the permutation.
>> After this, replace the strobe state with the key.
>> ```
>
> Added.
>
>> **customization string**
>>
>> The nice feature of STROBE is that you can have a single primitive in
>> code, and use it for your symmetric protocol, for a PRNG, as a hash
>> function, etc... But this feature is not clearly documented, the only
>> mentions of a "customization string" are through the `proto` variable
>> and the comment: "Turn on Strobe padding and do per-proto separation".
>> I think this would benefit from more explanations.
>
> I’ve added at least a little bit of explanation about this.  Let me know if
> you think it needs more.
>
>> **Streaming mode**
>>
>> Streaming is not clearly explained in my opinion. It needs an example!
>> Here's one:
>>
>> ```py
>> operate(A, "this is a long")
>> operate(A, "message", more=True)
>> ```
>>
>> is equal to
>>
>> ```py
>> operate(A, "this is a long message")
>> ```
>
> Added.
>
>>
>> **Initiator**
>>
>> From section 5:
>>
>>> A variable I0 which is 0, 1 or None. This variable marks whether the current party's role is the initiator (I0 = 0), responder (I0 = 1), or undecided (I0 = None)
>>
>> Then in section 7.3:
>>
>> ```py
>> def _beginOp(self, flags):
>>    # Adjust direction information so that sender and receiver agree
>>    if flags & T:
>>        if self.I0 is None: self.I0 = flags & I
>>        flags ^= self.I0
>> ```
>>
>> Again, I think this should be put into words. My attempt:
>>
>> The role is chosen when the first transport operation (T) is used:
>>
>> * if the message is inbound: we're the responder
>> * if the message is outbound: we're the initiator
>>
>> After that, if we're the receiver of a transport operation (T), the
>> received flags are toggled so that the responder can use the same flag
>> in the duplex call.
>
> This is also mentioned in Section 2, but I’ve added some more info about this.
>
>> **As a hash**
>>
>> What if I want to use the STROBE code as a hash function?
>> The 6.1.6: PRF section doesn't really detail that. My take:
>>
>> ```py
>> strobe = new strobe(proto="hash")
>> strobe.Operate(A, data="message to be hashed")
>> strobe.Operate(IAC, output_length)
>> ```
>
> Added
>
>> ## Code Clarity
>>
>> **Main Operation**
>>
>> ```py
>>    if (flags & (I|T) != (I|T)) and (flags & (I|A) != A):
>>        # Operation takes no input, only a length
>>        assert isinstance(data,int)
>> ```
>>
>> The use of `isinstance` does not help the implementer for low level languages.
>
> Clarified.
>
>> **cbefore & cafter in _duplex**
>>
>> In section 7.1.:
>>
>> ```py
>> for i in range(len(data)):
>>  if cbefore: data[i] ^= self.st[self.pos]
>>  self.st[self.pos]   ^= data[i]
>>  if cafter:  data[i]  = self.st[self.pos]
>> ```
>>
>> I don't think optimization should be in a spec. This is clearer:
>>
>> ```py
>> for i in range(len(data)):
>>  if cbefore:
>>    state_temp = self.st[self.pos]
>>    data[i] ^= self.st[self.pos]
>>    self.st[self.pos] = data[i] ^ state_temp
>>  if cafter:
>>    self.st[self.pos] ^= data[i]
>>    data[i] = self.st[self.pos]
>> ```
>
> I don’t see this as clearer, and I don’t see the other as an optimization.
>
> You either apply the state as a cipher (“C”) before or after absorbing
> the data, or neither.  It seems to me that this translates most naturally
> to the existing code.
>
>> **Flags**
>>
>> My understanding of the flags in the logic (not talking about using
>> them in the state): they're here to reduce lines of code. I think this
>> is a major barrier in facilitating understanding of the spec. I'd
>> prefer to see an actual explanation of the flow (instead of snippets
>> of code), and in snippets a clearer logic. What I'm doing in my
>> implementation: `switch(operation) { case "KEY": ... case ...`.
>>
>> Should the specification talk about optimizations? Or should it be the
>> role of a different document/reference implementations?
>
> I mean, sort of?  The idea is to categorize the operations in such a
> way that the implementation is systematic.  So that’s why it’s based
> on flags.
>
> Also, technically the other combinations of flags are not (currently)
> forbidden.  In particular, you can use flags=0 to waste time, even
> though you can also waste time by calling RATCHET.
>
> But I’ve added outer functions that call operate() with the right flags.
>
>> **_function()**
>>
>> Correct me if I'm wrong, but you're using underscore (`_`) prior to a
>> function name to mean that it should not be considered part of the
>> public API. If this is the reason you are using it, I think you should
>> add that to the specification, not everyone might know this :)
>
> Added.
>
>> **Python as pseudo-code**
>>
>> Just my own opinion: I like Python and the snippets of code in the
>> specification. It made it really easy to understand the spec. But I'd
>> rather have pseudo-code in the spec and a reference implementation in
>> python. This would remove a lot of the ugly `self`, or python-centric
>> functions like `isinstance()`.
>
> True.  But then I’d have to figure out what semantics I want from
> my pseudocode.  Maybe I’ll change to this later, but for now it’s easier
> to use Python.
>
>> **_duplex for PRF/MAC/...**
>>
>> ```py
>> def _duplex(self, data, cbefore=False, cafter=False, forceF=False):
>>    assert not (cbefore and cafter)
>>
>>    # Copy data, and convert string or int to bytearray
>>    # This converts an integer n to an array of n zeros
>>    data = bytearray(data)
>> ```
>>
>> This is clear if you know python, but it doesn't help you for lower
>> level languages. What needs to be done here: if the operation is
>> MAC/PRF/... sends a data = [0...0] of length data to _duplex
>>
>> In my implementation I accept an length parameter in the `Operation` function.
>
> Yeah.  My C implementation takes a length and a nullable array of bytes.
> I’ve tried to clarify this, but perhaps I should rewrite it as length * [0] or something.
>
>> ## Noise + STROBE
>>
>> Just a thought: I think it should be possible to use the STROBE object
>> to replace the symmetric object in NOISE. This would be really
>> interesting to see and could greatly reduce the complexity of the
>> noise spec :]
>>
>> Strobe, like Noise is a framework to create protocols. What would be
>> cool to see is an actual Noise-Strobe (or Noibe :D) protocol
>> specified, implemented and ready to be used :)
>
> That’s a great idea, though I don’t really want to step on Trevor’s feet.
>
>> ## Noise on embedded devices
>>
>> I'm wondering how advisable STROBE will become. What would you say are
>> the advantages of STROBE for embedded devices? The number of buffers
>> it uses? The maximum length of these? The speed? The number of
>> mallocs? The lines of codes (both for space and ease of code review)?
>> The calls to keccak that could be implemented in hardware in the
>> future?
>
> Simplicity, code size and memory.  Also, if you’re cooking a custom protocol,
> STROBE is supposed to make it easier to get it right, but we need real-world
> testing (and probably better documentation) to demonstrate that.
>
> That you could offload it to hardware is also kind of neat, but it’s a little bit
> niche for most developers since I don’t think most vendors will add hardware
> support for Keccak.
>
> Speed is not really an advantage.  On a small MCU, STROBE is probably
> slower than chacha+poly1305 or BLAKE2, and about the same speed as
> AES-CCM.  But it’s only a little slower, it’s not horrible.  Also you might find
> hardware acceleration for AES.
>
>> Regards,
>> David
>
> Thanks again,
> — Mike

[Strobe-discuss] Some feedback on the specification

[David W. <dav...@gm...>]

Heyo all and Mike!

First, thank-you Mike for coming-up with STROBE! It is really awesome
research and I've had a lot of fun reading the paper/spec and
implementing it :)

I have a not well tested, and very biased implementation, here:
https://github.com/mimoo/StrobeGo/blob/master/golang.org/x/crypto/sha3/strobe.go

It's more of a "readable" implementation than an efficient one. I've
taken some decisions and made the code longer in places, changed the
APIs and broke the spec in some other places (for example I initialize
with R, not with R+2).

Here is some feedback. This is a pretty long post so I apologize in
advance. A more "readable" version is available here:
https://gist.github.com/mimoo/96906d703f133b7f6aeab68c090ed59a

## Sponge

I don't think a Duplex construction is a sponge construction, it does
not rely on a sponge either, they just share the permutation. I think
any mention of the word "sponge" should be removed from the
specification as it produces confusion. For example "sponge state"
could be changed to "duplex state" or better "strobe state".

## Protocol name and its length

In section 4:

> The parameters below define the protocol framework called "Strobe-Keccak-sec/b-v1.0.2", which is abbreviated to "Strobe-sec/b". In general, the name would be "Strobe-F-sec/b-vX.Y.Z".

then this is the string you use to initialize the state in section 5.1:

> "STROBEvX.Y.Z

Why not using the specified "STROBE-Keccak-sec/b-vX.Y.Z"?

## cSHAKE

cSHAKE is mentioned many times. I think it should not. It's good to
mention it in the paper and to explain how the padding of STROBE was
created. But I think it adds confusion more than anything in the spec.

Furthermore, the initialization in section 5.1. is not using the
correct cSHAKE formatting. It should be `bytepad(encode_string(N =
empty) || encode_string(S), 168)` whereas right now it is just
`left_encode(S) || S = "STROBEvX.Y.Z\00"`. I also see no reason to
count a null terminator character at the end (not everything is C :D).

In spite of this, the initialization in section 7.2 is correct!

```py
domain = bytearray([1,self.R,1,0,1,12*8]) + bytearray("STROBEv1.0.2")
self._duplex(domain, forceF=True)
```

which is indeed

```
[encode length rate | encode N | N | encode STROBE string in bytes |
STROBEv1.0.2]
```

## cSHAKE test vectors

section 3:

> Strobe-128/1600 and Strobe-256/1600 are based on the draft draft cSHAKE specification from NIST. If that specification changes, then the initialization and padding of Strobe might need to be adjusted. It is also possible that I have misinterpreted the cSHAKE specification, since **there are no test vectors yet**.

[there are test vectors
now](http://csrc.nist.gov/groups/ST/toolkit/documents/Examples/cSHAKE_samples.pdf)!

## Byte Convention

In section 2:

> Strobe operates almost exclusively on bytes, which are elements of the set [0,...,255]. When data types larger or smaller than a byte are used, it follows a little-endian convention.

I recommend against writing a spec for things smaller than a byte.
Just because it unnecessarily complicate things.

## Strobe's padding

I wrote up what I understood of the padding from the snippets:
https://cryptologie.net/article/392/strobes-padding/

One thing I find weird is that if you start an operation in a new
block, you will still have an `old_begin = 0` starting the block,
which is kind of redundant with the previous block `pos_begin` (by the
way, I think you should rename that to `op_begin`).

This could be avoided by the following algorithm when starting a new operation:

* Use `0xff` to mean "operation started in a previous block" instead of `0`.
* If I’m at `pos != 0`, then append `old_begin`, then start the new
operation (nothing changed).
* If I’m at `pos = 0`, do not add the `old_Begin`; add the new
operation with `pos_begin = 0`.

## Potential weird pos_begin value

section 7.3 in the snippet:

```py
oldbegin, self.posbegin = self.posbegin, self.pos+1
```

if we start an operation at the last byte of a block (R-1),
`self.posbegin` will be out of the block size (=R) boundaries.
Example:

current buffer:

```
[data, ...]
```

new operation starts:

```
[data, ..., old_begin, pos_begin=R, 0x04, 0, ..., 0, 0x80]
[flag, data, ..., pos_begin = 0, 0x04, ..., 0x80]
```

Which is parseable but weird?

## 7.2. Initialization

```py
self.cur_flags = None
```

I think this could be set to 0 instead of having a None value.

```py
self.R  = F.nbytes - sec//4
# ...
# initialization happens
# ...
self.R -= 2
```

Is it a problem if the initialization spans over multiple blocks? In
my implementation I'm using a constant R instead of having an extra 2
bytes in the initialization.

Strobe-128/400 is the smallest instance, this implies `R = N -
(2*sec)/8 - 2 = 400/8 - (2*128)/8 - 2 = 16`. The snippet's input is
`[1, strobe_R, 1, 0, 1, 12*8, "STROBEv1.0.2"]` → 18 bytes. This will
work with `R + 2`, but not if you use a longer string anyway.

So I propose not having this varying R and just accept the fact that
the initialization might span over several blocks. (In my
implementation I do not use the `R+2` trick.)


## Test Vectors?

That would be useful to test one's implementation :]

## Things that are not clear in the spec

A lot of my critiques here is that a lot of the spec is understandable
by looking at the code snippets, whereas I think it should be written
in words. Here's a non-exhaustive list of things that bugged me:

**operations per block?**

It is not clear from the spec without looking at the code if the
permutation has to be run after each operation, or if two operations
can take place before one permutation does.

**_duplex**

It would be nice if every operation section (`KEY`, `CLR`, ...)
described exactly what it did. For example here is how you could
define the `KEY` section (6.1.2):

```
KEY: Provide cipher key
flag: AC
description: <!-- same one -->
If the operation does not start in a new block, apply the permutation.
After this, replace the strobe state with the key.
```

**customization string**

The nice feature of STROBE is that you can have a single primitive in
code, and use it for your symmetric protocol, for a PRNG, as a hash
function, etc... But this feature is not clearly documented, the only
mentions of a "customization string" are through the `proto` variable
and the comment: "Turn on Strobe padding and do per-proto separation".
I think this would benefit from more explanations.

**Streaming mode**

Streaming is not clearly explained in my opinion. It needs an example!
Here's one:

```py
operate(A, "this is a long")
operate(A, "message", more=True)
```

is equal to

```py
operate(A, "this is a long message")
```

**Initiator**

>From section 5:

> A variable I0 which is 0, 1 or None. This variable marks whether the current party's role is the initiator (I0 = 0), responder (I0 = 1), or undecided (I0 = None)

Then in section 7.3:

```py
def _beginOp(self, flags):
    # Adjust direction information so that sender and receiver agree
    if flags & T:
        if self.I0 is None: self.I0 = flags & I
        flags ^= self.I0
```

Again, I think this should be put into words. My attempt:

The role is chosen when the first transport operation (T) is used:

* if the message is inbound: we're the responder
* if the message is outbound: we're the initiator

After that, if we're the receiver of a transport operation (T), the
received flags are toggled so that the responder can use the same flag
in the duplex call.

**As a hash**

What if I want to use the STROBE code as a hash function?
The 6.1.6: PRF section doesn't really detail that. My take:

```py
strobe = new strobe(proto="hash")
strobe.Operate(A, data="message to be hashed")
strobe.Operate(IAC, output_length)
```


## Code Clarity

**Main Operation**

```py
    if (flags & (I|T) != (I|T)) and (flags & (I|A) != A):
        # Operation takes no input, only a length
        assert isinstance(data,int)
```

The use of `isinstance` does not help the implementer for low level languages.

**cbefore & cafter in _duplex**

In section 7.1.:

```py
for i in range(len(data)):
  if cbefore: data[i] ^= self.st[self.pos]
  self.st[self.pos]   ^= data[i]
  if cafter:  data[i]  = self.st[self.pos]
```

I don't think optimization should be in a spec. This is clearer:

```py
for i in range(len(data)):
  if cbefore:
    state_temp = self.st[self.pos]
    data[i] ^= self.st[self.pos]
    self.st[self.pos] = data[i] ^ state_temp
  if cafter:
    self.st[self.pos] ^= data[i]
    data[i] = self.st[self.pos]
```

**Flags**

My understanding of the flags in the logic (not talking about using
them in the state): they're here to reduce lines of code. I think this
is a major barrier in facilitating understanding of the spec. I'd
prefer to see an actual explanation of the flow (instead of snippets
of code), and in snippets a clearer logic. What I'm doing in my
implementation: `switch(operation) { case "KEY": ... case ...`.

Should the specification talk about optimizations? Or should it be the
role of a different document/reference implementations?

**_function()**

Correct me if I'm wrong, but you're using underscore (`_`) prior to a
function name to mean that it should not be considered part of the
public API. If this is the reason you are using it, I think you should
add that to the specification, not everyone might know this :)

**Python as pseudo-code**

Just my own opinion: I like Python and the snippets of code in the
specification. It made it really easy to understand the spec. But I'd
rather have pseudo-code in the spec and a reference implementation in
python. This would remove a lot of the ugly `self`, or python-centric
functions like `isinstance()`.

**_duplex for PRF/MAC/...**

```py
def _duplex(self, data, cbefore=False, cafter=False, forceF=False):
    assert not (cbefore and cafter)

    # Copy data, and convert string or int to bytearray
    # This converts an integer n to an array of n zeros
    data = bytearray(data)
```

This is clear if you know python, but it doesn't help you for lower
level languages. What needs to be done here: if the operation is
MAC/PRF/... sends a data = [0...0] of length data to _duplex

In my implementation I accept an length parameter in the `Operation` function.

## Noise + STROBE

Just a thought: I think it should be possible to use the STROBE object
to replace the symmetric object in NOISE. This would be really
interesting to see and could greatly reduce the complexity of the
noise spec :]

Strobe, like Noise is a framework to create protocols. What would be
cool to see is an actual Noise-Strobe (or Noibe :D) protocol
specified, implemented and ready to be used :)

## Noise on embedded devices

I'm wondering how advisable STROBE will become. What would you say are
the advantages of STROBE for embedded devices? The number of buffers
it uses? The maximum length of these? The speed? The number of
mallocs? The lines of codes (both for space and ease of code review)?
The calls to keccak that could be implemented in hardware in the
future?

Regards,
David

[Strobe-discuss] STROBE version 1.0.1

[Mike H. <mi...@sh...>]

Welp, that didn’t take long.  And someone is subscribed to the announcement list (welcome!).

Following a discussion with John Kelsey today at RWC, I discovered that the cSHAKE spec (NIST SP 800-185) has changed between draft and release.  This update brings STROBE into line with the released cSHAKE.  Hopefully.  I’m not actually sure, because cSHAKE doesn’t appear to have test vectors yet.

This isn’t a security-related change.  It only affects how STROBE is initialized.

Non-NIST algorithms are required to use the empty string for the NIST part of customization.  The encoding of the empty string in the draft was not well-defined, but was probably best interpreted as [0]. The revised SP 800-185 removes this ambiguity, and sets the encoding as [1,0].

Furthermore, the order of customization strings is now (N,S) instead of (S,N).

So this patch changes the initialization string to contain [1,0] instead of [0], and swaps N with S.  It also bumps the spec version to 1.0.1.

This is released in commit 4eb201b64fe3a834cc44e08c3a928d14d9ef8b41.

The strobe-announce list is moderated.  If you’d like to discuss this release, please do so on strobe-discuss.

Cheers,
— Mike