Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fido2 - conte91 #437

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@
[submodule "tools/ttf2ugui"]
path = tools/ttf2ugui
url = https://github.com/digitalbitbox/ttf2ugui
[submodule "external/tinycbor"]
path = external/tinycbor
url = https://github.com/intel/tinycbor
Copy link
Contributor

Choose a reason for hiding this comment

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

It would make sense to fork and point here to a digitalbitbox/tinycbor imho, like all the other submodule deps.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, quite a few annoying warnings about things like __cplusplus not being defined.
I wonder if tinycbor needs different or additional flags. For example, looks like they're building with -std=gnu99 but we default to -std=c99.

9 changes: 9 additions & 0 deletions external/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ set_property(TARGET secp256k1
set_target_properties(secp256k1 PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/libwally-core/src/secp256k1/include)
set_target_properties(secp256k1 PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/libwally-core/src/secp256k1/include)

add_library(tinycbor STATIC
tinycbor/src/cborerrorstrings.c
tinycbor/src/cborencoder.c
tinycbor/src/cborencoder_close_container_checked.c
tinycbor/src/cborparser.c
tinycbor/src/cborpretty.c
)
set_target_properties(tinycbor PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/tinycbor/src)
set_target_properties(tinycbor PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/tinycbor/src)

if(CMAKE_CROSSCOMPILING)
# Cortex Microcontroller Software Interface Standard
Expand Down
1 change: 1 addition & 0 deletions external/tinycbor
Submodule tinycbor added at 2b1105
144 changes: 144 additions & 0 deletions scripts/create_fido2_attestation_cert
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
#!/usr/bin/env python3

import asn1
import base64
import os
import subprocess
import sys
import tempfile

def get_root_cert(base_dir):
return os.path.join(base_dir, 'root_cert.pem')

def get_final_cert(base_dir):
return os.path.join(base_dir, 'cert.der')

def get_root_privkey(base_dir):
return os.path.join(base_dir, 'root_privkey.pem')

def get_final_privkey(base_dir):
return os.path.join(base_dir, 'privkey.pem')

def run_ssl_command(cmd):
print("Running: {}".format(cmd))
p = subprocess.Popen(cmd, stdout=sys.stdout, stdin=sys.stdin, stderr=sys.stderr)
p.communicate()

def generate_privkey(privkey_filename):
openssl_command = ['openssl', 'ecparam', '-name', 'secp256r1', '-genkey', '-out', privkey_filename]
run_ssl_command(openssl_command)

def parse_privkey(privkey_filename):
p = subprocess.Popen(['openssl', 'ec', '-in', privkey_filename, '-text', '-noout'], stdin=None, stdout=subprocess.PIPE, stderr=sys.stderr)
Copy link
Contributor

Choose a reason for hiding this comment

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

A slightly too long line.
stdin can be dropped, it's None by default: https://docs.python.org/3/library/subprocess.html#subprocess.Popen

privkey_out, _ = p.communicate()
privkey_out = privkey_out.decode().splitlines()
print("PVO:\n{}".format(privkey_out))
privkey_lines = []
in_privkey = False
for line in privkey_out:
if line[0].isspace():
if in_privkey:
privkey_lines.append(line)
elif line.strip().startswith('priv:'):
in_privkey = True
else:
in_privkey = False
privkey_data = bytes([int(b, 16) for b in ''.join([l.strip() for l in privkey_lines]).split(':')])

assert len(privkey_data) == 32, "Wrong data generated for private key ({}).".format(len(privkey_data))
return privkey_data

def generate_root_cert(base_dir):
print("**** Creating root certificate ****")
cert_filename = get_root_cert(base_dir)
privkey_filename = get_root_privkey(base_dir)
openssl_command = ['openssl', 'req', '-new', '-x509', '-key', privkey_filename, '-outform', 'PEM', '-out', cert_filename, '-days', '10000',
'-subj', '/C=CH/ST=Zurich/L=Adliswil/O=Shift Cryptosecurity AG/CN=Shift Cryptosecurity/[email protected]'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for testing? Actually, I think we'll need to change this anyway, and I'd drop ST, L and emailAddress.

]
run_ssl_command(openssl_command)

def generate_cert_file(base_dir):
# We need an extension file in order to specify additional flags; this
# will result in a version 3 certificate as required by FIDO2.
ext_file = os.path.join(base_dir, 'v3.ext')
with open(ext_file, 'w') as f:
f.write("authorityKeyIdentifier=keyid,issuer\nbasicConstraints=CA:FALSE\nkeyUsage = digitalSignature, nonRepudiation, keyEncipherment, dataEncipherment")

print("**** Creating final certificate ****")
cert_filename = get_final_cert(base_dir)
csr_filename = os.path.join(base_dir, 'final_csr.csr')
run_ssl_command(['openssl', 'req', '-new', '-key', get_final_privkey(base_dir), '-out', csr_filename,
'-subj', '/C=CH/ST=Zurich/L=Adliswil/O=Shift Cryptosecurity AG/OU=Authenticator Attestation/CN=Shift Cryptosecurity/[email protected]'
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, a bit too long lines here and in other places, harder to review.

])
print("**** Signing final certificate ****")
openssl_command = ['openssl', 'x509', '-req', '-in', csr_filename, '-extfile', ext_file, '-CA', get_root_cert(base_dir), '-CAkey', get_root_privkey(base_dir), '-CAcreateserial', '-outform', 'DER', '-out', cert_filename, '-days', '10000'
Copy link
Contributor

Choose a reason for hiding this comment

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

note to self: is 10000 days a good number?

]
run_ssl_command(openssl_command)
with open(cert_filename, 'rb') as cert:
cert_data = cert.read()
return cert_data


def dump_bytearray_to_file(f, name, data):
f.write('const uint8_t {}[] = {{'.format(name))
items_per_line = 10
line_separator = ',\n '
# * f.write(', '.join(['0x{:02x}'.format(b) for b in test2cert]))
split_data = [data[x:x+10] for x in range(0, len(data), items_per_line)]
split_lines = [', '.join(['0x{:02x}'.format(b) for b in line]) for line in split_data]
f.write('\n ')
f.write(line_separator.join(split_lines))
f.write('\n }};\n'.format(line_separator))


def generate_fido2_cert_source(cert_data, privkey):
with open('fido2.c', 'w') as f:
f.write('#include "fido2_keys.h"\n\n')
f.write('/**\n')
f.write(' * Automatically generated with the following command:\n')
f.write(' * {}\n'.format(sys.argv))
f.write(' */\n')
f.write('\n')
dump_bytearray_to_file(f, 'FIDO2_ATT_PRIV_KEY', privkey)
f.write('\n')
dump_bytearray_to_file(f, 'FIDO2_ATT_CERT', cert_data)
f.write('const size_t FIDO2_ATT_CERT_SIZE = sizeof(FIDO2_ATT_CERT);\n')

def parse_private_key_from_pem(privkey_data):
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this program would've been easier in Go - no shelling out to openssl and various encodings like PEM are already provided in stdlib.

privkey_lines = privkey_data.decode().splitlines()
started = False
base64_lines = []
for line in privkey_lines:
if 'BEGIN EC PRIVATE KEY' in line:
started = True
elif 'END EC PRIVATE KEY' in line:
break
elif started:
base64_lines.append(line.strip())
base64_data = ''.join(base64_lines)
print("Base64-encoded privkey data: {}".format(base64_data))
privkey_binary = base64.b64decode(base64_data)
decoder = asn1.Decoder()
decoder.start(privkey_binary)
tag, value = decoder.read()
print('{}, {}'.format(tag, value))


with tempfile.TemporaryDirectory() as tmpdirname:
print("Using temporary directory: {}".format(tmpdirname))
print("**** Creating root privkey ****")
generate_privkey(get_root_privkey(tmpdirname))
generate_root_cert(tmpdirname)
print("**** Creating final privkey ****")
generate_privkey(get_final_privkey(tmpdirname))
privkey_data = parse_privkey(get_final_privkey(tmpdirname))
cert_data = generate_cert_file(tmpdirname)
print("**** Dumping to fido2.c ****")
generate_fido2_cert_source(cert_data, privkey_data)
print("Done :)")
#
#/**
# * Generated with:
# * test2cert = open('/home/simone/provola/server.der', 'rb').read()
# * f.write(', '.join(['0x{:02x}'.format(b) for b in test2cert]))
# */
9 changes: 9 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ set(DBB-FIRMWARE-SOURCES
${CMAKE_SOURCE_DIR}/src/workflow/restore.c
${CMAKE_SOURCE_DIR}/src/workflow/restore_from_mnemonic.c
${CMAKE_SOURCE_DIR}/src/workflow/sdcard.c
${CMAKE_SOURCE_DIR}/src/workflow/select_ctap_credential.c
${CMAKE_SOURCE_DIR}/src/workflow/show_mnemonic.c
${CMAKE_SOURCE_DIR}/src/workflow/orientation_screen.c
${CMAKE_SOURCE_DIR}/src/workflow/status.c
Expand Down Expand Up @@ -81,6 +82,7 @@ set(DBB-FIRMWARE-USB-SOURCES
${CMAKE_SOURCE_DIR}/src/usb/usb.c
${CMAKE_SOURCE_DIR}/src/usb/usb_frame.c
${CMAKE_SOURCE_DIR}/src/usb/usb_packet.c
${CMAKE_SOURCE_DIR}/src/usb/u2f/u2f_keys.c
${CMAKE_SOURCE_DIR}/src/u2f/u2f_packet.c
)
set(DBB-FIRMWARE-USB-SOURCES ${DBB-FIRMWARE-USB-SOURCES} PARENT_SCOPE)
Expand Down Expand Up @@ -211,8 +213,14 @@ set(FIRMWARE-DRIVER-SOURCES
set(FIRMWARE-DRIVER-SOURCES ${FIRMWARE-DRIVER-SOURCES} PARENT_SCOPE)

set(FIRMWARE-U2F-SOURCES
${CMAKE_SOURCE_DIR}/src/fido2/ctap.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you modify those files in any way?

would be great to move them to external/ instead and compile them as a lib, like ctaes for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for moving to external/

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, quite a few changes diffing against their latest at solokeys/solo1@c248b5d which is 4.0.0+. Here's the diff: https://gist.github.com/x1ddos/19cce770b2929bd6c0a4f933f527ecbe

But diffing against their earlier 3.1.3 release seems to diverge even more: https://gist.github.com/x1ddos/8f93bc4d91455460225c0300fea57603.

I'm guessing it might be somewhere between 3.1.3 and 4.0.0. @conte91 which upstream commit did you start with?

${CMAKE_SOURCE_DIR}/src/fido2/ctap_parse.c
${CMAKE_SOURCE_DIR}/src/fido2/device.c
${CMAKE_SOURCE_DIR}/src/fido2/fido2_keys.c
${CMAKE_SOURCE_DIR}/src/fido2/extensions.c
${CMAKE_SOURCE_DIR}/src/u2f.c
${CMAKE_SOURCE_DIR}/src/u2f/u2f_app.c
${CMAKE_SOURCE_DIR}/src/u2f/u2f_keyhandle.c
)
set(FIRMWARE-U2F-SOURCES ${FIRMWARE-U2F-SOURCES} PARENT_SCOPE)

Expand Down Expand Up @@ -521,6 +529,7 @@ if(CMAKE_CROSSCOMPILING)
PRIVATE
cryptoauthlib
fatfs
tinycbor
base32
bignum # TODO: only eth
sha3 # TODO: Only eth
Expand Down
8 changes: 6 additions & 2 deletions src/bootloader/bootloader.c
Original file line number Diff line number Diff line change
Expand Up @@ -843,10 +843,14 @@ static size_t _api_command(const uint8_t* input, uint8_t* output, const size_t m
return len;
}

static void _api_msg(const Packet* in_packet, Packet* out_packet, const size_t max_out_len)
static void _api_msg(const Packet* in_packet)
{
size_t len = _api_command(in_packet->data_addr, out_packet->data_addr, max_out_len);
Packet* out_packet = util_malloc(sizeof(*out_packet));
prepare_usb_packet(in_packet->cmd, in_packet->cid, out_packet);
size_t len = _api_command(in_packet->data_addr, out_packet->data_addr, USB_DATA_MAX_LEN);
out_packet->len = len;
usb_processing_send_packet(usb_processing_hww(), out_packet);
free(out_packet);
}

static void _api_setup(void)
Expand Down
2 changes: 2 additions & 0 deletions src/common_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include "common_main.h"
#include "fido2/ctap.h"
#include "driver_init.h"
#include "flags.h"
#include "hardfault.h"
Expand Down Expand Up @@ -84,4 +85,5 @@ void common_main(void)
if (!securechip_setup(&_securechip_interface_functions)) {
Abort("securechip_setup failed");
}
//ctap_init();
}
22 changes: 22 additions & 0 deletions src/fido2/cose_key.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2019 SoloKeys Developers
//
// Licensed under the Apache License, Version 2.0, <LICENSE-APACHE or
// http://apache.org/licenses/LICENSE-2.0> or the MIT license <LICENSE-MIT or
// http://opensource.org/licenses/MIT>, at your option. This file may not be
// copied, modified, or distributed except according to those terms.
#ifndef _COSE_KEY_H
#define _COSE_KEY_H

#define COSE_KEY_LABEL_KTY 1
#define COSE_KEY_LABEL_ALG 3
#define COSE_KEY_LABEL_CRV -1
#define COSE_KEY_LABEL_X -2
#define COSE_KEY_LABEL_Y -3

#define COSE_KEY_KTY_EC2 2
#define COSE_KEY_CRV_P256 1

#define COSE_ALG_ES256 -7
#define COSE_ALG_ECDH_ES_HKDF_256 -25

#endif
Loading