Add deriving of Ord instances for private and public RSA and DSA keys types

[ikkeps]

Key types had no Ord instance deriving so I can not use it, for example, as Map keys. I suggest that add standalone deriving in my project is bad idea.

[TomMD]

I'm curious what the motivation is for this patch. Why would you want cryptographic keys as the key to a Map structure?

[ikkeps]

For efficient lookup. I have many cryptographic keys in memory and when new added or read form database (it happens quite often) I need to find information related to it. So I use list + find, and have, actually, generalization as second problem, because many other types have Ord instances, so i can generalize logic to rely only on it. Also, I suggested that adding deriving of Ord instance here does not makes any problems.

[TomMD]

For efficient lookup.

That is why you use a Map, sure. What confuses me is using a cryptographic key as the index. I just haven't came across such a situation. Generally you have an SPI or other connection/resource information that gets you the key and any metadata.

Also, I suggested that adding deriving of Ord instance here does not makes any problems.

That's a questionable assertion. If someone were to use a variable time comparison then this could cause a timing issue when used incorrectly.

[vincenthz]

yep, i agree with @TomMD. i've been mulling the same question relative to Ord instance. I've been thinking long about those Eq instances too, I'm not sure they are appropriate when derived automatically.

[superbobry]

@TomMD, can you please elaborate a bit on the timing issue with Ord instances?

[ikkeps]

Generally you have an SPI or other connection/resource information that gets you the key and any metadata.

Generally, yes. But I have thousands of users, whose keys stored separately from other data, so need to "join" it in runtime. By using keys, well, as keys. Yes, this approach is bad, but, saldy, I have to deal with it, and didn't knew that Ord instance deriving can cause big problems and, if I knew it did, I would not make this merge request, since situation is kinda specific.

I've been thinking long about those Eq instances too, I'm not sure they are appropriate when derived automatically.

Is there any speed issues with automatic deriving? Or it cause some kind of architectural problems?

[TomMD]

@ikkeps It is because Eq instances suffer the same timing issue as an Ord instance might suffer. This is why the constTimeEq function was added to crypto-api.

@superbobry Timing attacks are a class of problem in crypto systems that revolves around any routine that takes a different amount of time depending on some critical value (a key or even the plaintext). In this case, comparing two integers for equality will call GMP (on most Haskell compiler setups) which first verifies the two integer vectors are equal length and (conditional on that first check) verifies the contained bytes are equal. It is trivial to show the vulnerability using criterion and benchmarks of bench "somebench" $ whnf (== randomVal) guessNumberX.

[knsd]

@TomMD, is it safe to use something like yours safeCompare? Why did you remove it?

[knsd]

Bump.

Is mentioned function safe enough to use it as compare in Ord instance? And similar == function for Eq instance.

[vincenthz]

I'm still unsured on what to do here. I haven't took the time to think about it properly, but as a brain dump:

• There's nothing inherently unsafe about deriving eq/ord, provided it's not used in an unsafe context. at the moment it's the responsability of the user to make sure he's using those functions properly.
• Do we provide safe operations by default so that it's less likely to shoot yourself by default using the normal Eq/Ord functions, or we keep it the responsability of the user with maybe adding big fat warnings somewhere.
• Does that make sense to provide Ord instance on keys at all, if yes does Ord should be derived automatically, or using only part of the keys (as some parts are just by-products that we keep to go faster). Also haven't look, but it's probably a good idea to see what Ord code is derived in a multi fields value.
• Need to develop a timing test suite, it's on my todo list.. eventually.

[TomMD]

@knsd I really don't know why safeCompare went away. Nothing in my memory or inbox gives me sufficient clues. I do see that Vincent rightly objected to the name. safeCompare might come back as constTimeCompare if you would find it useful.

[knsd]

I'm currently working on comparaptor library that provides type classes SafeEq and SafeOrd and instances for Integer and ByteString. Will be very grateful for review.

@vincenthz, why are you objected to the safe? I think that constTime is not right name because time is differ for data with different size.

[vincenthz]

i object to the 'safe' because safe/unsafe is already use in haskell to mean something else. Also with some usage, it's OK and absolutely safe to compare a secret without constant time. it's not inherently unsafe. i still think constTime is better, because with correct usage (comparing data of 2 differents size shouldn't be correct in this context) will yield a constant time function independant of the data

[vincenthz]

archiving repository