[botan-devel] newbie observations and questions
lloyd at randombit.net
Mon Apr 23 09:47:55 EDT 2012
On Sat, Apr 21, 2012 at 11:30:33PM -0700, Patrick Pelletier wrote:
> I noticed that ARC4::name() and Core_Engine::find_stream_cipher() use
> different naming conventions for ARC4. find_stream_cipher recognizes
> "ARC4" with an argument, or "RC4_drop" as shorthand for 768. On the
> other hand, ARC4::name returns "ARC4" if skip is 0, "MARK-4" if skip
> is 256, or "RC4_skip" with an argument for any other skip value. Why
> the discrepancy?
This is a historical mess. The 'ARC4' is because initially (circa
2000) I was concerned that RSA might attempt to enforce their
trademark of RC4, though I have never heard of this occuring so
clearly it's not much of an issue. MARK-4 comes from SCAN , which
is (or was - it hasn't been updated since 2002) a registry of name ->
algorithm mappings originally designed for Java crypto, but given that
I was implementing much the same idea it seemed reasonable to make the
names the same. I thought RC4_skip also came from SCAN, but apparently
it uses RC4-drop for this. And RC4_drop is also specified but read-only,
for RC4_skip(768) (which is the default value of SCAN's RC4-drop!).
Doing it over again, RC4 would be called RC4 and any leading keystream
skipping would be left to the application.
> I might be missing something, but it looks to me like XSalsa20
> requires setting the key again each time before setting a new IV.
> (Unlike regular Salsa20, where the key can be set once, and then the
> IV can be set multiple times.) This is because Salsa20::set_iv()
> overwrites state[1..4] and state[11..14] (which contain the key) in
> the XSalsa20 case.
I'll have to check on this to be sure, my memory of the XSalsa20 spec
is very fuzzy, but I think set_iv is assuming you've already called
set_key, thus the key is set up in this->state, then the hsalsa20
transform is called on the zero values plus the existing key state.
So the final XSalsa20 state is a keyed transform of the first 128 bits
of the IV.
I'll run some tests to make sure I didn't mess anything up though.
> In checks/bench.cpp, I noticed that REVERSE_ITERATOR_BUG is defined,
> but not used.
> A few typos:
> checks/validate.cpp: "expecing" -> "expecting"
> src/libstate/init.cpp: "so for" -> "so forth"
> src/engine/openssl/ossl_arc4.cpp: "suported" -> "supported"
> "reqeust" should probably be "request" (appears twice in
> core_engine.h, and once each in asm_engine.h, simd_engine.h, and
All fixed, thank you.
> In Library_State::initialize(), the case for BOTAN_HAS_MUTEX_QT looks
> suspicious, because it looks like a declaration ("mutex_factory
> Qt_Mutex_Factory;"), rather than an assignment like the other cases
> ("mutex_factory = new Pthread_Mutex_Factory;").
Suspicious indeed. In fact, fixing the definition problem and
attempting to compile the module, it turns out that the include and
macro check are both wrong for Qt4. I think it's safe to say that this
module has not been used by anyone for a long time. Not wanting to
let a good opportunity to kill dead code go to waste, I'm going to
remove the Qt mutex support entirely.
(I'm really looking forward to being able to just used std::mutex
> I believe that the lines in validate.dat of the form "[MyCipher-123
> <EXTENSION>]" actually never do anything, because the code only
> removes "<EXTENSION>" from the end and then tests "MyCipher-123
> " (note the space at the end) which does not exist. (But since it's
> an extension and isn't required to be present, the test doesn't fail.
> It just means the test is never checked, even if "MyCipher-123" is
> present.) Either validate.dat needs to have the space removed before
> "<EXTENSION>", or else the code needs to be changed to remove the
> space after "MyCipher-123" when it removes "<EXTENSION>". (I believe
> the only current uses of <EXTENSION> are for zlib and bzip2, but I ran
> into this when adding tests for Camellia, since in 1.10.1, Camellia
> wasn't implemented in the core, so I had to make the tests optional.)
> Even after working around the space issue mentioned above, it isn't
> possible to use <EXTENSION> in CMAC tests, because this happens:
> Testing MACs: ...........Exception: Could not find any algorithm named
> ERROR: "CMAC(Camellia-128)" failed test #1
> This is because Algorithm_Factory::make_block_cipher throws
> Algorithm_Not_Found, and is called by Core_Engine::find_mac. find_mac
> should catch the exception and return NULL. Or, if this is really
> desired, then the testing code should catch the exception and allow it
> when "<EXTENSION>" is present.
I concur, this behavior is better.
I don't have time to look at this one right now but both of these are
tracked in http://bugs.randombit.net/show_bug.cgi?id=193
> In eax_test.cpp, there was a comment that LibTomCrypt's lone Noekeon
> test vector does not match Botan. I tracked this down, and there are
> actually two reasons for this:
> 1) LTC uses direct Noekeon (no key schedule), while Botan uses
> indirect Noekeon (key scheduled by encrypting with Noekeon). See page
> 14 of http://gro.noekeon.org/Noekeon-slides.pdf
Ah well that would explain it. I'm surprised I missed this when I
checked LTC's version.
> 2) LTC has a bug, where its PI1 and PI2 functions do not match the
> Noekeon specification.
> I wrote some code which can produce Noekeon EAX test vectors suitable
> for Botan, using BouncyCastle:
Thank you for tracking this down, and for the test cases!
http://bugs.randombit.net/show_bug.cgi?id=194 so I remember to
integrate this in.
> Also, I've implemented an "engine" for Nettle, analogous to the
> existing one for OpenSSL. Is this the sort of thing you'd like to
> have contributed to the Botan codebase, or is it more appropriate for
> it to be a separate, add-on project?
I'll have to think about this one a bit. The original reason for the
OpenSSL module was as a proof of concept for hardware support (and at
one point RSA hardware by one company was supported, but they went out
of business and the drivers went rotten and I ended up giving the card
to someone who wanted to hack OpenBSD support). The goal of the engine
code wasn't really to support an OpenSSL wrapper. On the other hand,
that's pretty much what it is doing currently.
> The existing OpenSSL engine has three ciphers for Camellia:
> "Camellia-128", "Camellia-192", and "Camellia-256", analogous to the
> three AES ones. I wrote my Nettle engine using this scheme, to match
> the OpenSSL engine, which is the only Camellia implementation in Botan
> 1.10.1. But, after I pulled down the Monotone head tonight, I saw
> that Botan has its own Camellia implementation now, but it is
> implemented as a single cipher, "Camellia", with variable key size,
> much like how Serpent and Twofish were already done. Any thoughts on
> which approach is better?
This is a little embarrassing to admit, but I had forgotten entirely
that I had added Camellia hooks in the OpenSSL code.
Being able to use the OpenSSL Camellia EVP interface seems useful
(especially as the OpenSSL version is much faster than what's in the
library itself right now, I just hacked that together so I could add
support for Camellia in TLS), which would suggest I should redo my
version to use split -128/-192/-256 types as well.
> Nettle is the fastest for some algorithms, but not for others:
> AES-128 [nettle] 147.22 [openssl] 123.83 [core] 108.59
> AES-192 [nettle] 125.06 [openssl] 106.83 [core] 95.84
> AES-256 [nettle] 105.04 [openssl] 93.90 [core] 85.72
a) Nettle's AES is pretty good!
b) Botan's AES is pretty bad! :(
> Serpent [nettle] 84.75 [simd] 84.69 [core] 55.74
That's interesting! Either Nettle is also using SSE2 for Serpent or it
has some other tricks I should know about.
> ARC4 [openssl] 183.33 [nettle] 164.45 [core] 90.98
Again suggesting the core lib could be doing a lot better.
Thank you very much for your careful review and comments. I'll try to
address at least the Camellia and extension testing issues this week.
Probably getting around time for a new release in any case.
More information about the botan-devel