Skip to content

Commit

Permalink
(net) add interactive controls / split trust
Browse files Browse the repository at this point in the history
This patch adds the option to temporarily or permanently trust unknown
public keys when making an outbound connection. Accepted keys are added
to the 'outbound' domain as to not allow someone you trusted outbound
to, by default, also work inbound.

This needs more testing across multiple devices.
  • Loading branch information
letoram committed Jul 22, 2023
1 parent 2c15bf0 commit 8c6729b
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 50 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -29,6 +29,10 @@
## Net
* allow h264 passthrough, sidestepping local encode
* re-add afsrv\_net, have it support directory, source and sink access modes.
* split a12:// into a12:// and a12s:// ARCAN_CONNPATH with the former permitting connecting to unknowns
* add interactive accept/reject/trust for arcan-net use
* reserve 'outbound' domain for interactively added outbound keys
* enforce domain separation for allowed pubkeys

## Terminal
* SGR reset fix, add CNL / CPL
Expand Down
2 changes: 1 addition & 1 deletion src/a12/a12.h
Expand Up @@ -81,7 +81,7 @@ struct a12_context_options {
* completed */
char secret[32];

/* can be set to ROLE_PROBE, ROLE_SOURCE, ROLE_SINK or ROLE_DIR(EIMPL) */
/* can be set to ROLE_PROBE, ROLE_SOURCE, ROLE_SINK or ROLE_DIR */
int local_role;

/* if set, the a12_flush() will not return a buffer to write out, but rather
Expand Down
13 changes: 12 additions & 1 deletion src/a12/net/HACKING.md
Expand Up @@ -486,7 +486,8 @@ as well.

This command is respected if the receiver is running in directory mode. If
permitted (rate limit, key access restrictions, ...) it will result in a series
of directory-state updates, giving the current set of available appls.
of directory-state updates, giving the current set of available appls,
sources or sinks (depending if connecting as a source, or sink yourself).

If notify is set to !0, the sender requests that any changes to the set will
be provided dynamically without the caller polling through additional
Expand All @@ -506,6 +507,16 @@ about the update, removal, creation or presence of a retrievable application.
An empty identifier terminates. The applname can be used as the extension
field of a BCHUNKSTATE event to initiate the actual transfer.

### command - 11, directory-discover
- [18.. 19] role : uint8 (0) source, (1) sink, (2) directory
- [20 ] state : uint8, (0) added, (1) lost
- [21.. 36] petname : (+16) utf-8 server-generated identifier / user-provided
- [37.. 52] id : (+16) Kpub (x25519)

This is provided when a new source or sink has flagged for availability or been
disconnected. The petname is chosen by hashing into a server-local dictionary
and allocated on first-use or provided on initial HELLO.

## Event (2), fixed length
- [0..7] sequence number : uint64
- [8 ] channel-id : uint8
Expand Down
2 changes: 1 addition & 1 deletion src/a12/net/README.md
Expand Up @@ -253,7 +253,7 @@ Milestone 3 - big stretch (0.6.x)
- [ ] relay a12 traffing between source/sink
- [ ] NAT-punch between source/sink
- [x] state store/restore
- [ ] Add to afsrv\_net (x)
- [x] Add to afsrv\_net (x)
- [ ] Fast-forward known partial binary transfer (resume)
- [ ] Resume- session from different IP (ap)
- [ ] N-Key connection-unlock and monitors (a)
Expand Down
125 changes: 97 additions & 28 deletions src/a12/net/net.c
Expand Up @@ -55,6 +55,7 @@ static struct {
const char* reqname;
struct anet_dirsrv_opts dirsrv;
struct anet_dircl_opts dircl;
char* trust_domain;
} global = {
.backpressure_soft = 2,
.backpressure = 6,
Expand All @@ -80,6 +81,26 @@ static const char* trace_groups[] = {
static struct a12_vframe_opts vcodec_tuning(
struct a12_state* S, int segid, struct shmifsrv_vbuffer* vb, void* tag);

/* keystore is singleton global */
static bool open_keystore(const char** err)
{
int dir = a12helper_keystore_dirfd(err);
if (-1 == dir)
return false;

struct keystore_provider prov = {
.directory.dirfd = dir,
.type = A12HELPER_PROVIDER_BASEDIR
};

if (!a12helper_keystore_open(&prov)){
*err = "Couldn't open keystore from basedir (ARCAN_STATEPATH)";
return false;
}

return true;
}

static int tracestr_to_bitmap(char* work)
{
int res = 0;
Expand Down Expand Up @@ -403,6 +424,14 @@ static struct anet_cl_connection find_connection(
int rc = opts->retry_count;
int timesleep = 1;

const char* err;
if (!open_keystore(&err)){
fprintf(stderr, "couldn't open keystore: %s\n", err);
}

if (!global.trust_domain)
global.trust_domain = strdup("outbound");

/* connect loop until retry count exceeded */
while (rc != 0 && (!cl || (shmifsrv_poll(cl) != CLIENT_DEAD))){
anet = anet_cl_setup(opts);
Expand Down Expand Up @@ -588,12 +617,17 @@ static bool show_usage(const char* msg)
"Forward-local options:\n"
"\t-X \t Disable EXIT-redirect to ARCAN_CONNPATH env (if set)\n"
"\t-r, --retry n \t Limit retry-reconnect attempts to 'n' tries\n\n"
"Options:\n"
"Authentication:\n"
"\t --no-ephem-rt \t Disable ephemeral keypair roundtrip (outbound only)\n"
"\t-a, --auth n \t Read authentication secret from stdin (maxlen:32)\n"
"\t --soft-auth \t authentication secret (password) only, ignore keystore\n"
"\t \t if [n] is provided, n keys added to trusted\n"
"\t-T, --trust s \t Specify trust domain for splitting keystore\n"
"\t \t outbound connections default to 'outbound' while\n"
"\t \t serving/listening defaults to a wildcard ('*')\n\n",
""
"Options:\n"
"\t-t \t Single- client (no fork/mt - easier troubleshooting)\n"
"\t --no-ephem-rt \t Disable ephemeral keypair roundtrip (outbound only)\n"
"\t --probe-only \t (outbound) Authenticate and print server primary state\n"
"\t-d bitmap \t Set trace bitmap (bitmask or key1,key2,...)\n"
"\t-v, --version \t Print build/version information to stdout\n\n"
Expand Down Expand Up @@ -791,6 +825,13 @@ static int apply_commandline(int argc, char** argv, struct arcan_net_meta* meta)
else if (strcmp(argv[i], "-t") == 0){
opts->mt_mode = MT_SINGLE;
}
else if (strcmp(argv[i], "-T") == 0 || strcmp(argv[i], "--trust") == 0){
i++;
if (i == argc)
return show_usage("--trust without domain argument");

global.trust_domain = argv[i];
}
else if (strcmp(argv[i], "--keep-appl") == 0){
global.dircl.keep_appl = true;
}
Expand Down Expand Up @@ -850,7 +891,6 @@ static int apply_commandline(int argc, char** argv, struct arcan_net_meta* meta)
a12int_trace(A12_TRACE_SECURITY,
"trust_first=%zu", global.accept_n_pk_unknown);
}

}
else if (strcmp(argv[i], "-B") == 0){
if (i == argc - 1)
Expand Down Expand Up @@ -884,26 +924,6 @@ static int apply_commandline(int argc, char** argv, struct arcan_net_meta* meta)
return i;
}

/* keystore is singleton global */
static bool open_keystore(const char** err)
{
int dir = a12helper_keystore_dirfd(err);
if (-1 == dir)
return false;

struct keystore_provider prov = {
.directory.dirfd = dir,
.type = A12HELPER_PROVIDER_BASEDIR
};

if (!a12helper_keystore_open(&prov)){
*err = "Couldn't open keystore from basedir (ARCAN_STATEPATH)";
return false;
}

return true;
}

static int apply_keystore_command(int argc, char** argv)
{
/* (opt, -b dir) */
Expand Down Expand Up @@ -954,13 +974,20 @@ static int apply_keystore_command(int argc, char** argv)
* This is used both for the inbound and outbound connection, the difference
* though is that the outbound connection has already provided keymaterial
* as that is necessary for constructing the HELLO packet - so the auth is
* just against if the public key is known/trusted or not in that case.
* just against if the public key is known/trusted or not in that case, with
* the options of what to do in the event of an unknown key:
*
* 1. reject / disconnect.
* 2. accept for this session.
* 3. accept and add to the trust store for outbound connections.
* 4. accept and add to the trust store for out and onbound.
* 5. prompt the user.
*
* Thus the a12 implementation will only respect the 'authentic' part and
* disregard whatever is put in key.
*
* For inbound, there is an option of differentiation - a different keypair
* could be returned based on the public key.
* could be returned based on the public key that the connecting party uses.
*/
static struct pk_response key_auth_local(uint8_t pk[static 32])
{
Expand All @@ -972,7 +999,7 @@ static struct pk_response key_auth_local(uint8_t pk[static 32])
unsigned char* out = a12helper_tob64(pk, 32, &outl);

/* is the key in our trusted set? */
if (a12helper_keystore_accepted(pk, NULL)){
if (a12helper_keystore_accepted(pk, global.trust_domain)){
auth.authentic = true;
a12int_trace(A12_TRACE_SECURITY, "accept=%s", out);
a12helper_keystore_hostkey("default", 0, auth.key, &tmp, &tmpport);
Expand All @@ -989,11 +1016,53 @@ static struct pk_response key_auth_local(uint8_t pk[static 32])
global.accept_n_pk_unknown--;
a12int_trace(A12_TRACE_SECURITY,
"left=%zu:accept-unknown=%s", global.accept_n_pk_unknown, out);
a12helper_keystore_accept(pk, NULL);

/* trust-domain covers both case 3 and 4. */
a12helper_keystore_accept(pk, global.trust_domain);
a12helper_keystore_hostkey("default", 0, auth.key, &tmp, &tmpport);
}

/* Since SSH has trained people on this behaviour, allow interactive override.
* A caveat here with far reaching consequences is if 'remember' should mean
* to add it to the trust store for both inbound and outbound connections.
*
* There are arguments to be had for both cases. By adding it it also means
* that by default, should the local end ever listen for an external connection,
* previous servers would be allowed in. In the SSH case that would be terrible.
*
* Here it is more nuanced. One can argue that if you are serving you should
* split / config / domain of use separate the keystore anyhow and that the
* current one is called 'naive' for a reason. This is trivially done in the
* filesystem, though not as easy for the HCI side.
*
* At the same time the 'accept_n_pk' option is more explicit (and encourages
* another temporary password) for the pake 'first time setup' / sideband / f2f
* case.
*/
else if (!auth.authentic){
a12int_trace(A12_TRACE_SECURITY, "reject-unknown=%s", out);
if (isatty(STDIN_FILENO)){
fprintf(stdout,
"The other end is using an unknown public key (%s).\n"
"Are you sure you want to continue (yes/no/remember):\n", out
);
char buf[16];
fgets(buf, 16, stdin);
if (strcmp(buf, "yes\n") == 0){
auth.authentic = true;
a12helper_keystore_hostkey("default", 0, auth.key, &tmp, &tmpport);
a12int_trace(A12_TRACE_SECURITY, "interactive-soft-auth=%s", out);
}
else if (strcmp(buf, "remember\n") == 0){
auth.authentic = true;
a12helper_keystore_accept(pk, global.trust_domain);
a12int_trace(A12_TRACE_SECURITY, "interactive-add-trust=%s", out);
a12helper_keystore_hostkey("default", 0, auth.key, &tmp, &tmpport);
}
else
a12int_trace(A12_TRACE_SECURITY, "rejected-interactive");
}
else
a12int_trace(A12_TRACE_SECURITY, "reject-unknown=%s", out);
}

if (auth.authentic && global.directory != -1){
Expand Down
7 changes: 6 additions & 1 deletion src/frameserver/net/default/net.c
Expand Up @@ -26,6 +26,7 @@
#include <netdb.h>
#include <fcntl.h>
#include <errno.h>
#include <signal.h>
#include "a12.h"
#include "a12_int.h"
#include "net/a12_helper.h"
Expand Down Expand Up @@ -53,6 +54,7 @@ static bool flush_shmif(struct arcan_shmif_cont* C)

static struct {
bool soft_auth;
char* trust_domain;
} global;

static struct pk_response key_auth_local(uint8_t pk[static 32])
Expand All @@ -65,7 +67,7 @@ static struct pk_response key_auth_local(uint8_t pk[static 32])
* here is the option of deferring pubk- auth to arcan end by sending it as a
* message onwards and wait for an accept or reject event before moving on.
*/
if (a12helper_keystore_accepted(pk, NULL) || global.soft_auth){
if (a12helper_keystore_accepted(pk, global.trust_domain) || global.soft_auth){
auth.authentic = true;
a12helper_keystore_hostkey("default", 0, auth.key, &tmp, &tmpport);
}
Expand Down Expand Up @@ -866,6 +868,9 @@ int afsrv_netcl(struct arcan_shmif_cont* C, struct arg_arr* args)
{
/* lua:net_discover maps to this */
const char* dmethod = NULL;
signal(SIGPIPE, SIG_IGN);
signal(SIGCHLD, SIG_IGN);

if (arg_lookup(args, "help", 0, &dmethod)){
return show_help();
}
Expand Down
20 changes: 9 additions & 11 deletions src/frameserver/util/anet_helper.h
Expand Up @@ -100,33 +100,31 @@ uint8_t* a12helper_tob64(const uint8_t* data, size_t inl, size_t* outl);
* increment index to fetch the next possible host.
*
* returns false when there are no more keys on the store */
bool a12helper_keystore_hostkey(const char* tagname, size_t index,
bool a12helper_keystore_hostkey(const char* petname, size_t index,
uint8_t privk[static 32], char** outhost, uint16_t* outport);

/* list all the known outbound tags, terminates with a NULL tagname */
bool a12helper_keystore_tags(bool (*cb)(const char* tagname, void*), void* tag);
/* list all the known outbound tags, terminates with a NULL petname */
bool a12helper_keystore_tags(bool (*cb)(const char* petname, void*), void* tag);

/* Append or crete a new tag with the specified host, this will also
* create a new private key if needed. Returns the public key in outk */
bool a12helper_keystore_register(
const char* tagname, const char* host, uint16_t port, uint8_t pubk[static 32]);
const char* petname, const char* host, uint16_t port, uint8_t pubk[static 32]);

/*
* Check if the public key is known and accepted for the supplied connection
* point (can be null for any connection point).
* Check if the public key is known and accepted for the specified trust domain
* (not to be confused with host/domain names as in DNS).
*/
bool a12helper_keystore_accepted(
const uint8_t pubk[static 32], const char* connp);

/*
* add the supplied public key to the accepted keystore.
*
* if [connp] is NULL, it will only be added as a permitted outgoing connection
* (client mode), otherwise set a comma separated list (or * for wildcard) of
* valid connection points this key is allowed to bind to.
* if [connp] is NULL, the domain will default to 'outbound'.
*
* the [state-cap] indicates if the owner of the key is allowed to store state
* locally or not, with 0 blocking state transfers
* otherwise connp is a comma separated list of local names (similar to
* connection points) or a wildcard '*'.
*/
bool a12helper_keystore_accept(const uint8_t pubk[static 32], const char* connp);

Expand Down
10 changes: 3 additions & 7 deletions src/frameserver/util/anet_keystore_naive.c
Expand Up @@ -347,7 +347,7 @@ bool a12helper_keystore_accept(const uint8_t pubk[static 32], const char* connp)
return false;

if (!connp)
connp = "*";
connp = "outbound";

/* get base64 of pubk, just write that + space + connp <lf> */
size_t outl;
Expand Down Expand Up @@ -662,6 +662,8 @@ int a12helper_keystore_statestore(
bool a12helper_keystore_accepted(const uint8_t pubk[static 32], const char* connp)
{
struct key_ent* ent = keystore.hosts;
if (!connp)
connp = "outbound";

while (ent){
/* not this key? */
Expand All @@ -675,12 +677,6 @@ bool a12helper_keystore_accepted(const uint8_t pubk[static 32], const char* conn
return true;
}

/* nope, and this is an unspecified connection point, so ignore */
if (!connp){
ent = ent->next;
continue;
}

/* then host- list is separated cp1,cp2,cp3,... so find needle in haystack */
const char* needle = strstr(connp, ent->host);
if (!needle){
Expand Down

0 comments on commit 8c6729b

Please sign in to comment.