From d47364dedb3b8f549ec88ca6edd97389e57bbabc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 31 May 2024 15:30:01 +0200 Subject: [PATCH 1/6] feat: [WIP] use proof keys to ensure requests come from trusted source Basic code handling is in the proofkeys package. The idea is to implement it as a middleware and hook it after the wopicontext middleware. Note that we need a way to switch this middleware off in case there are problems with the verification, even though it should be active by default. --- .../collaboration/pkg/proofkeys/handler.go | 216 ++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 services/collaboration/pkg/proofkeys/handler.go diff --git a/services/collaboration/pkg/proofkeys/handler.go b/services/collaboration/pkg/proofkeys/handler.go new file mode 100644 index 000000000..c2f135a55 --- /dev/null +++ b/services/collaboration/pkg/proofkeys/handler.go @@ -0,0 +1,216 @@ +package proofkeys + +import ( + "bytes" + "crypto" + "crypto/rsa" + "crypto/sha256" + "crypto/tls" + "encoding/base64" + "errors" + "math/big" + "net/http" + "strings" + + "github.com/beevik/etree" + "github.com/owncloud/ocis/v2/ocis-pkg/log" +) + +type PubKeys struct { + Key *rsa.PublicKey + OldKey *rsa.PublicKey +} + +type Verifier interface { + Verify(accessToken, url, timestamp, sig64, oldSig64 string) error +} + +type VerifyHandler struct { + discoveryURL string + insecure bool + logger log.Logger +} + +func NewVerifyHandler(discoveryURL string, insecure bool, logger log.Logger) Verifier { + return &VerifyHandler{ + discoveryURL: discoveryURL, + insecure: insecure, + logger: logger, + } +} + +// Verify the request comes from a trusted source +// All the provided parameters are strings: +// * accessToken: The access token used for this request (targeting this collaboration service) +// * url: The full url for this request, including scheme, host and all query parameters, +// something like "https://wopiserver.test.private/wopi/file/abcbcbd?access_token=oiuiu" or +// "http://wopiserver:8888/wopi/file/abcdef?access_token=zzxxyy" +// * timestamp: The timestamp provided by the WOPI app in the "X-WOPI-TimeStamp" header, as string +// * sig64: The base64-encoded signature, which should come directly from the "X-WOPI-Proof" header +// * olSig64: The base64-encoded previous signature, coming from the "X-WOPI-ProofOld" header +// +// The public keys will be obtained from the /hosting/discovery path of the target WOPI app. +// Note that the method will perform the following checks in that order: +// * current signature with the current key +// * old signature with the current key +// * current signature with the old key +// If all of those checks are wrong, the method will fail, and the request should be rejected. +// +// The method will return an error if something fails, or nil if everything is ok +func (vh *VerifyHandler) Verify(accessToken, url, timestamp, sig64, oldSig64 string) error { + // need to decode the signatures + signature, err := base64.StdEncoding.DecodeString(sig64) + if err != nil { + return err + } + + var oldSignature []byte + if oldSig64 != "" { + if oldSig, err := base64.StdEncoding.DecodeString(oldSig64); err != nil { + return nil + } else { + oldSignature = oldSig + } + } + + // fetch the public keys + pubkeys, err := vh.fetchPublicKeys() + if err != nil { + return err + } + + // build and hash the expected proof + expectedProof := vh.generateProof(accessToken, url, timestamp) + hashedProof := sha256.Sum256(expectedProof) + + // verify + if err := rsa.VerifyPKCS1v15(pubkeys.Key, crypto.SHA256, hashedProof[:], signature); err != nil { + if err := rsa.VerifyPKCS1v15(pubkeys.Key, crypto.SHA256, hashedProof[:], oldSignature); err != nil { + if pubkeys.OldKey != nil { + return rsa.VerifyPKCS1v15(pubkeys.OldKey, crypto.SHA256, hashedProof[:], signature) + } else { + return err + } + } + } + return nil +} + +// generateProof will generated a expected proof to be verified later. +// The method will return a slice of bytes with the proof (consider it binary +// data). +// The bytes will need to be hashed later in order to perform the verification +func (vh *VerifyHandler) generateProof(accessToken, url, timestamp string) []byte { + tokenBytes := []byte(accessToken) + tokenLen := len(tokenBytes) + tokenLenBytes := big.NewInt(int64(tokenLen)).FillBytes(make([]byte, 4)) + + // url needs to be uppercase + urlBytes := []byte(strings.ToUpper(url)) + urlLen := len(urlBytes) + urlLenBytes := big.NewInt(int64(urlLen)).FillBytes(make([]byte, 4)) + + stampBigInt, _ := new(big.Int).SetString(timestamp, 10) + stampBytes := stampBigInt.FillBytes(make([]byte, 8)) + stampLen := len(stampBytes) + stampLenBytes := big.NewInt(int64(stampLen)).FillBytes(make([]byte, 4)) + + proof := new(bytes.Buffer) + proof.Write(tokenLenBytes) + proof.Write(tokenBytes) + proof.Write(urlLenBytes) + proof.Write(urlBytes) + proof.Write(stampLenBytes) + proof.Write(stampBytes) + return proof.Bytes() +} + +// fetchPublicKeys will fetch the public keys from the /hosting/discovery URL +// of the provided WOPI app. +// It will return a PubKeys struct to hold the public keys based on the modulus +// and exponent found. +// The PubKeys returned might be either nil (with the non-nil error), or might +// contain only a PubKeys.Key field (the PubKeys.OldKey might be nil) +func (vh *VerifyHandler) fetchPublicKeys() (*PubKeys, error) { + httpClient := http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: vh.insecure, + }, + }, + } + + httpResp, err := httpClient.Get(vh.discoveryURL) + if err != nil { + vh.logger.Error(). + Err(err). + Str("WopiAppUrl", vh.discoveryURL). + Msg("WopiDiscovery: failed to access wopi app url") + return nil, err + } + + defer httpResp.Body.Close() + + if httpResp.StatusCode != http.StatusOK { + vh.logger.Error(). + Str("WopiAppUrl", vh.discoveryURL). + Int("HttpCode", httpResp.StatusCode). + Msg("WopiDiscovery: wopi app url failed with unexpected code") + return nil, err + } + + doc := etree.NewDocument() + if _, err := doc.ReadFrom(httpResp.Body); err != nil { + return nil, err + } + + root := doc.SelectElement("wopi-discovery") + if root == nil { + return nil, errors.New("wopi-discovery element not found in the XML body") + } + + proofKey := root.SelectElement("proof-key") + if proofKey == nil { + return nil, errors.New("proof-key element not found in the XML body") + } + + mod64 := proofKey.SelectAttrValue("modulus", "") + exp64 := proofKey.SelectAttrValue("exponent", "") + oldMod64 := proofKey.SelectAttrValue("oldmodulus", "") + oldExp64 := proofKey.SelectAttrValue("oldexponent", "") + + if mod64 == "" || exp64 == "" { + return nil, errors.New("modulus or exponent not found in the proof-key element") + } + + keys := &PubKeys{ + Key: vh.keyFromBase64(mod64, exp64), + } + + if oldMod64 != "" && oldExp64 != "" { + keys.OldKey = vh.keyFromBase64(oldMod64, oldExp64) + } + + return keys, nil +} + +// keyFromBase64 will create a rsa public key from the provided modulus and +// exponent, both encoded with base64. +// If any of the provided strings can't be decoded, nil will be returned. +func (vh *VerifyHandler) keyFromBase64(mod64, exp64 string) *rsa.PublicKey { + dataMod, err := base64.StdEncoding.DecodeString(mod64) + if err != nil { + return nil + } + + dataE, err := base64.StdEncoding.DecodeString(exp64) + if err != nil { + return nil + } + + pub := &rsa.PublicKey{ + N: new(big.Int).SetBytes(dataMod), + E: int(new(big.Int).SetBytes(dataE).Int64()), + } + return pub +} From 36d9a8c4252f4355d9d9be5fc7a44b7c117091cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 11 Jun 2024 18:39:53 +0200 Subject: [PATCH 2/6] feat: proof keys verification for the collaboration service --- services/collaboration/pkg/config/app.go | 7 ++ .../pkg/config/defaults/defaultconfig.go | 4 + .../collaboration/pkg/middleware/proofkeys.go | 55 ++++++++++ .../pkg/middleware/wopicontext.go | 3 + .../collaboration/pkg/proofkeys/handler.go | 101 +++++++++++++++--- .../collaboration/pkg/proofkeys/option.go | 35 ++++++ .../collaboration/pkg/server/http/server.go | 7 ++ 7 files changed, 195 insertions(+), 17 deletions(-) create mode 100644 services/collaboration/pkg/middleware/proofkeys.go create mode 100644 services/collaboration/pkg/proofkeys/option.go diff --git a/services/collaboration/pkg/config/app.go b/services/collaboration/pkg/config/app.go index e255f21e8..ee569189a 100644 --- a/services/collaboration/pkg/config/app.go +++ b/services/collaboration/pkg/config/app.go @@ -9,4 +9,11 @@ type App struct { Addr string `yaml:"addr" env:"COLLABORATION_APP_ADDR" desc:"The URL where the WOPI app is located, such as https://127.0.0.1:8080." introductionVersion:"6.0.0"` Insecure bool `yaml:"insecure" env:"COLLABORATION_APP_INSECURE" desc:"Skip TLS certificate verification when connecting to the WOPI app" introductionVersion:"6.0.0"` + + ProofKeys ProofKeys `yaml:"proofkeys"` +} + +type ProofKeys struct { + Disable bool `yaml:"disable" env:"COLLABORATION_APP_PROOF_DISABLE" desc:"Disable the proof keys verification" introductionVersion:"6.0.0"` + Duration string `yaml:"duration" env:"COLLABORATION_APP_PROOF_DURATION" desc:"Duration for the proof keys to be cached in memory, using time.ParseDuration format. If the duration can't be parsed, we'll use the default 12h as duration" introductionVersion:"6.0.0"` } diff --git a/services/collaboration/pkg/config/defaults/defaultconfig.go b/services/collaboration/pkg/config/defaults/defaultconfig.go index 1fd0e4e71..19e5618f2 100644 --- a/services/collaboration/pkg/config/defaults/defaultconfig.go +++ b/services/collaboration/pkg/config/defaults/defaultconfig.go @@ -26,6 +26,10 @@ func DefaultConfig() *config.Config { LockName: "com.github.owncloud.collaboration", Addr: "https://127.0.0.1:9980", Insecure: false, + ProofKeys: config.ProofKeys{ + // they'll be enabled by default + Duration: "12h", + }, }, GRPC: config.GRPC{ Addr: "127.0.0.1:9301", diff --git a/services/collaboration/pkg/middleware/proofkeys.go b/services/collaboration/pkg/middleware/proofkeys.go new file mode 100644 index 000000000..be9f92675 --- /dev/null +++ b/services/collaboration/pkg/middleware/proofkeys.go @@ -0,0 +1,55 @@ +package middleware + +import ( + "net/http" + "net/url" + "time" + + "github.com/owncloud/ocis/v2/services/collaboration/pkg/config" + "github.com/owncloud/ocis/v2/services/collaboration/pkg/proofkeys" + "github.com/rs/zerolog" +) + +func ProofKeysMiddleware(cfg *config.Config, next http.Handler) http.Handler { + wopiDiscovery := cfg.App.Addr + "/hosting/discovery" + insecure := cfg.App.Insecure + cacheDuration, err := time.ParseDuration(cfg.App.ProofKeys.Duration) + if err != nil { + cacheDuration = 12 * time.Hour + } + + pkHandler := proofkeys.NewVerifyHandler(wopiDiscovery, insecure, cacheDuration) + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + logger := zerolog.Ctx(r.Context()) + + // the url we need is the one being requested, but we need the + // scheme and host, so we'll get those from the configured WOPISrc + wopiSrcURL, _ := url.Parse(cfg.Wopi.WopiSrc) + currentURL, _ := url.Parse(r.URL.String()) + currentURL.Scheme = wopiSrcURL.Scheme + currentURL.Host = wopiSrcURL.Host + + accessToken := r.URL.Query().Get("access_token") + stamp := r.Header.Get("X-WOPI-TimeStamp") + + err := pkHandler.Verify( + accessToken, + currentURL.String(), + stamp, + r.Header.Get("X-WOPI-Proof"), + r.Header.Get("X-WOPI-ProofOld"), + proofkeys.VerifyWithLogger(logger), + ) + // Need to check that the timestamp was sent within the last 20 minutes + + if err != nil { + logger.Error().Err(err).Msg("ProofKeys verification failed") + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + + logger.Debug().Msg("ProofKeys verified") + + next.ServeHTTP(w, r) + }) +} diff --git a/services/collaboration/pkg/middleware/wopicontext.go b/services/collaboration/pkg/middleware/wopicontext.go index e9998c0d7..d1b73e437 100644 --- a/services/collaboration/pkg/middleware/wopicontext.go +++ b/services/collaboration/pkg/middleware/wopicontext.go @@ -90,6 +90,9 @@ func WopiContextAuthMiddleware(jwtSecret string, next http.Handler) http.Handler logger := zerolog.Ctx(ctx) ctx = logger.With(). Str("WopiOverride", r.Header.Get("X-WOPI-Override")). + Str("WopiProof", r.Header.Get("X-WOPI-Proof")). + Str("WopiProofOld", r.Header.Get("X-WOPI-ProofOld")). + Str("WopiStamp", r.Header.Get("X-WOPI-TimeStamp")). Str("FileReference", claims.WopiContext.FileReference.String()). Str("ViewMode", claims.WopiContext.ViewMode.String()). Str("Requester", claims.WopiContext.User.GetId().String()). diff --git a/services/collaboration/pkg/proofkeys/handler.go b/services/collaboration/pkg/proofkeys/handler.go index c2f135a55..36042f07e 100644 --- a/services/collaboration/pkg/proofkeys/handler.go +++ b/services/collaboration/pkg/proofkeys/handler.go @@ -11,31 +11,46 @@ import ( "math/big" "net/http" "strings" + "time" "github.com/beevik/etree" - "github.com/owncloud/ocis/v2/ocis-pkg/log" + "github.com/rs/zerolog" ) type PubKeys struct { - Key *rsa.PublicKey - OldKey *rsa.PublicKey + Key *rsa.PublicKey + OldKey *rsa.PublicKey + ExpireTime time.Time } type Verifier interface { - Verify(accessToken, url, timestamp, sig64, oldSig64 string) error + Verify(accessToken, url, timestamp, sig64, oldSig64 string, opts ...VerifyOption) error } type VerifyHandler struct { discoveryURL string insecure bool - logger log.Logger + cachedKeys *PubKeys + cachedDur time.Duration } -func NewVerifyHandler(discoveryURL string, insecure bool, logger log.Logger) Verifier { +// NewVerifyHandler will return a new Verifier with the provided parameters +// The discoveryURL must point to the https://office.wopi/hosting/discovery +// address, which contains the xml with the proof keys (and more information) +// The insecure parameter can be used to disable certificate verification when +// conecting to the provided discoveryURL +// CachedDur is the duration the keys will be cached in memory. The cached keys +// will be used for the duration provided, after that new keys will be fetched +// from the discoveryURL. +// +// For WOPI apps whose proof keys rotate after a while, you must ensure that +// the provided duration is shorter than the rotation time. This should +// guarantee that we can't fail to verify a request due to obsolete keys. +func NewVerifyHandler(discoveryURL string, insecure bool, cachedDur time.Duration) Verifier { return &VerifyHandler{ discoveryURL: discoveryURL, insecure: insecure, - logger: logger, + cachedDur: cachedDur, } } @@ -47,7 +62,7 @@ func NewVerifyHandler(discoveryURL string, insecure bool, logger log.Logger) Ver // "http://wopiserver:8888/wopi/file/abcdef?access_token=zzxxyy" // * timestamp: The timestamp provided by the WOPI app in the "X-WOPI-TimeStamp" header, as string // * sig64: The base64-encoded signature, which should come directly from the "X-WOPI-Proof" header -// * olSig64: The base64-encoded previous signature, coming from the "X-WOPI-ProofOld" header +// * oldSig64: The base64-encoded previous signature, coming from the "X-WOPI-ProofOld" header // // The public keys will be obtained from the /hosting/discovery path of the target WOPI app. // Note that the method will perform the following checks in that order: @@ -57,7 +72,14 @@ func NewVerifyHandler(discoveryURL string, insecure bool, logger log.Logger) Ver // If all of those checks are wrong, the method will fail, and the request should be rejected. // // The method will return an error if something fails, or nil if everything is ok -func (vh *VerifyHandler) Verify(accessToken, url, timestamp, sig64, oldSig64 string) error { +func (vh *VerifyHandler) Verify(accessToken, url, timestamp, sig64, oldSig64 string, opts ...VerifyOption) error { + verifyOptions := newOptions(opts...) + + // check timestamp + if err := vh.checkTimestamp(timestamp); err != nil { + return err + } + // need to decode the signatures signature, err := base64.StdEncoding.DecodeString(sig64) if err != nil { @@ -73,10 +95,14 @@ func (vh *VerifyHandler) Verify(accessToken, url, timestamp, sig64, oldSig64 str } } - // fetch the public keys - pubkeys, err := vh.fetchPublicKeys() - if err != nil { - return err + pubkeys := vh.cachedKeys + if pubkeys == nil || pubkeys.ExpireTime.Before(time.Now()) { + // fetch the public keys + newpubkeys, err := vh.fetchPublicKeys(verifyOptions.Logger) + if err != nil { + return err + } + pubkeys = newpubkeys } // build and hash the expected proof @@ -96,6 +122,46 @@ func (vh *VerifyHandler) Verify(accessToken, url, timestamp, sig64, oldSig64 str return nil } +// checkTimestamp will check if the provided timestamp is valid. +// The timestamp is valid if it isn't older than 20 minutes (info from +// MS WOPI docs). +// +// Note: the timestamp is based on C# DateTime.UtcNow.Ticks +// "One tick equals 100 nanoseconds. The value of this property represents +// the number of ticks that have elapsed since 12:00:00 midnight, January 1, 0001." +// It is NOT a unix timestamp (current unix timestamp ~1718123417 secs ; +// expected timestamp ~638537195321890000 100-nanosecs) +func (vh *VerifyHandler) checkTimestamp(timestamp string) error { + var stamp, unixBaseStamp, unixStamp, unixStampSec, unixStampNanoSec big.Int + + // set the stamp + _, ok := stamp.SetString(timestamp, 10) + if !ok { + return errors.New("Invalid timestamp") + } + + // 62135596800 seconds from "January 1, 1 AD" to "January 1, 1970 12:00:00 AM" + // need to convert those secs into 100-nanosecs in order to compare the stamp + unixBaseStamp.Mul(big.NewInt(62135596800), big.NewInt(1000*1000*10)) + + // stamp - unixBaseStamp gives us the unix-based timestamp we can use + unixStamp.Sub(&stamp, &unixBaseStamp) + + // divide between 1000*1000*10 to get the seconds and 100-nanoseconds + unixStampSec.DivMod(&unixStamp, big.NewInt(1000*1000*10), &unixStampNanoSec) + + // time package requires nanoseconds (var will be overwritten with the result) + unixStampNanoSec.Mul(&unixStampNanoSec, big.NewInt(100)) + + // both seconds and nanoseconds should be within int64 range + convertedUnixTimestamp := time.Unix(unixStampSec.Int64(), unixStampNanoSec.Int64()) + + if time.Now().After(convertedUnixTimestamp.Add(20 * time.Minute)) { + return errors.New("Timestamp expired") + } + return nil +} + // generateProof will generated a expected proof to be verified later. // The method will return a slice of bytes with the proof (consider it binary // data). @@ -131,7 +197,7 @@ func (vh *VerifyHandler) generateProof(accessToken, url, timestamp string) []byt // and exponent found. // The PubKeys returned might be either nil (with the non-nil error), or might // contain only a PubKeys.Key field (the PubKeys.OldKey might be nil) -func (vh *VerifyHandler) fetchPublicKeys() (*PubKeys, error) { +func (vh *VerifyHandler) fetchPublicKeys(logger *zerolog.Logger) (*PubKeys, error) { httpClient := http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ @@ -142,7 +208,7 @@ func (vh *VerifyHandler) fetchPublicKeys() (*PubKeys, error) { httpResp, err := httpClient.Get(vh.discoveryURL) if err != nil { - vh.logger.Error(). + logger.Error(). Err(err). Str("WopiAppUrl", vh.discoveryURL). Msg("WopiDiscovery: failed to access wopi app url") @@ -152,7 +218,7 @@ func (vh *VerifyHandler) fetchPublicKeys() (*PubKeys, error) { defer httpResp.Body.Close() if httpResp.StatusCode != http.StatusOK { - vh.logger.Error(). + logger.Error(). Str("WopiAppUrl", vh.discoveryURL). Int("HttpCode", httpResp.StatusCode). Msg("WopiDiscovery: wopi app url failed with unexpected code") @@ -184,7 +250,8 @@ func (vh *VerifyHandler) fetchPublicKeys() (*PubKeys, error) { } keys := &PubKeys{ - Key: vh.keyFromBase64(mod64, exp64), + Key: vh.keyFromBase64(mod64, exp64), + ExpireTime: time.Now().Add(vh.cachedDur), } if oldMod64 != "" && oldExp64 != "" { diff --git a/services/collaboration/pkg/proofkeys/option.go b/services/collaboration/pkg/proofkeys/option.go new file mode 100644 index 000000000..86dacefae --- /dev/null +++ b/services/collaboration/pkg/proofkeys/option.go @@ -0,0 +1,35 @@ +package proofkeys + +import ( + "github.com/rs/zerolog" +) + +// VerifyOption defines a single option function. +type VerifyOption func(o *VerifyOptions) + +// VerifyOptions defines the available options for the Verify function. +type VerifyOptions struct { + Logger *zerolog.Logger +} + +// newOptions initializes the available default options. +func newOptions(opts ...VerifyOption) VerifyOptions { + defaultLog := zerolog.Nop() + opt := VerifyOptions{ + //Logger: log.NopLogger(), // use a NopLogger by default + Logger: &defaultLog, // use a NopLogger by default + } + + for _, o := range opts { + o(&opt) + } + + return opt +} + +// VerifyWithLogger provides a function to set the Logger option. +func VerifyWithLogger(val *zerolog.Logger) VerifyOption { + return func(o *VerifyOptions) { + o.Logger = val + } +} diff --git a/services/collaboration/pkg/server/http/server.go b/services/collaboration/pkg/server/http/server.go index 6218d0383..c331d6b67 100644 --- a/services/collaboration/pkg/server/http/server.go +++ b/services/collaboration/pkg/server/http/server.go @@ -120,6 +120,13 @@ func prepareRoutes(r *chi.Mux, options Options) { return colabmiddleware.WopiContextAuthMiddleware(options.Config.Wopi.Secret, h) }) + // check whether we should check for proof keys + if !options.Config.App.ProofKeys.Disable { + r.Use(func(h stdhttp.Handler) stdhttp.Handler { + return colabmiddleware.ProofKeysMiddleware(options.Config, h) + }) + } + r.Get("/", func(w stdhttp.ResponseWriter, r *stdhttp.Request) { adapter.CheckFileInfo(w, r) }) From 7134374a2e67740b31771d169c632d8cc4b51bae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 12 Jun 2024 14:14:44 +0200 Subject: [PATCH 3/6] fix: use int64 instead of bigInt --- .../collaboration/pkg/middleware/proofkeys.go | 1 - .../collaboration/pkg/proofkeys/handler.go | 19 ++++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/services/collaboration/pkg/middleware/proofkeys.go b/services/collaboration/pkg/middleware/proofkeys.go index be9f92675..064e54131 100644 --- a/services/collaboration/pkg/middleware/proofkeys.go +++ b/services/collaboration/pkg/middleware/proofkeys.go @@ -40,7 +40,6 @@ func ProofKeysMiddleware(cfg *config.Config, next http.Handler) http.Handler { r.Header.Get("X-WOPI-ProofOld"), proofkeys.VerifyWithLogger(logger), ) - // Need to check that the timestamp was sent within the last 20 minutes if err != nil { logger.Error().Err(err).Msg("ProofKeys verification failed") diff --git a/services/collaboration/pkg/proofkeys/handler.go b/services/collaboration/pkg/proofkeys/handler.go index 36042f07e..151b72cb4 100644 --- a/services/collaboration/pkg/proofkeys/handler.go +++ b/services/collaboration/pkg/proofkeys/handler.go @@ -10,6 +10,7 @@ import ( "errors" "math/big" "net/http" + "strconv" "strings" "time" @@ -132,29 +133,25 @@ func (vh *VerifyHandler) Verify(accessToken, url, timestamp, sig64, oldSig64 str // It is NOT a unix timestamp (current unix timestamp ~1718123417 secs ; // expected timestamp ~638537195321890000 100-nanosecs) func (vh *VerifyHandler) checkTimestamp(timestamp string) error { - var stamp, unixBaseStamp, unixStamp, unixStampSec, unixStampNanoSec big.Int - // set the stamp - _, ok := stamp.SetString(timestamp, 10) - if !ok { + stamp, err := strconv.ParseInt(timestamp, 10, 64) + if err != nil { return errors.New("Invalid timestamp") } // 62135596800 seconds from "January 1, 1 AD" to "January 1, 1970 12:00:00 AM" // need to convert those secs into 100-nanosecs in order to compare the stamp - unixBaseStamp.Mul(big.NewInt(62135596800), big.NewInt(1000*1000*10)) + unixBaseStamp := int64(62135596800 * 1000 * 1000 * 10) // stamp - unixBaseStamp gives us the unix-based timestamp we can use - unixStamp.Sub(&stamp, &unixBaseStamp) + unixStamp := stamp - unixBaseStamp // divide between 1000*1000*10 to get the seconds and 100-nanoseconds - unixStampSec.DivMod(&unixStamp, big.NewInt(1000*1000*10), &unixStampNanoSec) - - // time package requires nanoseconds (var will be overwritten with the result) - unixStampNanoSec.Mul(&unixStampNanoSec, big.NewInt(100)) + unixStampSec := unixStamp / (1000 * 1000 * 10) + unixStampNanoSec := (unixStamp % (1000 * 1000 * 10)) * 100 // both seconds and nanoseconds should be within int64 range - convertedUnixTimestamp := time.Unix(unixStampSec.Int64(), unixStampNanoSec.Int64()) + convertedUnixTimestamp := time.Unix(unixStampSec, unixStampNanoSec) if time.Now().After(convertedUnixTimestamp.Add(20 * time.Minute)) { return errors.New("Timestamp expired") From 2133785838a14d2fb332c4ce4dde425b90203acf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 12 Jun 2024 18:22:13 +0200 Subject: [PATCH 4/6] fix: disable proofkeys in CI and don't skip validator tests Proofkeys feature can't be tested with the wopi validator for now. The "FakeOffice" doesn't provide the keys in the discovery endpoint, and the wopi validator doesn't seem to send any header with the proof. --- .drone.star | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.drone.star b/.drone.star index 154b1b402..646670531 100644 --- a/.drone.star +++ b/.drone.star @@ -999,6 +999,8 @@ def wopiValidatorTests(ctx, storage, wopiServerType, accounts_hash_difficulty = "COLLABORATION_LOG_LEVEL": "debug", "COLLABORATION_HTTP_ADDR": "0.0.0.0:9300", "COLLABORATION_GRPC_ADDR": "0.0.0.0:9301", + # no proof keys available in the FakeOffice + "COLLABORATION_APP_PROOF_DISABLE": "true", "COLLABORATION_APP_NAME": "FakeOffice", "COLLABORATION_APP_ADDR": "http://fakeoffice:8080", "COLLABORATION_APP_INSECURE": "true", @@ -1025,7 +1027,7 @@ def wopiValidatorTests(ctx, storage, wopiServerType, accounts_hash_difficulty = "export WOPI_SRC=$(cat wopisrc)", "echo $WOPI_SRC", "cd /app", - "/app/Microsoft.Office.WopiValidator -s -t $WOPI_TOKEN -w $WOPI_SRC -l $WOPI_TTL --testgroup %s" % testgroup, + "/app/Microsoft.Office.WopiValidator -t $WOPI_TOKEN -w $WOPI_SRC -l $WOPI_TTL --testgroup %s" % testgroup, ], }) From 02aef4d3081a804b3db5a2a16b9c9260ce333722 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 13 Jun 2024 08:40:46 +0200 Subject: [PATCH 5/6] fix: wrong return value in the verify --- services/collaboration/pkg/proofkeys/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/collaboration/pkg/proofkeys/handler.go b/services/collaboration/pkg/proofkeys/handler.go index 151b72cb4..2ecbd6fe5 100644 --- a/services/collaboration/pkg/proofkeys/handler.go +++ b/services/collaboration/pkg/proofkeys/handler.go @@ -90,7 +90,7 @@ func (vh *VerifyHandler) Verify(accessToken, url, timestamp, sig64, oldSig64 str var oldSignature []byte if oldSig64 != "" { if oldSig, err := base64.StdEncoding.DecodeString(oldSig64); err != nil { - return nil + return err } else { oldSignature = oldSig } From d2f66799214cf185b7bd4d381871091fa0a37992 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 25 Jul 2024 08:56:50 +0200 Subject: [PATCH 6/6] docs: add changelog --- changelog/unreleased/collaboration-proofkeys.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/collaboration-proofkeys.md diff --git a/changelog/unreleased/collaboration-proofkeys.md b/changelog/unreleased/collaboration-proofkeys.md new file mode 100644 index 000000000..15975b73b --- /dev/null +++ b/changelog/unreleased/collaboration-proofkeys.md @@ -0,0 +1,8 @@ +Enhancement: Add support for proof keys for the collaboration service + +Proof keys support will be enabled by default in order to ensure that all +the requests come from a trusted source. +Since proof keys must be set in the WOPI app (OnlyOffice, Collabora...), it's +possible to disable the verification of the proof keys via configuration. + +https://github.com/owncloud/ocis/pull/9366