4
\$\begingroup\$

In a recent project I was to create a web service that would be consumed by another application. One of the requirements was to have a key in the service that authenticated the request as valid. But this key must never be the same and the key should be invalid soon after its use.

This is the code I wrote to that purpose.

public static bool isKeyValid(long testKey)
{
    var k = GetKey();

    var allowedVariance = 0.00000000004;
    var variance = k * allowedVariance;

    return (k + variance >= testKey && k - variance <= testKey);
}

public static long GetKey()
{
    var i = (long)DateTime.UtcNow.Ticks * 5;
    var d = (long)Math.Sqrt(Math.Ceiling(400 - (DateTime.UtcNow.Day * Math.PI)));
    var k = (long)((Math.Round((Math.Sqrt(i) * Math.Pow(Math.PI + d, 3)) * Math.E - d)) * d) + DateTime.UtcNow.Minute;
    return k;
}

The point being that the caller generates a key and the receiver generates its own and then compares these two and says everything is ok if the difference is within a given variance.

Note that nothing in the message is really secret and this is just a simple check so that the request is probably from a valid source.

I have two questions for this.

  1. How could I have done this better in terms of existing encryption libraries (in .Net) or using other things such as certificates. This must work cross platform (Windows, iOS, and Android devices)

  2. How can the given code be modified or improved to make it better for the intended purpose?

\$\endgroup\$
  • \$\begingroup\$ How did you come up with the key calculation? Have you made this up or does this algorithm have a name? \$\endgroup\$ – t3chb0t Jan 20 '17 at 18:13
  • \$\begingroup\$ I made it up. Purpose being for it is to increase rapidly during the day. And make a jump in numbers every night. \$\endgroup\$ – JensB Jan 21 '17 at 8:45
  • \$\begingroup\$ I think you might get more feedback about your algorithm on the Information Security site. \$\endgroup\$ – t3chb0t Jan 21 '17 at 11:56
  • \$\begingroup\$ Is the key type required to be long? \$\endgroup\$ – Hekku2 Feb 7 '17 at 5:52
  • \$\begingroup\$ In my algorithm the generated number is larger than an int, so yes it is. You could rewrite it to produce smaller numbers if you wanted to. \$\endgroup\$ – JensB Feb 7 '17 at 13:13
1
\$\begingroup\$

General coding style

  • avoid using var because it obscures the datatypes.
  • use meaningful variable names.
  • when comparing ranges, put the thing being tested in the middle and keep both comparison operators the same.
  • always use the same value for UtcNow, because the clock is ticking.
  • document every function you write (1 or 2 lines of comments above each function)

Example:

// Returns true if this key value came from GetKey(), false otherwise.
public static bool IsKeyValid(long testKey)
{
    long k = GetKey();

    double allowedVariance = 0.00000000004;   // say what this is ...

    // You use of 'var' obscured the differing datatypes: k and testKey are long.
    double variance = k * allowedVariance;    

    return (k - variance <= testKey && testKey <= k + variance);
}

// Returns a key based on the current time.
public static long GetKey()
{
    DateTime now = DateTime.UtcNow;

    // Explanation is called for here...
    long i = now.Ticks * 5;
    long d = (long)Math.Sqrt(Math.Ceiling(400 - (now.Day * Math.PI)));
    long k = (long)((Math.Round((Math.Sqrt(i) * Math.Pow(Math.PI + d, 3)) * Math.E - d)) * d) + now.Minute;

    return k;
}

I know nothing about encryption so cannot say whether there are built-in .NET functions which perform the same task, but there probably are.

\$\endgroup\$
  • 1
    \$\begingroup\$ avoid using var because it obscures the datatypes var everywhere! ;-) \$\endgroup\$ – t3chb0t Jan 20 '17 at 15:17
  • 1
    \$\begingroup\$ The one nice thing about var is it helps you find accidental implicit type conversions. double value = 1.0f; won't tell you that you're implicitly converting a float to a double, but if you do var value = 1.0f; and inspect the type you can see that it's a float much more easily. \$\endgroup\$ – 410_Gone Jan 20 '17 at 16:20
  • \$\begingroup\$ Why would you care that you are implicitly converting a compile-time constant float to a double? @ebrown \$\endgroup\$ – Cody Gray Jan 20 '17 at 18:33
  • \$\begingroup\$ @CodyGray That's an example, ignore the compile-time issue. Consider: double result = SomeMethodReturnsFloat(); vs var result = SomeMethodReturnsFloat();. \$\endgroup\$ – 410_Gone Jan 20 '17 at 18:34
  • \$\begingroup\$ Yes, that's a much better example. Still, it's a widening conversion, so it's guaranteed to be safe. I remain unconvinced that this is something I need to be concerned about. @ebrown \$\endgroup\$ – Cody Gray Jan 20 '17 at 18:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.