Skip to content

Increase TOTP secret length from 80 bits to 160 bits#5604

Merged
jmhooper merged 3 commits intomainfrom
jmhooper-totp-secret-length
Nov 12, 2021
Merged

Increase TOTP secret length from 80 bits to 160 bits#5604
jmhooper merged 3 commits intomainfrom
jmhooper-totp-secret-length

Conversation

@jmhooper
Copy link
Contributor

Why: RFC 4226 specifies that the length should meet or exceed 128 bits.

@jmhooper jmhooper merged commit 1fdeb2c into main Nov 12, 2021
@jmhooper jmhooper deleted the jmhooper-totp-secret-length branch November 12, 2021 15:44
@@ -287,7 +287,7 @@
it 'generates a secret 16 characters long' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this spec title is now wrong

Suggested change
it 'generates a secret 16 characters long' do
it 'generates a secret 32 characters long' do


def generate_totp_secret
ROTP::Base32.random_base32(16)
ROTP::Base32.random(20)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if we left a comment here describing the # of bits?#5604

Suggested change
ROTP::Base32.random(20)
ROTP::Base32.random(20) # 160-bit secret, per RFC 4226

@aduth
Copy link
Contributor

aduth commented Nov 18, 2021

I think this might have caused some visual oddity on the auth app setup screen?

image

@aduth
Copy link
Contributor

aduth commented Dec 3, 2021

I think this might have caused some visual oddity on the auth app setup screen?

For posterity: Ticket filed at LG-5436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants