Correct Base32 implementation to exactly match Google Authenticator

- Correctly pad extra bits
- Update tests
- Fixes #86

Author: mdp

lib/rotp/base32.rb

@@ -1,51 +1,67 @@
 module ROTP
   class Base32
     class Base32Error < RuntimeError; end
-    CHARS = 'abcdefghijklmnopqrstuvwxyz234567'.each_char.to_a
+    CHARS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'.each_char.to_a
+    SHIFT = 5
+    MASK = 31
 
     class << self
+
       def decode(str)
-        str = str.tr('=', '')
-        output = []
-        str.scan(/.{1,8}/).each do |block|
-          char_array = decode_block(block).map(&:chr)
-          output << char_array
+        buffer = 0
+        idx = 0
+        bits_left = 0
+        str = str.tr('=', '').upcase
+        result = []
+        str.split('').each do |char|
+          buffer = buffer << SHIFT
+          buffer = buffer | (decode_quint(char) & MASK)
+          bits_left = bits_left + SHIFT
+          if bits_left >= 8
+            result[idx] = (buffer >> (bits_left - 8)) & 255
+            idx = idx + 1
+            bits_left = bits_left - 8
+          end
         end
-        output.join
+        result.pack('c*')
       end
 
-      def random_base32(length = 32)
-        b32 = ''
-        SecureRandom.random_bytes(length).each_byte do |b|
-          b32 << CHARS[b % 32]
+      def encode(b)
+        data = b.unpack('c*')
+        out = ''
+        buffer = data[0]
+        idx = 1
+        bits_left = 8
+        while bits_left > 0 || idx < data.length
+          if bits_left < SHIFT
+            if idx < data.length
+              buffer = buffer << 8
+              buffer = buffer | (data[idx] & 255)
+              bits_left = bits_left + 8
+              idx = idx + 1
+            else
+              pad = SHIFT - bits_left
+              buffer = buffer << pad
+              bits_left = bits_left + pad
+            end
+          end
+          val = MASK & (buffer >> (bits_left - SHIFT))
+          bits_left = bits_left - SHIFT
+          out.concat(CHARS[val])
         end
-        b32
+        return out
       end
 
-      private
-
-      def decode_block(block)
-        length = block.scan(/[^=]/).length
-        quints = block.each_char.map { |c| decode_quint(c) }
-        bytes = []
-        bytes[0] = (quints[0] << 3) + (quints[1] ? quints[1] >> 2 : 0)
-        return bytes if length < 3
-
-        bytes[1] = ((quints[1] & 3) << 6) + (quints[2] << 1) + (quints[3] ? quints[3] >> 4 : 0)
-        return bytes if length < 4
-
-        bytes[2] = ((quints[3] & 15) << 4) + (quints[4] ? quints[4] >> 1 : 0)
-        return bytes if length < 6
-
-        bytes[3] = ((quints[4] & 1) << 7) + (quints[5] << 2) + (quints[6] ? quints[6] >> 3 : 0)
-        return bytes if length < 7
-
-        bytes[4] = ((quints[6] & 7) << 5) + (quints[7] || 0)
-        bytes
+      # Defaults to 160 bit long secret (meaning a 32 character long base32 secret)
+      def random(byte_length = 20)
+       rand_bytes = SecureRandom.random_bytes(byte_length)
+       self.encode(rand_bytes)
       end
 
+      private
+
       def decode_quint(q)
-        CHARS.index(q.downcase) || raise(Base32Error, "Invalid Base32 Character - '#{q}'")
+        CHARS.index(q) || raise(Base32Error, "Invalid Base32 Character - '#{q}'")
       end
     end
   end

lib/rotp/cli.rb

@@ -19,7 +19,7 @@ def errors
       if %i[time hmac].include?(options.mode)
         if options.secret.to_s == ''
           red 'You must also specify a --secret. Try --help for help.'
-        elsif options.secret.to_s.chars.any? { |c| ROTP::Base32::CHARS.index(c.downcase).nil? }
+        elsif options.secret.to_s.chars.any? { |c| ROTP::Base32::CHARS.index(c.upcase).nil? }
           red 'Secret must be in RFC4648 Base32 format - http://en.wikipedia.org/wiki/Base32#RFC_4648_Base32_alphabet'
         end
       end

spec/lib/rotp/base32_spec.rb

@@ -1,24 +1,25 @@
 require 'spec_helper'
 
 RSpec.describe ROTP::Base32 do
-  describe '.random_base32' do
+  describe '.random' do
     context 'without arguments' do
-      let(:base32) { ROTP::Base32.random_base32 }
+      let(:base32) { ROTP::Base32.random }
 
-      it 'is 32 characters long' do
+      it 'is 20 bytes (160 bits) long (resulting in a 32 character base32 code)' do
+        expect(ROTP::Base32.decode(base32).length).to eq 20
         expect(base32.length).to eq 32
       end
 
       it 'is base32 charset' do
-        expect(base32).to match(/\A[a-z2-7]+\z/)
+        expect(base32).to match(/\A[A-Z2-7]+\z/)
       end
     end
 
     context 'with arguments' do
-      let(:base32) { ROTP::Base32.random_base32 32 }
+      let(:base32) { ROTP::Base32.random 48 }
 
-      it 'allows a specific length' do
-        expect(base32.length).to eq 32
+      it 'returns the appropriate byte length code' do
+        expect(ROTP::Base32.decode(base32).length).to eq 48
       end
     end
   end
@@ -33,22 +34,40 @@
 
     context 'valid input data' do
       it 'correctly decodes a string' do
-        expect(ROTP::Base32.decode('F').unpack('H*').first).to eq '28'
-        expect(ROTP::Base32.decode('23').unpack('H*').first).to eq 'd6'
-        expect(ROTP::Base32.decode('234').unpack('H*').first).to eq 'd6f8'
-        expect(ROTP::Base32.decode('234A').unpack('H*').first).to eq 'd6f800'
-        expect(ROTP::Base32.decode('234B').unpack('H*').first).to eq 'd6f810'
-        expect(ROTP::Base32.decode('234BCD').unpack('H*').first).to eq 'd6f8110c'
-        expect(ROTP::Base32.decode('234BCDE').unpack('H*').first).to eq 'd6f8110c80'
-        expect(ROTP::Base32.decode('234BCDEFG').unpack('H*').first).to eq 'd6f8110c8530'
-        expect(ROTP::Base32.decode('234BCDEFG234BCDEFG').unpack('H*').first).to eq 'd6f8110c8536b7c0886429'
+        expect(ROTP::Base32.decode('2EB7C66WC5TSO').unpack('H*').first).to eq 'd103f17bd6176727'
+        expect(ROTP::Base32.decode('Y6Y5ZCAC7NABCHSJ').unpack('H*').first).to eq 'c7b1dc8802fb40111e49'
+      end
+
+      it 'correctly decode strings with trailing bits (not a multiple of 8)' do
+        # Dropbox style 26 characters (26*5==130 bits or 16.25 bytes, but chopped to 128)
+        # Matches the behavior of Google Authenticator, drops extra 2 empty bits
+        expect(ROTP::Base32.decode('YVT6Z2XF4BQJNBMTD7M6QBQCEM').unpack('H*').first).to eq 'c567eceae5e0609685931fd9e8060223'
+
+        # For completeness, test all the possibilities allowed by Google Authenticator
+        # Drop the incomplete empty extra 4 bits (28*5==140bits or 17.5 bytes, chopped to 136 bits)
+        expect(ROTP::Base32.decode('5GGZQB3WN6LD7V3L5HPDYTQUANEQ').unpack('H*').first).to eq 'e98d9807766f963fd76be9de3c4e140349'
+
       end
 
       context 'with padding' do
         it 'correctly decodes a string' do
-          expect(ROTP::Base32.decode('F==').unpack('H*').first).to eq '28'
+          expect(ROTP::Base32.decode('234A===').unpack('H*').first).to eq 'd6f8'
         end
       end
+
     end
   end
+
+  describe '.encode' do
+    context 'encode input data' do
+      it 'correctly encodes data' do
+        expect(ROTP::Base32.encode(hex_to_bin('3c204da94294ff82103ee34e96f74b48'))).to eq 'HQQE3KKCST7YEEB64NHJN52LJA'
+      end
+    end
+  end
+
+end
+
+def hex_to_bin(s)
+  s.scan(/../).map { |x| x.hex }.pack('c*')
 end