From 5a5b72abc7db0b86ead92c01addda56dccd0fb58 Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Thu, 25 Apr 2024 09:02:30 +0200 Subject: [PATCH 1/9] feat: use seekable in download --- sda-download/api/sda/sda.go | 64 +++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/sda-download/api/sda/sda.go b/sda-download/api/sda/sda.go index 7e5856cfe..e97744b73 100644 --- a/sda-download/api/sda/sda.go +++ b/sda-download/api/sda/sda.go @@ -272,23 +272,18 @@ func Download(c *gin.Context) { return } - switch c.Param("type") { - case "encrypted": - // calculate coordinates - start, end, err = calculateEncryptedCoords(start, end, c.GetHeader("Range"), fileDetails) - if err != nil { - log.Errorf("Byte range coordinates invalid! %v", err) + wholeFile := true + if start != 0 || end != 0 { + wholeFile = false + } - return - } - if start > 0 { - // reading from an offset in encrypted file is not yet supported - log.Errorf("Start coordinate for encrypted files not implemented! %v", start) - c.String(http.StatusBadRequest, "Start coordinate for encrypted files not implemented!") + start, end, err = calculateCoords(start, end, c.GetHeader("Range"), fileDetails, c.Param("type")) + if err != nil { + log.Errorf("Byte range coordinates invalid! %v", err) - return - } - default: + return + } + if c.Param("type") != "encrypted" { // set the content-length for unencrypted files if start == 0 && end == 0 { c.Header("Content-Length", fmt.Sprint(fileDetails.DecryptedSize)) @@ -299,11 +294,6 @@ func Download(c *gin.Context) { } } - wholeFile := true - if start != 0 || end != 0 { - wholeFile = false - } - // Get archive file handle var file io.Reader @@ -355,6 +345,7 @@ func Download(c *gin.Context) { } // Prepare the file for streaming, encrypted or decrypted + var fileStream io.Reader switch c.Param("type") { @@ -387,13 +378,15 @@ func Download(c *gin.Context) { fileStream = io.MultiReader(newHr, file) } else { seeker, _ := file.(io.ReadSeeker) - fileStream, err = storage.SeekableMultiReader(newHr, seeker) + seekStream, err := storage.SeekableMultiReader(newHr, seeker) if err != nil { log.Errorf("Failed to construct SeekableMultiReader, reason: %v", err) c.String(http.StatusInternalServerError, "file decoding error") return } + start, end, err = seekStart(seekStream, start, end) + fileStream = seekStream } default: // Reencrypt header for use with our temporary key @@ -428,16 +421,7 @@ func Download(c *gin.Context) { return } - if start != 0 { - // We don't want to read from start, skip ahead to where we should be - _, err = c4ghfileStream.Seek(start, 0) - if err != nil { - log.Errorf("error occurred while finding sending start: %v", err) - c.String(http.StatusInternalServerError, "an error occurred") - - return - } - } + start, end, err = seekStart(c4ghfileStream, start, end) fileStream = c4ghfileStream } @@ -450,6 +434,24 @@ func Download(c *gin.Context) { } } +var seekStart = func(fileStream io.ReadSeeker, start, end int64) (int64, int64, error) { + if start != 0 { + + // We don't want to read from start, skip ahead to where we should be + _, err := fileStream.Seek(start, 0) + if err != nil { + + return 0, 0, fmt.Errorf("error occurred while finding sending start: %v", err) + } + // adjust end to reflect that the file start has been moved + end -= start + start = 0 + + } + + return start, end, nil +} + // used from: https://github.com/neicnordic/crypt4gh/blob/master/examples/reader/main.go#L48C1-L113C1 var sendStream = func(reader io.Reader, writer http.ResponseWriter, start, end int64) error { From c3445660e04d1f0db796dea572a5e1027f37ab8f Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Thu, 25 Apr 2024 09:03:11 +0200 Subject: [PATCH 2/9] remove server public key header --- sda-download/api/sda/sda.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sda-download/api/sda/sda.go b/sda-download/api/sda/sda.go index e97744b73..efec010d1 100644 --- a/sda-download/api/sda/sda.go +++ b/sda-download/api/sda/sda.go @@ -327,7 +327,6 @@ func Download(c *gin.Context) { // set the user and server public keys that is send from htsget log.Debugf("Got to setting the headers: %s", c.GetHeader("client-public-key")) c.Header("Client-Public-Key", c.GetHeader("Client-Public-Key")) - c.Header("Server-Public-Key", c.GetHeader("Server-Public-Key")) } if c.Request.Method == http.MethodHead { From 8dee11f73d2aa8b41fe1621b457bb178340120a2 Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Thu, 25 Apr 2024 09:03:50 +0200 Subject: [PATCH 3/9] calculate additional header bytes for htsget --- sda-download/api/sda/sda.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/sda-download/api/sda/sda.go b/sda-download/api/sda/sda.go index efec010d1..ad2dca6fd 100644 --- a/sda-download/api/sda/sda.go +++ b/sda-download/api/sda/sda.go @@ -331,13 +331,20 @@ func Download(c *gin.Context) { if c.Request.Method == http.MethodHead { + // Create headers for htsget, containing size of the crypt4gh header + reencKey := c.GetHeader("Client-Public-Key") + headerSize := bytes.NewReader(fileDetails.Header).Size() + // Size of the header in the archive + c.Header("Server-Additional-Bytes", fmt.Sprint(headerSize)) + if reencKey != "" { + newHeader, _ := reencryptHeader(fileDetails.Header, reencKey) + headerSize = bytes.NewReader(newHeader).Size() + // Size of the header if the file is re-encrypted before downloading + c.Header("Client-Additional-Bytes", fmt.Sprint(headerSize)) + } if c.Param("type") == "encrypted" { - c.Header("Content-Length", fmt.Sprint(fileDetails.ArchiveSize)) - - // set the length of the crypt4gh header for htsget - c.Header("Server-Additional-Bytes", fmt.Sprint(bytes.NewReader(fileDetails.Header).Size())) - // TODO figure out if client crypt4gh header will have other size - // c.Header("Client-Additional-Bytes", ...) + // Update the content length to match the encrypted file size + c.Header("Content-Length", fmt.Sprint(int(headerSize)+fileDetails.ArchiveSize)) } return From 0283401f5243e3c7f8c5ac8f4933f9b965971d67 Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Thu, 25 Apr 2024 09:04:14 +0200 Subject: [PATCH 4/9] stop using user agent header --- sda-download/api/sda/sda.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sda-download/api/sda/sda.go b/sda-download/api/sda/sda.go index ad2dca6fd..b8d94a445 100644 --- a/sda-download/api/sda/sda.go +++ b/sda-download/api/sda/sda.go @@ -358,11 +358,8 @@ func Download(c *gin.Context) { case "encrypted": // The key provided in the header should be base64 encoded reencKey := c.GetHeader("Client-Public-Key") - if strings.HasPrefix(c.GetHeader("User-Agent"), "htsget") { - reencKey = c.GetHeader("Server-Public-Key") - } if reencKey == "" { - c.String(http.StatusBadRequest, "c4gh public key is mmissing from the header") + c.String(http.StatusBadRequest, "c4gh public key is missing from the header") return } From 0b36d61d816fecb4b91ba4dedd6cf7c31296489f Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Thu, 25 Apr 2024 09:04:53 +0200 Subject: [PATCH 5/9] make byte header range inclusive --- sda-download/api/sda/sda.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/sda-download/api/sda/sda.go b/sda-download/api/sda/sda.go index b8d94a445..a4e9dff56 100644 --- a/sda-download/api/sda/sda.go +++ b/sda-download/api/sda/sda.go @@ -503,9 +503,10 @@ var sendStream = func(reader io.Reader, writer http.ResponseWriter, start, end i } // Calculates the start and end coordinats to use. If a range is set in HTTP headers, -// it will be used as is. If not, the functions parameters will be used, -// and adjusted to match the data block boundaries of the encrypted file. -var calculateEncryptedCoords = func(start, end int64, htsget_range string, fileDetails *database.FileDownload) (int64, int64, error) { +// it will be used as is. If not, the functions parameters will be used. +// If in encrypted mode, the parameters will be adjusted to match the data block boundaries. +var calculateCoords = func(start, end int64, htsget_range string, fileDetails *database.FileDownload, encryptedType string) (int64, int64, error) { + log.Warnf("calculate") if htsget_range != "" { startEnd := strings.Split(strings.TrimPrefix(htsget_range, "bytes="), "-") if len(startEnd) > 1 { @@ -521,9 +522,17 @@ var calculateEncryptedCoords = func(start, end int64, htsget_range string, fileD return 0, 0, fmt.Errorf("endCoordinate must be greater than startCoordinate") } - return a, b, nil + // Byte ranges are inclusive; +1 so that the last byte is included + + return a, b + 1, nil } } + + // For unencrypted files, return the coordinates as is + if encryptedType != "encrypted" { + return start, end, nil + } + // Adapt end coordinate to follow the crypt4gh block boundaries headlength := bytes.NewReader(fileDetails.Header) bodyEnd := int64(fileDetails.ArchiveSize) @@ -536,4 +545,5 @@ var calculateEncryptedCoords = func(start, end int64, htsget_range string, fileD } return start, headlength.Size() + bodyEnd, nil + } From 092cf1c1a9406824ad0af3dc3f4dc12fde1b9293 Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Thu, 25 Apr 2024 09:05:11 +0200 Subject: [PATCH 6/9] tests: update download tests --- sda-download/api/sda/sda_test.go | 36 +++++++++++++++++++------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/sda-download/api/sda/sda_test.go b/sda-download/api/sda/sda_test.go index 8c3e966b7..fd9ddaa5c 100644 --- a/sda-download/api/sda/sda_test.go +++ b/sda-download/api/sda/sda_test.go @@ -491,9 +491,14 @@ func TestDownload_Fail_OpenFile(t *testing.T) { return fileDetails, nil } - // Mock request and response holders + // Mock request and response holders and initialize headers w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) + req := &http.Request{ + URL: &url.URL{}, + Header: make(http.Header), + } + c.Request = req // Test the outcomes of the handler Download(c) @@ -520,7 +525,7 @@ func TestDownload_Fail_OpenFile(t *testing.T) { } -func TestEncrypted_Coords(t *testing.T) { +func Test_CalucalateCoords(t *testing.T) { var to, from int64 from, to = 0, 1000 fileDetails := &database.FileDownload{ @@ -529,41 +534,44 @@ func TestEncrypted_Coords(t *testing.T) { Header: make([]byte, 124), } - // htsget_range should be used first and as is + // htsget_range should be used first and its end position should be increased by one headerSize := bytes.NewReader(fileDetails.Header).Size() fullSize := headerSize + int64(fileDetails.ArchiveSize) - start, end, err := calculateEncryptedCoords(from, to, "bytes=10-20", fileDetails) + var endPos int64 + endPos = 20 + start, end, err := calculateCoords(from, to, "bytes=10-"+strconv.FormatInt(endPos, 10), fileDetails, "default") assert.Equal(t, start, int64(10)) - assert.Equal(t, end, int64(20)) + assert.Equal(t, end, endPos+1) assert.NoError(t, err) // end should be greater than or equal to inputted end - _, end, err = calculateEncryptedCoords(from, to, "", fileDetails) + _, end, err = calculateCoords(from, to, "", fileDetails, "encrypted") assert.GreaterOrEqual(t, end, from) assert.NoError(t, err) // end should not be smaller than a header - _, end, err = calculateEncryptedCoords(from, headerSize-10, "", fileDetails) + _, end, err = calculateCoords(from, headerSize-10, "", fileDetails, "encrypted") assert.GreaterOrEqual(t, end, headerSize) assert.NoError(t, err) // end should not be larger than file length + header - _, end, err = calculateEncryptedCoords(from, fullSize+1900, "", fileDetails) + _, end, err = calculateCoords(from, fullSize+1900, "", fileDetails, "encrypted") assert.Equal(t, fullSize, end) assert.NoError(t, err) - // range 0-0 should give whole file - start, end, err = calculateEncryptedCoords(0, 0, "", fileDetails) + // param range 0-0 should give whole file + start, end, err = calculateCoords(0, 0, "", fileDetails, "encrypted") assert.Equal(t, end-start, fullSize) assert.NoError(t, err) - // range 0-0 with range in the header should return the range size - _, end, err = calculateEncryptedCoords(0, 0, "bytes=0-1000", fileDetails) - assert.Equal(t, end, int64(1000)) + // byte range 0-1000 should return the range size, end coord inclusive + endPos = 1000 + _, end, err = calculateCoords(0, 0, "bytes=0-"+strconv.FormatInt(endPos, 10), fileDetails, "encrypted") + assert.Equal(t, end, endPos+1) assert.NoError(t, err) // range in the header should return error if values are not numbers - _, _, err = calculateEncryptedCoords(0, 0, "bytes=start-end", fileDetails) + _, _, err = calculateCoords(0, 0, "bytes=start-end", fileDetails, "encrypted") assert.Error(t, err) } From 7ad8c1aa7b0c1c9baeb709006471925693070618 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Thu, 25 Apr 2024 11:00:22 +0200 Subject: [PATCH 7/9] Fix tls config --- sda-download/api/api.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sda-download/api/api.go b/sda-download/api/api.go index 1b200ed20..f0cdc5de5 100644 --- a/sda-download/api/api.go +++ b/sda-download/api/api.go @@ -53,11 +53,6 @@ func Setup() *http.Server { log.Info("(3/5) Configuring TLS") cfg := &tls.Config{ MinVersion: tls.VersionTLS12, - CurvePreferences: []tls.CurveID{tls.CurveP521, tls.CurveP384, tls.CurveP256}, - PreferServerCipherSuites: true, - CipherSuites: []uint16{ - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - }, } // Configure web server From 4372d3ec7d95bd9451b90ba07a7e51e07790a22f Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Fri, 26 Apr 2024 12:54:54 +0200 Subject: [PATCH 8/9] linter, check error --- sda-download/api/api.go | 2 +- sda-download/api/sda/sda.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/sda-download/api/api.go b/sda-download/api/api.go index f0cdc5de5..d947f0c80 100644 --- a/sda-download/api/api.go +++ b/sda-download/api/api.go @@ -52,7 +52,7 @@ func Setup() *http.Server { // Configure TLS settings log.Info("(3/5) Configuring TLS") cfg := &tls.Config{ - MinVersion: tls.VersionTLS12, + MinVersion: tls.VersionTLS12, } // Configure web server diff --git a/sda-download/api/sda/sda.go b/sda-download/api/sda/sda.go index a4e9dff56..a54001ff0 100644 --- a/sda-download/api/sda/sda.go +++ b/sda-download/api/sda/sda.go @@ -389,6 +389,12 @@ func Download(c *gin.Context) { return } start, end, err = seekStart(seekStream, start, end) + if err != nil { + log.Errorf("Could not seek stream: %v", err) + c.String(http.StatusInternalServerError, "file decoding error") + + return + } fileStream = seekStream } default: @@ -425,6 +431,12 @@ func Download(c *gin.Context) { return } start, end, err = seekStart(c4ghfileStream, start, end) + if err != nil { + log.Errorf("Could not seek stream: %v", err) + c.String(http.StatusInternalServerError, "file decoding error") + + return + } fileStream = c4ghfileStream } From 468848378f87ce5ff92692864ef622a6f7113184 Mon Sep 17 00:00:00 2001 From: MalinAhlberg Date: Fri, 26 Apr 2024 13:56:32 +0200 Subject: [PATCH 9/9] refactor: rename function --- sda-download/api/sda/sda.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sda-download/api/sda/sda.go b/sda-download/api/sda/sda.go index a54001ff0..92d62f538 100644 --- a/sda-download/api/sda/sda.go +++ b/sda-download/api/sda/sda.go @@ -388,7 +388,7 @@ func Download(c *gin.Context) { return } - start, end, err = seekStart(seekStream, start, end) + start, end, err = adjustSeekPos(seekStream, start, end) if err != nil { log.Errorf("Could not seek stream: %v", err) c.String(http.StatusInternalServerError, "file decoding error") @@ -430,7 +430,7 @@ func Download(c *gin.Context) { return } - start, end, err = seekStart(c4ghfileStream, start, end) + start, end, err = adjustSeekPos(c4ghfileStream, start, end) if err != nil { log.Errorf("Could not seek stream: %v", err) c.String(http.StatusInternalServerError, "file decoding error") @@ -449,7 +449,7 @@ func Download(c *gin.Context) { } } -var seekStart = func(fileStream io.ReadSeeker, start, end int64) (int64, int64, error) { +var adjustSeekPos = func(fileStream io.ReadSeeker, start, end int64) (int64, int64, error) { if start != 0 { // We don't want to read from start, skip ahead to where we should be