Skip to content

Commit

Permalink
Improved TOTP implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
fdurand committed Jan 25, 2025
1 parent f9d5183 commit 7e028aa
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 12 deletions.
15 changes: 12 additions & 3 deletions html/captive-portal/templates/mfa/TOTP.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,25 @@
<form action="/mfa" method="post">
<div class="input-container">
<label for="otp">[% i18n("Authentication code") %]</label>
<input id="otp" name="otp" title="OTP" type="tel" inputmode="numeric" pattern="[0-9]*" minlength="4" maxlength="6" required autocorrect="off" autocapitalize="off" class="text-center letter-expand" />
<input id="otp" name="otp" title="OTP" type="tel" inputmode="numeric" pattern="[0-9]*" minlength="1" maxlength="10" required autocorrect="off" autocapitalize="off" class="text-center letter-expand" />
</div>
<button type="submit" name="submit" class="c-btn c-btn--primary u-1/1 u-margin-top" disabled>
[% i18n("Continue") %]
</button>
</form>
[% ELSE %]
<p>[% i18n("Please scan the code below in your favorite authenticator app") %]</p>
<div id="qrcode" data-username="[% username %]" data-otp="[% otp %]"></div>
[% END %]
<div id="qrcode" data-username="[% username %]" data-otp="[% otp %]" data-digits="[% token_size %]" data-period="[% period %]" data-suffix="[% suffix %]"></div>
<form action="/mfa" method="post">
<div class="input-container">
<label for="otp">[% i18n("Verify authentication code") %]</label>
<input id="otp" name="otp" title="OTP" type="tel" inputmode="numeric" pattern="[0-9]*" minlength="1" maxlength="10" required autocorrect="off" autocapitalize="off" class="text-center letter-expand" />
</div>
<button type="submit" name="submit" class="c-btn c-btn--primary u-1/1 u-margin-top" disabled>
[% i18n("Continue") %]
</button>
</form>
[% END %]
</div>
</div>
[% IF exist %]
Expand Down
5 changes: 4 additions & 1 deletion html/common/qrcode.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@ document.addEventListener('DOMContentLoaded', function () {
const qrcode = document.getElementById('qrcode');
const otp = qrcode.getAttribute('data-otp');
const username = qrcode.getAttribute('data-username');
new QRCode(qrcode, `otpauth://totp/${username}.packetfence?secret=${otp}`);
const digits = qrcode.getAttribute('data-digits');
const period = qrcode.getAttribute('data-period');
const suffix = qrcode.getAttribute('data-suffix');
new QRCode(qrcode, `otpauth://totp/${username}.${suffix}?secret=${otp}&digits=${digits}&period=${period}`);
});
29 changes: 28 additions & 1 deletion html/pfappserver/lib/pfappserver/Form/Config/Mfa/TOTP.pm
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,36 @@ has_field 'radius_mfa_method' =>
default => 'strip-otp',
);

has_field 'period' =>
(
type => 'Duration',
default => {
interval => 30,
unit => 's',
},
);

has_field 'token_size' =>
(
type => 'IntRange',
label => 'Token size',
required => 1,
default => 6,
range_start => 1,
range_end => 10,
);

has_field 'suffix' =>
(
type => 'Text',
required => 1,
default => 'packetfence',
messages => { required => 'Please specify the suffix that will be added to the username' },
);

has_block definition =>
(
render_list => [ qw(id radius_mfa_method) ],
render_list => [ qw(id radius_mfa_method period token_size suffix) ],
);

=over
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
:text="$i18n.t('Define the method to be used in RADIUS to trigger OTP.')"
/>

<form-group-suffix namespace="suffix"
:column-label="$i18n.t('Suffix added to the username')"
:text="$i18n.t('The suffix added to the username. Thiw will appear in the MFA application (username.suffix).')"
/>

<form-group-split-char namespace="split_char"
:column-label="$i18n.t('Character separator')"
:text="$i18n.t('The char with which the password and the code are split during RADIUS authentication.')"
Expand All @@ -25,6 +30,16 @@
:text="$i18n.t('The duration that is used to cache the MFA information. This should approximately represent the time for the user to complete the authentication.')"
/>

<form-group-token-size namespace="token_size"
:column-label="$i18n.t('Token size')"
:text="$i18n.t('Define the number of digits of the token. (default is 6)')"
/>

<form-group-period namespace="period"
:column-label="$i18n.t('Token rotation period')"
:text="$i18n.t('Define the period to rotate the token. (default is 30s)')"
/>

<form-group-post-mfa-validation-cache-duration namespace="PostMfaValidationCacheDuration"
:column-label="$i18n.t('Post MFA Validation Cache Duration')"
:text="$i18n.t('The duration time to keep the information the user did validate the MFA authentication (represent the time between the portal validation and the next RADIUS request).')"
Expand All @@ -37,7 +52,10 @@ import { BaseForm } from '@/components/new/'
import {
FormGroupIdentifier,
FormGroupRadiusMfaMethod,
FormGroupSuffix,
FormGroupSplitChar,
FormGroupTokenSize,
FormGroupPeriod,
FormGroupCacheDuration,
FormGroupPostMfaValidationCacheDuration,
} from './'
Expand All @@ -47,7 +65,10 @@ const components = {
FormGroupIdentifier,
FormGroupRadiusMfaMethod,
FormGroupSuffix,
FormGroupSplitChar,
FormGroupTokenSize,
FormGroupPeriod,
FormGroupCacheDuration,
FormGroupPostMfaValidationCacheDuration
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export {
BaseFormGroupInput as FormGroupHost,
BaseFormGroupInput as FormGroupCallbackUrl,
BaseFormGroupInput as FormGroupSplitChar,
BaseFormGroupInput as FormGroupSuffix,
BaseFormGroupChosenOne as FormGroupTokenSize,
BaseFormGroupIntervalUnit as FormGroupPeriod,
BaseFormGroupIntervalUnit as FormGroupCacheDuration,
BaseFormGroupIntervalUnit as FormGroupPostMfaValidationCacheDuration,

Expand Down
43 changes: 36 additions & 7 deletions lib/pf/mfa/TOTP.pm
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,31 @@ Character that split the username and otp

has split_char => (is => 'rw' );

=head2 token_size
Size of the token
=cut

has token_size => (is => 'rw' );

=head2 period
Period of time to generate the token
=cut

has period => (is => 'rw' );


=head2 suffix
Suffix added to the username
=cut

has suffix => (is => 'rw' );

sub module_description { 'Generic TOTP MFA' }

=head2 check_user
Expand All @@ -52,7 +77,8 @@ sub check_user {
my $logger = get_logger();
my $message;
if ($self->radius_mfa_method eq 'strip-otp' || $self->radius_mfa_method eq 'second-password') {
if ($otp =~ /^\d{6,6}$/) {
my $token_size = self->token_size;
if ($otp =~ /^\d{$token_size,$token_size}$/) {
return $self->verify_otp($username, $otp);
} else {
$message = "Method not supported for user $username";
Expand Down Expand Up @@ -87,18 +113,18 @@ sub verify_otp {
sub generateCurrentNumber {
my ($self, $otp) = @_;

my $paddedTime = sprintf("%016x", int(time() / 30));
my $paddedTime = sprintf("%016x", int(time() / normalize_time($self->period)));
my $data = pack('H*', $paddedTime);
my $key = $self->decodeBase32($otp);

my $hmac = hmac_sha1_hex($data, $key);

my $offset = hex(substr($hmac, -1));
my $encrypted = hex(substr($hmac, $offset * 2, 8)) & 0x7fffffff;

my $token = $encrypted % 1000000;
return sprintf("%06d", $token);

my $div = '1'. '0' x $self->token_size;
my $token = $encrypted % $div;
my $token_size = '%0' . $self->token_size . 'd';
return sprintf($token_size, $token);
}

sub decodeBase32 {
Expand Down Expand Up @@ -130,7 +156,10 @@ sub redirect_info {
return {
exist => $exist,
username => $username,
otp => $otp
otp => $otp,
period => normalize_time($self->period),
token_size => $self->token_size,
suffix => $self->suffix,
};
}

Expand Down

0 comments on commit 7e028aa

Please sign in to comment.