From 89673b58eb3e8c15d16353f0c49886e7612360be Mon Sep 17 00:00:00 2001 From: Dag Heyman Date: Tue, 7 Apr 2020 11:38:54 +0200 Subject: [PATCH] drop support for OTP mode this commit drops support for OTP mode, where TOTP secrets could be stored on the YubiKey configuration slots --- py/yubikey.py | 88 ++++--------------------------------- qml/CredentialCard.qml | 39 ++--------------- qml/NewCredentialView.qml | 68 ++++------------------------- qml/NoYubiKeySection.qml | 6 +-- qml/SettingsView.qml | 91 +++------------------------------------ qml/StyledToolBar.qml | 2 +- qml/YubiKey.qml | 44 ++++--------------- qml/main.qml | 24 +---------- 8 files changed, 40 insertions(+), 322 deletions(-) diff --git a/py/yubikey.py b/py/yubikey.py index c9fd4944..26953405 100644 --- a/py/yubikey.py +++ b/py/yubikey.py @@ -128,11 +128,8 @@ def catch_error(f): return wrapped -def usb_selectable(dev, otp_mode): - if otp_mode: - return dev.mode.has_transport(TRANSPORT.OTP) - else: - return dev.mode.has_transport(TRANSPORT.CCID) and ( +def usb_selectable(dev): + return dev.mode.has_transport(TRANSPORT.CCID) and ( dev.config.usb_enabled & APPLICATION.OATH) @@ -235,7 +232,7 @@ class Controller(object): return YubiKey(Descriptor.from_driver(drv), drv) return None - def _get_devices(self, otp_mode=False): + def _get_devices(self): res = [] descs_to_match = self._descs[:] handled_serials = set() @@ -248,9 +245,9 @@ class Controller(object): return res serial = dev.serial - selectable = usb_selectable(dev, otp_mode) + selectable = usb_selectable(dev) - if selectable and not otp_mode and transport == TRANSPORT.CCID: + if selectable and transport == TRANSPORT.CCID: controller = OathController(dev.driver) has_password = controller.locked else: @@ -279,17 +276,17 @@ class Controller(object): descs_to_match.remove(matching_descriptor) return res - def refresh_devices(self, otp_mode=False, reader_filter=None): + def refresh_devices(self, reader_filter=None): self._devices = [] - if not otp_mode and reader_filter: + if reader_filter: self._reader_filter = reader_filter dev = self._get_dev_from_reader() if dev: if is_nfc(self._reader_filter): selectable = nfc_selectable(dev) else: - selectable = usb_selectable(dev, otp_mode) + selectable = usb_selectable(dev) if selectable: controller = OathController(dev.driver) has_password = controller.locked @@ -318,7 +315,7 @@ class Controller(object): self._current_derived_key = None return success({'devices': []}) - self._devices = self._get_devices(otp_mode) + self._devices = self._get_devices() # If no current serial, or current serial seems removed, # select the first serial found. @@ -401,77 +398,10 @@ class Controller(object): if e.sw == SW.INCORRECT_PARAMETERS: return failure('validate_failed') - def _otp_get_code_or_touch( - self, slot, digits, timestamp, wait_for_touch=False): - code = None - touch = False - with self._open_otp() as otp_controller: - try: - code = otp_controller.calculate( - slot, challenge=timestamp, totp=True, - digits=int(digits), wait_for_touch=wait_for_touch) - except YkpersError as e: - if e.errno == 11: # Operation would block, touch credential - touch = True - else: - raise - return code, touch - - def otp_calculate_all( - self, slot1_digits, slot2_digits, timestamp): - valid_from = timestamp - (timestamp % 30) - valid_to = valid_from + 30 - entries = [] - - def calc(slot, digits, label): - try: - code, touch = self._otp_get_code_or_touch( - slot, digits, timestamp) - entries.append({ - 'credential': cred_to_dict( - Credential(label.encode(), OATH_TYPE.TOTP, touch)), - 'code': code_to_dict( - Code(code, valid_from, valid_to)) if code else None - }) - except YkpersError as e: - if e.errno == 4: - pass - else: - raise - - if slot1_digits: - calc(1, slot1_digits, "Slot 1") - - if slot2_digits: - calc(2, slot2_digits, "Slot 2") - - return success({'entries': entries}) - - def otp_calculate(self, slot, digits, credential, timestamp): - valid_from = timestamp - (timestamp % 30) - valid_to = valid_from + 30 - code, _ = self._otp_get_code_or_touch( - slot, digits, timestamp, wait_for_touch=True) - return success({ - 'credential': credential, - 'code': code_to_dict(Code(code, valid_from, valid_to)) - }) - def otp_slot_status(self): with self._open_otp() as otp_controller: return success({'status': otp_controller.slot_status}) - def otp_add_credential(self, slot, key, touch): - key = parse_b32_key(key) - with self._open_otp() as otp_controller: - otp_controller.program_chalresp(int(slot), key, touch) - return success() - - def otp_delete_credential(self, slot): - with self._open_otp() as otp_controller: - otp_controller.zap_slot(slot) - return success() - def _unlock(self, controller): if controller.locked: keys = self.settings.get('keys', {}) diff --git a/qml/CredentialCard.qml b/qml/CredentialCard.qml index 04be6737..bd07acf0 100644 --- a/qml/CredentialCard.qml +++ b/qml/CredentialCard.qml @@ -86,20 +86,7 @@ Pane { hotpTouchTimer.start() } - if (settings.otpMode) { - yubiKey.otpCalculate(credential, function (resp) { - if (resp.success) { - hotpTouchTimer.stop() - if (copy) { - copyCode(resp.code.value) - } - entries.updateEntry(resp) - } else { - navigator.snackBarError(resp.error_id) - console.log("calculate failed:", resp.error_id) - } - }) - } else { + yubiKey.calculate(credential, function (resp) { if (resp.success) { hotpTouchTimer.stop() @@ -124,7 +111,7 @@ Pane { console.log("calculate failed:", resp.error_id) } }) - } + } else { copyCode(code.value) } @@ -141,25 +128,7 @@ Pane { "message": qsTr("This will permanently delete the account from your YubiKey."), "description": qsTr("You will not be able to generate security codes for the account anymore. Make sure 2FA has been disabled before proceeding."), "acceptedCb": function () { - if (settings.otpMode) { - yubiKey.otpDeleteCredential(credential, - function (resp) { - if (resp.success) { - if (favorite) - { - toggleFavorite() - } - entries.deleteEntry( - credential.key) - navigator.snackBar( - qsTr("Account deleted")) - } else { - navigator.snackBarError( - resp.error_id) - console.log("delete failed:", resp.error_id) - } - }) - } else { + yubiKey.deleteCredential(credential, function (resp) { if (resp.success) { @@ -181,7 +150,7 @@ Pane { } }) } - } + }) } diff --git a/qml/NewCredentialView.qml b/qml/NewCredentialView.qml index 67259d14..0dfc8a51 100644 --- a/qml/NewCredentialView.qml +++ b/qml/NewCredentialView.qml @@ -32,14 +32,10 @@ Flickable { function acceptableInput() { // trim spaces to accurately count length, parse_b32_key later trims them var secretKeyTrimmed = secretKeyLbl.text.replace(/ /g, "") - if (settings.otpMode) { - return secretKeyTrimmed.length > 0 && secretKeyTrimmed.length <= 32 - } else { - var nameAndKey = nameLbl.text.length > 0 + var nameAndKey = nameLbl.text.length > 0 && secretKeyTrimmed.length > 0 - var okTotalLength = (nameLbl.text.length + issuerLbl.text.length) < 60 - return nameAndKey && okTotalLength - } + var okTotalLength = (nameLbl.text.length + issuerLbl.text.length) < 60 + return nameAndKey && okTotalLength } function addCredentialNoCopy() { @@ -66,12 +62,6 @@ Flickable { } } - function _otpAddCredential() { - yubiKey.otpAddCredential(otpSlotComboBox.currentText, - secretKeyLbl.text, - requireTouchCheckBox.checked, callback) - } - function _ccidAddCredential(overwrite) { yubiKey.ccidAddCredential(nameLbl.text, secretKeyLbl.text, issuerLbl.text, @@ -93,27 +83,7 @@ Flickable { } if (acceptableInput()) { - if (settings.otpMode) { - yubiKey.otpSlotStatus(function (resp) { - if (resp.success) { - if (resp.status[parseInt( - otpSlotComboBox.currentText) - 1]) { - navigator.confirm({ - "heading": qsTr("Overwrite?"), - "message": qsTr("This slot is already configured, do you want to overwrite it?"), - "acceptedCb": _otpAddCredential - }) - } else { - _otpAddCredential() - } - } else { - navigator.snackBarError(navigator.getErrorMessage( - resp.error_id)) - } - }) - } else { - _ccidAddCredentialNoOverwrite() - } + _ccidAddCredentialNoOverwrite() settings.requireTouch = requireTouchCheckBox.checked } } @@ -121,17 +91,6 @@ Flickable { Keys.onEscapePressed: navigator.home() - function getEnabledOtpSlots() { - var res = [] - if (settings.slot1digits) { - res.push(1) - } - if (settings.slot2digits) { - res.push(2) - } - return res - } - ColumnLayout { anchors.fill: parent Layout.fillHeight: true @@ -208,7 +167,7 @@ Flickable { Layout.fillWidth: true text: credential && credential.issuer ? credential.issuer : "" - visible: !settings.otpMode + visible: true onSubmit: addCredential() } StyledTextField { @@ -217,7 +176,7 @@ Flickable { Layout.fillWidth: true required: true text: credential && credential.name ? credential.name : "" - visible: !settings.otpMode + visible: true onSubmit: addCredential() } StyledTextField { @@ -234,16 +193,6 @@ Flickable { onSubmit: addCredential() } - RowLayout { - Layout.fillWidth: true - StyledComboBox { - label: qsTr("Slot") - id: otpSlotComboBox - model: getEnabledOtpSlots() - } - visible: settings.otpMode - } - RowLayout { CheckBox { id: requireTouchCheckBox @@ -255,14 +204,13 @@ Flickable { font.pixelSize: 13 } visible: yubiKey.supportsTouchCredentials() - || settings.otpMode } StyledExpansionPanel { id: advancedSettingsPanel label: qsTr("Advanced settings") description: qsTr("Changing these may result in unexpected behavior.") - visible: manualEntry && !settings.otpMode + visible: manualEntry dropShadow: false backgroundColor: "transparent" @@ -326,7 +274,7 @@ Flickable { id: addBtn text: qsTr("Add") toolTipText: qsTr("Add account to YubiKey") - enabled: settings.otpMode ? secretKeyLbl.validated && acceptableInput() : secretKeyLbl.validated && acceptableInput() && nameLbl.validated + enabled: secretKeyLbl.validated && acceptableInput() && nameLbl.validated Layout.alignment: Qt.AlignLeft | Qt.AlignVCenter onClicked: addCredential() Layout.bottomMargin: -16 diff --git a/qml/NoYubiKeySection.qml b/qml/NoYubiKeySection.qml index e6128bc0..272d3db0 100644 --- a/qml/NoYubiKeySection.qml +++ b/qml/NoYubiKeySection.qml @@ -45,7 +45,7 @@ ColumnLayout { Label { text: { if (yubiKey.availableDevices.length > 0 && !yubiKey.availableDevices.some(dev => dev.selectable)) { - return qsTr("Yubico Authenticator requires a CCID/OTP enabled and compatible YubiKey.") + return qsTr("Yubico Authenticator requires a CCID enabled and compatible YubiKey.") } else { return "" @@ -76,8 +76,8 @@ ColumnLayout { } Label { - text: settings.useCustomReader ? qsTr("Interface: CCID - Custom reader") : qsTr("Interface: OTP") - visible: settings.useCustomReader || settings.otpMode + text: qsTr("Interface: CCID - Custom reader") + visible: settings.useCustomReader Layout.minimumWidth: 300 Layout.maximumWidth: app.width - dynamicMargin < dynamicWidth ? app.width - dynamicMargin : dynamicWidth diff --git a/qml/SettingsView.qml b/qml/SettingsView.qml index 87192130..1e33d34b 100644 --- a/qml/SettingsView.qml +++ b/qml/SettingsView.qml @@ -145,26 +145,6 @@ Flickable { } } - ListModel { - id: otpModeDigits - - ListElement { - text: "Off" - value: 0 - } - ListElement { - text: "6" - value: 6 - } - ListElement { - text: "7" - value: 7 - } - ListElement { - text: "8" - value: 8 - } - } ColumnLayout { id: content @@ -248,7 +228,7 @@ Flickable { id: passwordManagementPanel label: !!yubiKey.currentDevice && yubiKey.currentDevice.hasPassword ? qsTr("Change password") : qsTr("Set password") description: qsTr("For additional security the YubiKey may be protected with a password.") - visible: !!yubiKey.currentDevice && !settings.otpMode + visible: !!yubiKey.currentDevice ColumnLayout { @@ -308,7 +288,7 @@ Flickable { label: qsTr("Reset") description: qsTr("Warning: Reset will delete all accounts and restore factory defaults.") isEnabled: false - visible: !!yubiKey.currentDevice && !settings.otpMode + visible: !!yubiKey.currentDevice toolButtonIcon: "../images/reset.svg" toolButtonToolTip: qsTr("Reset device") toolButton.onClicked: navigator.confirm({ @@ -382,32 +362,15 @@ Flickable { id: interfacePanel label: qsTr("Interface") description: qsTr("Configure how to communicate with the YubiKey.") - property bool otpModeSelected: interfaceCombobox.currentIndex === 2 property bool customReaderSelected: interfaceCombobox.currentIndex === 1 - property bool aboutToChange: (otpModeSelected !== settings.otpMode) - || (slot1DigitsComboBox.currentIndex - !== getComboBoxIndex( - settings.slot1digits)) - || (slot2DigitsComboBox.currentIndex - !== getComboBoxIndex( - settings.slot2digits)) - || customReaderSelected !== settings.useCustomReader + property bool aboutToChange: customReaderSelected !== settings.useCustomReader || readerFilter.text !== settings.customReaderName && readerFilter.text.length > 0 function isValidMode() { return aboutToChange - && ((otpModeSelected - && (slot1DigitsComboBox.currentIndex !== 0 - || slot2DigitsComboBox.currentIndex !== 0)) - || !otpModeSelected) } function setInterface() { - settings.slot1digits = otpModeDigits.get( - slot1DigitsComboBox.currentIndex).value - settings.slot2digits = otpModeDigits.get( - slot2DigitsComboBox.currentIndex).value - settings.otpMode = otpModeSelected settings.useCustomReader = customReaderSelected settings.customReaderName = readerFilter.text yubiKey.clearCurrentDeviceAndEntries() @@ -440,14 +403,12 @@ Flickable { StyledComboBox { id: interfaceCombobox label: qsTr("Interface") - model: ["CCID (recommended)", "CCID with custom reader", "OTP"] + model: ["CCID (recommended)", "CCID with custom reader"] currentIndex: getCurrentIndex() function getCurrentIndex() { - if (settings.otpMode) { - return 2 - } - if (settings.useCustomReader && !settings.otpMode) { + + if (settings.useCustomReader) { return 1 } // default @@ -457,46 +418,6 @@ Flickable { } } - RowLayout { - visible: interfacePanel.otpModeSelected - Label { - Layout.fillWidth: true - font.pixelSize: 12 - color: primaryColor - opacity: lowEmphasis - text: qsTr("Using OTP slots should be considered for special cases only.") - wrapMode: Text.WordWrap - Layout.rowSpan: 1 - bottomPadding: 8 - } - } - - RowLayout { - visible: interfacePanel.otpModeSelected - - StyledComboBox { - id: slot1DigitsComboBox - label: qsTr("Slot 1 digits") - comboBox.textRole: "text" - model: otpModeDigits - currentIndex: interfacePanel.getComboBoxIndex( - settings.slot1digits) - } - - Item { - width: 16 - } - - StyledComboBox { - id: slot2DigitsComboBox - label: qsTr("Slot 2 digits") - comboBox.textRole: "text" - model: otpModeDigits - currentIndex: interfacePanel.getComboBoxIndex( - settings.slot2digits) - } - } - ColumnLayout { visible: interfacePanel.customReaderSelected diff --git a/qml/StyledToolBar.qml b/qml/StyledToolBar.qml index c19d09ef..78d2663e 100644 --- a/qml/StyledToolBar.qml +++ b/qml/StyledToolBar.qml @@ -32,7 +32,7 @@ ToolBar { function shouldShowSearch() { return !!(navigator.currentItem && navigator.currentItem.objectName === 'credentialsView' - && entries.count > 0 && !settings.otpMode) + && entries.count > 0) } function shouldShowSettings() { diff --git a/qml/YubiKey.qml b/qml/YubiKey.qml index d0d0b0a5..cedc5b6b 100644 --- a/qml/YubiKey.qml +++ b/qml/YubiKey.qml @@ -239,7 +239,7 @@ Python { function refreshDevicesDefault() { poller.running = false let customReaderName = settings.useCustomReader ? settings.customReaderName : null - refreshDevices(settings.otpMode, customReaderName, function (resp) { + refreshDevices(customReaderName, function (resp) { if (resp.success) { availableDevices = resp.devices // no current device, or current device is no longer available, pick a new one @@ -286,15 +286,12 @@ Python { } } - if (settings.useCustomReader && !settings.otpMode) { + if (settings.useCustomReader) { checkReaders(settings.customReaderName, callback) } else { checkDescriptors(callback) } - - if (!settings.otpMode) { - refreshReaders() - } + refreshReaders() } function calculateAll(cb) { @@ -323,11 +320,9 @@ Python { } } - if (settings.otpMode) { - otpCalculateAll(callback) - } else { - ccidCalculateAll(callback) - } + + ccidCalculateAll(callback) + } function updateNextCalculateAll() { @@ -380,14 +375,8 @@ Python { doCall('yubikey.controller.ccid_calculate_all', [now], cb) } - function otpCalculateAll(cb) { - var now = Utils.getNow() - doCall('yubikey.controller.otp_calculate_all', - [settings.slot1digits, settings.slot2digits, now], cb) - } - - function refreshDevices(otpMode, customReader, cb) { - doCall('yubikey.controller.refresh_devices', [otpMode, customReader], cb) + function refreshDevices(customReader, cb) { + doCall('yubikey.controller.refresh_devices', [customReader], cb) } function selectCurrentSerial(serial, cb) { @@ -401,23 +390,6 @@ Python { [credential, nowAndMargin], cb) } - function otpCalculate(credential, cb) { - var margin = credential.touch ? 10 : 0 - var nowAndMargin = Utils.getNow() + margin - var slot = (credential.key === "Slot 1") ? 1 : 2 - var digits = (slot === 1) ? settings.slot1digits : settings.slot2digits - doCall('yubikey.controller.otp_calculate', - [slot, digits, credential, nowAndMargin], cb) - } - - function otpDeleteCredential(credential, cb) { - var slot = (credential.key === "Slot 1") ? 1 : 2 - doCall('yubikey.controller.otp_delete_credential', [slot], cb) - } - - function otpAddCredential(slot, key, touch, cb) { - doCall('yubikey.controller.otp_add_credential', [slot, key, touch], cb) - } function ccidAddCredential(name, key, issuer, oathType, algo, digits, period, touch, overwrite, cb) { doCall('yubikey.controller.ccid_add_credential', diff --git a/qml/main.qml b/qml/main.qml index 2f470654..1c6df9d9 100644 --- a/qml/main.qml +++ b/qml/main.qml @@ -178,23 +178,6 @@ ApplicationWindow { qsTr("Touch your YubiKey now to generate security code."), SystemTrayIcon.NoIcon) } - if (settings.otpMode) { - yubiKey.otpCalculate(credential, function (resp) { - if (resp.success) { - clipBoard.push(resp.code.value) - sysTrayIcon.showMessage( - qsTr("Copied to clipboard"), - "The code for " + text + " is now in the clipboard.", - SystemTrayIcon.NoIcon) - } else { - sysTrayIcon.showMessage( - "Error", - "calculate failed: " + resp.error_id, - SystemTrayIcon.NoIcon) - console.log("calculate failed:", resp.error_id) - } - }) - } else { yubiKey.calculate(credential, function (resp) { if (resp.success) { clipBoard.push(resp.code.value) @@ -210,7 +193,7 @@ ApplicationWindow { console.log("calculate failed:", resp.error_id) } }) - } + } function getFavoriteEntries() { @@ -291,11 +274,6 @@ ApplicationWindow { Settings { id: settings - // Can be 0 (off), 6, 7 or 8 - property int slot1digits - property int slot2digits - - property bool otpMode property bool useCustomReader property string customReaderName