Skip to content

Conversation

@ArtificialOwl
Copy link
Contributor

@ArtificialOwl ArtificialOwl commented Aug 21, 2024

This is an attempt to add an way to confirm the origin of an incoming request when running an OCM request to ensure both party known each others

Signing Request

Signing request allows to confirm the identity of the sender.
Signing request does not encrypt nor affect its payload.
Signing request only adds metadata to the headers of the request.

Discovery

A signatory containing a keyId and the public key is added to the OCM discovery

{
  "enabled": true,
  "endPoint": "...",
  "resourceTypes": [
    [...]
  ],
  "protocols": {
    [...]
  },
  "publicKey": {
    "keyId": "https://cloud.mydomain.com/ocm#signature",
    "publicKeyPem": "[...]"
  }
}

Signature

The concept is to add unique metadata and sign them using a private/public key pair.
The location of the public key used to verify the signature will confirm the origin of the request.

Signature does not affect the data of the request, it only adds headers to it:

 {
     "(request-target)": "post /path",
     "content-length": 380,
     "date": "Mon, 08 Jul 2024 14:16:20 GMT",
     "digest": "SHA-256=U7gNVUQiixe5BRbp4Tg0xCZMTcSWXXUZI2\\/xtHM40S0=",
     "host": "hostname.of.the.recipient",
     "Signature": "keyId=\"https://author.hostname/key\",algorithm=\"ras-sha256\",headers=\"content-length date digest host\",signature=\"DzN12OCS1rsA[...]o0VmxjQooRo6HHabg==\""
 }
'content-length' is the total length of the data/content
'date' is the datetime the request have been initiated
'digest' is a checksum of the data/content
'host' is the hostname of the recipient of the request (remote when signing outgoing request, local on incoming request)
'Signature' contains the signature generated using the private key, and metadata:
    'keyId' is a unique id, formatted as an url. hostname is used to retrieve the public key via custom discovery
    'algorithm' define the algorithm used to generate signature
    'headers' contains a list of element used during the generation of the signature
    'signature' is the encrypted string, using local private key, of an array containing elements
    listed in 'headers' and their value. Some elements (content-length date digest host) are mandatory
    to ensure authenticity override protection.

@ArtificialOwl
Copy link
Contributor Author

And an implementation of the feature on Nextcloud: nextcloud/server#45979

@glpatcern
Copy link
Member

Hi there, interesting proposal indeed, it would nicely complement the recent additions related to discovering the remote end.

Could you please patch the spec.yaml file with the required snippet in the discovery endpoint? I'd add the publicKey section at the top level of the returned JSON though, not as one of the resourceTypes.

@ArtificialOwl
Copy link
Contributor Author

formatted result of the edit in spec.yaml

image

@michielbdejong
Copy link
Contributor

Great idea! Side note:
If we want to make OCM compatible with GNAP (see #98), we should require the information advertised via .well-known to match the client details described in section 2.3 of GNAP

@michielbdejong
Copy link
Contributor

Also, in GNAP I just read:

A client instance that is capable of talking to multiple AS's SHOULD
use a different key for each AS to prevent a class of mix-up attacks
as described in Section 13.31 unless other mechanisms can be used to
assure the identity of the AS for a given request.

I wonder if/how that would apply to OCM.

GNAP "client instance" = OCM receiving server
GNAP "Authorization Server (AS)" = OCM sending server

Copy link
Member

@glpatcern glpatcern left a comment

Choose a reason for hiding this comment

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

Hi there, thanks for the extra contribution. I went through the proposal in more details and apart some minor fixes/rephrasing, there are a couple of questions we need to address before proceeding.

@MahdiBaghbani
Copy link
Member

I've something to add here, it's from the ActivityPub,

Signing requests using HTTP Signatures
Server to server federation via inbox delivery POST requests is authenticated using HTTP Signatures in conjunction with the signing key from the actor's publicKey field. The keyId should link to the actor so that the publicKey field can be retrieved. At minimum, the digest field should be included in the set of headers being signed.

So the actors are the users on the server, and each user has a key pair. The requests are signed with the private key of the user initiating the request (and not the server).

There is also an Instance Actor which is the server itself.

@michielbdejong
Copy link
Contributor

We should merge this into the spec as optional, great stuff!
I implemented it in OcmStub for both sending and receiving.

@glpatcern
Copy link
Member

I concur this is a most welcome development to be merged to the spec.

I also linked #23 from a few years back as it suggests exactly the same thing.

@michielbdejong
Copy link
Contributor

michielbdejong commented Sep 3, 2024

An RFC came out in February, looks like this is intended to replace the I-D from 2019?

michielbdejong and others added 9 commits September 4, 2024 09:08
Co-authored-by: Giuseppe Lo Presti <[email protected]>
Co-authored-by: Giuseppe Lo Presti <[email protected]>
Co-authored-by: Giuseppe Lo Presti <[email protected]>
Co-authored-by: Giuseppe Lo Presti <[email protected]>
Co-authored-by: Giuseppe Lo Presti <[email protected]>
Co-authored-by: Giuseppe Lo Presti <[email protected]>
Co-authored-by: Giuseppe Lo Presti <[email protected]>
@michielbdejong michielbdejong self-requested a review September 4, 2024 07:19
Copy link
Member

@mickenordin mickenordin left a comment

Choose a reason for hiding this comment

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

This looks good to me now.

Copy link
Member

@glpatcern glpatcern left a comment

Choose a reason for hiding this comment

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

Only a few minor fixes remaining.

Otherwise, I don't think it's good to have language-specific code examples in the spec. If you want to move forward let's merge this as is, but please open an issue that the example on how to perform the signing and the verification should be done in a more abstract way, especially without language-specific primitives such as implode() (Let's say up to base64_encode(hash('sha256', utf8_encode($payload))) is fine!).

Co-authored-by: Giuseppe Lo Presti <[email protected]>
@michielbdejong
Copy link
Contributor

If you want to move forward let's merge this as is

Thanks! Yes, let's merge this now and then I'll add my own ideas in a follow-up PR

@michielbdejong michielbdejong merged commit b691b67 into cs3org:develop Sep 4, 2024
@ArtificialOwl
Copy link
Contributor Author

sorry, was overhelmed with work in the last few weeks, not even able to free me for those weekly calls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security proposal: trusted services and shared secrets / request signing

6 participants