mdp/rotp

Counter based OTP's example don't work

artmotion opened this issue · 4 comments

I'm tried the "Counter based OTP's" example on Ruby 2.4.1 and got vastly different results. I just added a puts in front of each line, so that I can see the output:

require 'rubygems'
require 'rotp'

hotp = ROTP::HOTP.new("base32secretkey3232")
puts hotp.at(0) # => "260182"
puts hotp.at(1) # => "055283"
puts hotp.at(1401) # => "316439"

# OTP verified with a counter
puts hotp.verify("316439", 1401) # => true
puts hotp.verify("316439", 1402) # => false

Output:

786922
595254
259769
false
false

For me, I get "259769" for hotp.at(1401).
Maybe you want to update your example.

Tested this on:
Ruby version: ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin16]
ropt: 3.3.0

I got the same values as yours, also using the same ruby and rotp versions.

The example is just showing what should be the result, the actual values aren't really important. Perhaps stating that

a = hotp.at(1401)
hotp.verify(a, 1401) == true
hotp.verify(a, 1402) == false

Is what you expect?

I tested this on Ruby 1.9.3, 2.3.4 and 2.4.1. They all gave the same values, so its not a Ruby problem.

That being said, the values do matter, as it is necessary for compliance with other implementations of RFC 4226. I wrote a small script to run against the test values from the spec, and they match. So if you are in doubt, you can run this:

# -*- coding: utf-8 -*-
require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'rotp'
  gem 'base32'
end

# Check if the library maches the RFC4226 test values
# https://www.ietf.org/rfc/rfc4226.txt
hotp = ROTP::HOTP.new(Base32.encode('12345678901234567890'))

puts(format('Test values matches: %s',
              case
              when hotp.at(0) != '755224' then false
              when hotp.at(1) != '287082' then false
              when hotp.at(2) != '359152' then false
              when hotp.at(5) != '254676' then false
              when hotp.at(9) != '520489' then false
              else true
              end))

Note: The spec compatibility is also found in the tests of this project: https://github.com/mdp/rotp/blob/master/spec/lib/rotp/hotp_spec.rb#L25

mdp commented

Yeah, so regardless, it's confusing. I think because the TOTP code is relative to the time it's run and not expected to match that these followed the same rules. But obviously it's a bit confusing to see code that doesn't work as documented. Updated the the real values. Thanks

mdp commented

Merged. Thanks