Add documentation for Unix domain socket authenticator #57

joechrisellis · 2020-08-07T12:29:17Z

No description provided.

ionut-arm

There are some more places where adding/modifying documentation is needed:

System architecture page - the auth tokens section
Interfaces and dataflow page - the dataflow diagram needs updating, same for the authenticator subsection
I think we'll need some stuff added to the threat model, but that can be done in another PR

ionut-arm · 2020-08-10T09:52:14Z

src/parsec_service/authenticators.md

@@ -7,4 +7,15 @@ named](https://github.com/parallaxsecond/parsec-interface-rs/issues/22) "simple
 the code, directly parse the authentication field as a UTF-8 string and uses that as application
 identity. The direct authenticator is the one currently used by the Parsec service.


You should probably remove the last phrase from here

ionut-arm · 2020-08-10T12:51:23Z

src/parsec_service/authenticators.md

+
+The Unix domain socket authenticator uses Unix peer credentials to authenticate the user. Unix peer
+credentials provide direct access to the (effective) user ID on the other end of a domain socket
+connect, without cooperation from the endpoint.


connect - > connection?

Also, without cooperation from the endpoint sounds a bit forceful, maybe something about it relying on the OS for this information?

ionut-arm · 2020-08-10T12:51:58Z

src/parsec_service/authenticators.md

+credentials provide direct access to the (effective) user ID on the other end of a domain socket
+connect, without cooperation from the endpoint.
+
+To use this authenticator, the application must self-declare its UID in the authentication request


in the authentication request -> in the authentication field of the request

ionut-arm · 2020-08-10T16:50:32Z

Changes in the API overview page are also needed, in the Authentication and Sessions section.

ionut-arm

Looks good, but I think you've only updated the .drawio diagram, not the actual PNG that is displayed on the page

paulhowardarm

Looks good overall. Suggest more precision when describing how to populate the auth field. Feels a bit vague as it is. Also our messaging around the auth mechanism vs. the transport seems slightly misleading to me.

paulhowardarm · 2020-08-11T15:22:54Z

src/parsec_service/authenticators.md

+successful and the application identity is set to the UID.
+
+Currently, the peer credentials authenticator only supports authentication via the peer credentials
+sourced from a Unix domain socket. However, the authenticator will likely support different


Not sure about the phrasing here. It seems like the reverse of what we intend. Peer credential specifically requires domain socket to be the transport, because we rely on the ability of UDS to be able to query the credential in a trusted manner. I think it's misleading to say that peer credential would support other transports in the future, unless we're willing to significantly generalise our definition of what a peer credential is (in which case I don't think we can specifically talk about Unix UIDs or GIDs).

It would be more accurate to say the reverse: that a UDS transport could be used with other auth mechanisms and not only peer credential.

That might've been my fault, as I suggested expanding what we assume - the definition of "peer credential" would indeed depend on the transport mechanism, but I don't see an issue in assuming that other IPC mechanisms (e.g. network, d-bus) could be covered with this kind of authentication if we point out that only UDS are supported for now.

Makes sense -- removed mentions of other transports for this section.

paulhowardarm · 2020-08-11T15:27:51Z

src/parsec_client/api_overview.md

@@ -177,6 +187,10 @@ permitted numerical values for this field are given as follows:-
 - A value of 2 (`0x02`) indicates authentication tokens. The service will expect the
   **authentication** field to contain a JWT token. Tokens must be signed with the private key of
   the identity provider and their validity period must cover the moment when the check is done.
+- A value of 3 (`0x03`) indicates peer credentials authentication. The service will expect the
+   **authentication** field to contain the stringified UID of the connecting process. The Parsec


"stringified uid" feels a bit vague here considering this section is supposed to be the specification of how a client process should populate the auth field. We don't mention that it has to be UTF8 (although that is mentioned somewhere further down, but I think it needs to be here). We also probably want to be very explicit that we expect a UID and not a user name. Also, since UIDs are numerical, we probably want to specify what number format we expect. Decimal? Hex? Will the service parse conventional notations such as the 0x prefix for hex numbers? This is the level of precision that I think we need here.

Good point -- I've changed this to:

- A value of 3 (`0x03`) indicates peer credentials authentication. The service will expect the **authentication** field to contain the non-padded decimal UID (**not** username) of the connecting process as a UTF-8 string (e.g. `123`). The Parsec service will verify that this self-declared UID is consistent with the UID from the peer credentials.

Please let me know if there's any further suggestions!

I think to answer to your original question about the format, we need to be clear about the scope of the authentictor. Is it a generic peer credentials authenticator which could be used with other operating systems/listener types or a Unix peer credential authenticator?
If the authenticator that we are describing here is only the later then I think we should:

be more precise where we mention "peer credentials" and always say "Unix peer credentials" and make sure that when we say "UID" we mean the specific Unix User identifier

the Unix User Identifier is represented with an integer type. Most Unix OS use 32 bits for this, and Linux uses the uid_t type which is an unsigned 32-bits number. For that reason, I think it would be better to expect the authentication payload to be the representation of a u32 value, 4-bytes padded with zeroes. For example the UID 4 would be [0x00, 0x00, 0x00, 0x04] (or reverse depending on our endianess). I think this is more efficient and less error-prone than having to parse/format a UTF-8 string.

I think to answer to your original question about the format, we need to be clear about the scope of the authentictor. Is it a generic peer credentials authenticator which could be used with other operating systems/listener types or a Unix peer credential authenticator?

AFAIK 'peer credentials' is a Unix-exclusive thing, but I'm all for being more explicit. 😄

be more precise where we mention "peer credentials" and always say "Unix peer credentials" and make sure that when we say "UID" we mean the specific Unix User identifier

ACK, will do.

the Unix User Identifier is represented with an integer type. Most Unix OS use 32 bits for this, and Linux uses the uid_t type which is an unsigned 32-bits number. For that reason, I think it would be better to expect the authentication payload to be the representation of a u32 value, 4-bytes padded with zeroes. For example the UID 4 would be [0x00, 0x00, 0x00, 0x04] (or reverse depending on our endianess). I think this is more efficient and less error-prone than having to parse/format a UTF-8 string.

Agreed, that's a lot nicer. Will do. 🙂

hug-dev

Thanks, that looks good to me 😃 One comment about the PNG

hug-dev · 2020-09-08T16:51:05Z

src/parsec_service/diagrams/interfaces_and_dataflow.drawio

Thanks for updating the diagram as well! Could you also please keep the white background? I believe that in the png you exported from draw.io the background is transparent. Just so that it is easier to read on our book 😃!

I just checked the other .drawio files in the repository and they all seem to have transparent backgrounds as well? Am I missing something?

Actually, you're right -- the .drawio files do have transparent background, but I had selected the 'export with transparent background' option.

hug-dev

Thanks for the changes, looks good to me!!

ionut-arm

👌

ionut-arm · 2020-09-09T13:13:31Z

src/parsec_service/authenticators.md

+The GID component of the Unix peer credentials is currently unused by the peer credentials
+authenticator.


Gonna guess the PID isn't used either.

Signed-off-by: Joe Ellis <[email protected]>

joechrisellis requested review from hug-dev and ionut-arm as code owners August 7, 2020 12:29

joechrisellis force-pushed the uds_authenticator_docs branch from acfc7d3 to 32b1b08 Compare August 7, 2020 13:41

ionut-arm reviewed Aug 10, 2020

View reviewed changes

joechrisellis force-pushed the uds_authenticator_docs branch from 32b1b08 to 99c622b Compare August 11, 2020 10:01

ionut-arm mentioned this pull request Aug 11, 2020

Add documentation for authentication additions #58

Closed

ionut-arm reviewed Aug 11, 2020

View reviewed changes

joechrisellis force-pushed the uds_authenticator_docs branch from 99c622b to ffe6908 Compare August 11, 2020 14:45

ionut-arm requested a review from paulhowardarm August 11, 2020 15:02

paulhowardarm reviewed Aug 11, 2020

View reviewed changes

joechrisellis force-pushed the uds_authenticator_docs branch from ffe6908 to 3fb63a0 Compare August 11, 2020 16:04

ionut-arm previously approved these changes Aug 13, 2020

View reviewed changes

joechrisellis dismissed ionut-arm’s stale review via 93ed107 August 17, 2020 08:14

joechrisellis force-pushed the uds_authenticator_docs branch from 3fb63a0 to 93ed107 Compare August 17, 2020 08:14

joechrisellis force-pushed the uds_authenticator_docs branch from 93ed107 to 3ee327b Compare September 8, 2020 14:38

hug-dev reviewed Sep 8, 2020

View reviewed changes

hug-dev mentioned this pull request Sep 8, 2020

Add Unix peer credentials authenticator parallaxsecond/parsec#214

Merged

joechrisellis force-pushed the uds_authenticator_docs branch 2 times, most recently from 134dc83 to 1ea2cb8 Compare September 9, 2020 09:44

hug-dev approved these changes Sep 9, 2020

View reviewed changes

ionut-arm approved these changes Sep 9, 2020

View reviewed changes

Add documentation for Unix domain socket authenticator

97b1a68

Signed-off-by: Joe Ellis <[email protected]>

joechrisellis force-pushed the uds_authenticator_docs branch from 1ea2cb8 to 97b1a68 Compare September 9, 2020 13:18

hug-dev merged commit 23e8872 into parallaxsecond:master Sep 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add documentation for Unix domain socket authenticator #57

Add documentation for Unix domain socket authenticator #57

joechrisellis commented Aug 7, 2020

ionut-arm left a comment

ionut-arm Aug 10, 2020

ionut-arm Aug 10, 2020

ionut-arm Aug 10, 2020

ionut-arm commented Aug 10, 2020

ionut-arm left a comment

paulhowardarm left a comment

paulhowardarm Aug 11, 2020

ionut-arm Aug 11, 2020

joechrisellis Aug 11, 2020

paulhowardarm Aug 11, 2020

joechrisellis Aug 11, 2020

hug-dev Sep 8, 2020

joechrisellis Sep 9, 2020

hug-dev left a comment

hug-dev Sep 8, 2020

joechrisellis Sep 9, 2020

joechrisellis Sep 9, 2020

hug-dev left a comment

ionut-arm left a comment

ionut-arm Sep 9, 2020

		@@ -7,4 +7,15 @@ named](https://github.com/parallaxsecond/parsec-interface-rs/issues/22) "simple
		the code, directly parse the authentication field as a UTF-8 string and uses that as application
		identity. The direct authenticator is the one currently used by the Parsec service.

		The GID component of the Unix peer credentials is currently unused by the peer credentials
		authenticator.

Add documentation for Unix domain socket authenticator #57

Add documentation for Unix domain socket authenticator #57

Conversation

joechrisellis commented Aug 7, 2020

ionut-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ionut-arm commented Aug 10, 2020

ionut-arm left a comment

Choose a reason for hiding this comment

paulhowardarm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hug-dev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hug-dev left a comment

Choose a reason for hiding this comment

ionut-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment