I just merged #144 which appeared to be green in the UI, but as it turns out it had check errors that are now on master.

For some reason, https://github.com/google/glome/pull/144/checks is empty.

Now that we agreed to use glib, we might want to consider moving away from libconfuse to something more solid/understandable like glib's Key-value file parser.

The C implementation of glome-login used & 0x7f to mask the top bit of the service key ID and ensure it fits into 7 bits. The same logic was reused for calculating the key ID from the top byte of the service public key:

  if (service_key_id == 0) {
    // If no key ID was specified, send the first key byte as the ID.
    handshake[0] = service_key[0] & 0x7f;
  } else {
    handshake[0] = service_key_id & 0x7f;
  }

This is broken and sadly has been replicated into test vector 2 as well. To get the top 7 bits of the key ID the correct calculation would be >> 1 and thus 0xd1 >> 1 = 0x68 instead of 0xd1 & 0x7f = 0x51.

It would be convenient if we could set a couple of options that are usually constant in a given deployment environment with the config file. That would allow sharing a glome-login binary between environments, and customizing each environment with the config file only. Things that I can think of:

• hostid-type (hostid would still be obtained from gethostname(2))
• syslog config
• timeouts and sleeps
• minimum tag length

Let's use this as an umbrella issue for clarifications that are needed in the glome-login protocol doc.

First off: Make sure we cover how URLs are supposed to be escaped in the protocol documentation.

The function ParseURLResponse does not accept URLs, only paths. Providing a URL as input does not fail, but produces unexpected results.

• The function is used correctly in server.go, but the naming and documentation is misleading.
• The HandshakeInfo seems to be computed correctly.
• The Message is wrong.

Example code with output:

package main

import (
	"fmt"
	"log"

	"github.com/google/glome/go/glome"
	"github.com/google/glome/go/login"
)

func main() {
	srv := login.Server{KeyFetcher: func(_ uint8) (*glome.PrivateKey, error) {
		var key glome.PrivateKey
		key[5] = 1
		return &key, nil
	}}
	resp, err := srv.ParseURLResponse("https://glome.example.com/v1/UYcvQ1u4uJ0OOtYqouURB07hleHDnvaogAFBi-ZW48N2/serial-number:123/reboot/")
	if err != nil {
		log.Fatal(err)
	}
	fmt.Printf("login.Message%+v\n", resp.Msg)
}

login.Message{HostIDType:https HostID: Action:/glome.example.com/v1/UYcvQ1u4uJ0OOtYqouURB07hleHDnvaogAFBi-ZW48N2/serial-number:123/reboot}

We have an internal user of GLOME that's written their own config parser. Let's provide an in-tree config parser in Go, so we can include it in test suites publically and keep it updated in lockstep with the C one.

It would be great if PAM module arguments naming was consistent with the config (and flags in the future once glome-login suports long flags).

Examples of inconsistency:

• service_key (PAM) vs key (config)
• service_key_version (PAM) vs key-version (config)
• url_prefix (PAM) vs url-prefix (config)

Personally, I like the config naming better. It's shorter and uses dashes instead of underscores.

See this action: https://github.com/google/glome/actions/runs/3758587828/jobs/6387107347. It seems like the issue is that below warning is treated as an error, but I don't understand why that changed since that action was merged.

/nix/store/mmmjp0xcmr0dm307jv1l34hvi95z6d8s-glibc-2.35-224-dev/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
  412 | #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
      |    ^~~~~~~
cc1: all warnings being treated as errors

We only test that a tag produced by the CLI is in turn correctly verified by the CLI - this fails to detect symmetric breakage, e.g. due to invalid parsing / reading. There should be a suite that verifies the test vectors from the specification.

In glome-login --help I spotted the following:

 -k KEY    use KEY as the hex-encoded service key (defaults to the embedded key)

I can't seem to find any reference to what this embedded key is and where it lives, and from looking at the source code I suspect it might not exist. Could you clarify the documentation and/or remove this?

When running "meson install", the build system installs example.cfg as the default configuration. Since example.cfg is a working config file with a hardcoded key, this seems like a dangerous behavior.

Luckily, it happens that a bug makes this harmless right now: meson installs to ${sysconfdir}/config while glome-login attempts to look up the config in ${sysconfdir}/glome/config. I suspect this is however not deliberate :-)

Instead of single sentences, the public functions should document all their parameters, especially wrt ownership, and describe the scenario where the function is appropriate.

https://www.openssl.org/docs/man3.1/man3/HMAC.html

The alternative is EVP_MAC_* afaict.

Currently GLOME Login prompts assume that the user is obtaining authcodes by following an URL. While this might be true in some environments, most individual users are going to simply use glome CLI to generate responses and they might be confused by the current UI.

Contexts where it is assumed that the challenge is an URL:

The following one doesn't say that the challenge is an URL but perhaps the wording here could be improved:

The build process contains a couple of minor choices that prevent porting onto non-glibc systems. My use case is Alpine, but the situation is probably similar on MacOS or *BSD.

• Bash shell is assumed to be available, thus bashisms in scripts.
• Non-POSIX utilities used: mktemp, realpath.
• glibc-only headers are used (unnecessarily :) ).

My plan for implementation:

• #165
• #163
• #164
• #166

We should document the various options for glome-login and explain what e.g. lockdown mode is.

We should actually go and design how glome-login is supposed to be used - with integration into Debian/Ubuntu as an example. I'd suggest the following:

• Users should be able to install a libpam-glome package and the distro scripts would add pam_glome to at least the login PAM auth chain, if not common-auth.
• This would do nothing (no failure or error message, PAM_AUTHINFO_UNAVAIL?) unless /etc/glome/config is found and contains both a service key and a URL prefix - or alternative both are passed via the arguments to the PAM module.
• The PAM module should use pam_get_authtok to get the tag, which honors try_first_pass. That way we can stack modules properly.
• pam_glome should then sort before pam_unix (in Debian/Ubuntu-speak its priority for pam-auth-update needs to be higher than unix's 256 - pam_unix would then automatically gain try_first_pass). This would print the GLOME URL and a Password: prompt that would accept the tag. If the tag does not validate, PAM would continue with pam_unix and check the token as a password.
• This would of course namespace collide tags and passwords, but I think that's an acceptable tradeoff here. Users who do not want password validation can deactivate the password in /etc/shadow and pam_unix will DTRT.
• My understanding is that such a setup would work with regular terminal-like setups and even SSH - although very likely not with screensavers as the URL would not be displayed properly in that case. Note that the default timeout for login on Debian/Ubuntu is 60s - which is a little short for our protocol. We might want to document raising this through /etc/login.defs.
• We should have a minimal CLI that actually works with the glome-login protocol rather than just GLOME. The current CLI is kinda weird - it does not even generate the hex keys necessary for glome-login, so you first need to convert them away from binary.
• We could go with glome login sign/tag - or glome-login sign, I don't really care. But it also turns out that we do not have URL decomposition in C yet, which would be somewhat useful to feed URLs into. Basically I think it'd be great to have a CLI that takes the private key file as an argument and parses everything from the /v1/ to the end and generates the appropriate tag to login with. Additionally there should be commands alike wg genkey and wg pubkey that DTRT. Then we'd be at the same technical level as Wireguard, where you still need to put those keys into the appropriate places in the config file - although we'd still ship an example config with the keys commented out. Technically this could also be written in a language different from C. I think from a distro point of view I'd weakly prefer it to be Python or C rather than Go.
• We might need to decide if glome-login is worth keeping for more minimal environments without PAM.

cc @burgerdev @bluecmd @vvidic @l9i

Implementing a PAM module for GLOME would allow for an easier adoption of GLOME.

Starting glome-login with the reboot username immediately reboots the machine without asking for any authentication.

Not sure if this is expected or not?

We should have clang-format at least for glome-login

User friendliness of the main README should be improved before the first release:

• Provide tl;dr that explain the goal of the project
• Illustrate how GLOME Login could be used (e.g., with a hypothetical scenario)
• Add instructions on how to use GLOME Login

Both go/login and go/login/server would ejoy having a README.md file that guide the user through the code.

The private keys produced by glome genkey (and the corresponding public keys from glome pubkey) are x25519 keys, i.e. 32 raw octets.

Key Exchange

Dealing with GLOME keys is inconvenient: transferring a public key to another system requires either a direct file transfer (e.g. scp) or an intermediate encoding into the printable range (e.g. base64). SSH keys, for comparison, are PEM encoded and can be easily copied or included in an email. Furthermore, given only the key file it is impossible to determine its nature, which could lead to accidental confusion of private and public keys. In the best case, you just cannot determine whether the 32B file in your home directory created two years ago is a GLOME key or something else.

Encoding Logic

All keys in the spec are hex-encoded, as are the keys in tests and for glome-login. The lack of encoding in the GLOME library causes a proliferation of encoding logic elsewhere, as can be seen by the definition of a decode_hex in glome_test.c, cli/commands.c and login/ui.c. Since parsing and deserialization logic is a common source of security vulnerabilities, it would be better to have one central piece of logic for dealing with encoding that is well tested.

Protocol Variants

The keys do not specify which protocol variant they are to be used with. If the two parties disagree on the variant to be used we would at least expect mismatching tags, while a deliberately introduced confusion by a person-in-the-middle could ostensibly lead to more severe outcomes.

Fold the changes from the RfD into the protocol documentation.

A tiny nit I've had with glome-login for a while (this behavior has been here for 1+ year): after I enter my authentication code and I get logged in, it seems like there is a spurious newline still in the input buffer which causes my shell to give me two prompts:

smol login: root
Obtain the one-time authorization code from:
<snip>
> <snip>
Authorization code: OK

[root@smol:~]# 

[root@smol:~]# 

The range of acceptable config files for g_key_file_load_from_file is limited to 'regular' files (as determined by S_ISREG), unfortunately excluding a bunch of legitimate config sources:

$ ./build/login/glome-login -c /dev/null root
ERROR: config file could not be read: Not a regular file

Error: parse-config
$ ./build/login/glome-login -c <(echo -e "[service]\nkey = fee1dead...") root
ERROR: config file could not be read: Not a regular file

Error: parse-config

It's slightly confusing that the CLI prints and verifies tags in hex format, while the tag is exchanged base64-encoded in the login protocol. base64 has arguably better utility support and would allow using the cli as client without re-encoding, so I propose changing the output of glome tag to base64.

As pointed out by @burgerdev, Key list should be HTML output with a <table>, or at least configurable to be raw text or html using the Accept header for it, rather than a query parameter.

The glome-cli demo conveniently uses Alice and Bob, respectively 5 and 3 characters. Replacing Alice with test ends up showing some spurious errors due to trying to parse test as a hex key:

$ glome-cli test
ERROR while parsing byte 0 ('te') as hex
1f8b8245ddbe65db2e593683fbc4596d0859cbdd2885a20708478880c57c8e4e

Things still work, but it's confusing.

We should have yapf-format and test checking for pyglome.

Requirements files should be used to describe the complete Python environment necessary. This includes the environment needed to test the package. The CI invocation should then just install the requirements from the file.

Backwards-compatibility on the C code is not necessary as long as the server/authorizer supports the v2 protocol, so we can drop v1 in the same PR.

PyGLOME should be installable using pip.