allow SameSite cookie attribute to be configured by pdurbin · Pull Request #11210 · IQSS/dataverse

pdurbin · 2025-02-03T20:49:19Z

What this PR does / why we need it:

This PR sets the SameSite cookie attribute to "Lax", which is more secure than "None" and documents how to change it to "Strict".

Which issue(s) this PR closes:

Closes https://github.com/IQSS/dataverse-security/issues/27

Special notes for your reviewer:

None.

Suggestions on how to test this:

Confirm the former and new attributes for the JSESSIONID cookie's attributes.

Follow the docs and try to change the SameSite value. Changing the SameSite value to "Strict" for example is more easily done in a classic environment than in Docker because "Lax" is baked into the base image. It's possible to change the value and rebuild the base image. The file to edit is modules/container-base/src/main/docker/Dockerfile. Then, to rebuild, you follow https://guides.dataverse.org/en/6.5/container/base-image.html#build-instructions

Here's "before" and "after":

Before: SameSite attribute is absent (Dataverse 6.5 and earlier)

% curl -s -I http://localhost:8080 | grep JSESSIONID
Set-Cookie: JSESSIONID=6574324d75aebeb86dc96ecb3bb0; Path=/

After: SameSite attribute is present (twice) (this pull request)

Note: we suspect that the repeated SameSite attribute is a bug in Payara. We should retest after we've upgraded Payara in #11128 before reporting it upstream.

% curl -s -I http://localhost:8080 | grep JSESSIONID
Set-Cookie: JSESSIONID=6574324d75aebeb86dc96ecb3bb0; Path=/;SameSite=Lax;SameSite=Lax

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No.

Is there a release notes update needed for this change?:

Yes, included.

Additional documentation:

Preview docs at https://dataverse-guide--11210.org.readthedocs.build/en/11210/installation/config.html#samesite-cookie-attribute

IQSS/dataverse-security#27

doc/release-notes/ds27-samesite-attr.md

qqmyers · 2025-02-05T22:03:16Z

doc/sphinx-guides/source/installation/config.rst

+
+See :ref:`samesite-cookie-attribute` for context.
+
+The Dataverse installer configures the Payara **server-config.network-config.protocols.protocol.http-listener-1.http.cookie-same-site-value** setting to "Lax". From `Payara's documentation <https://docs.payara.fish/community/docs/6.2024.6/Technical%20Documentation/Payara%20Server%20Documentation/General%20Administration/Administering%20HTTP%20Connectivity.html>`_, the other possible values are "Strict" or "None". To change this to "Strict", for example, you could run the following command...


The value for this appears to do nothing if the http.cookie-same-site-enabled isn't set to true.

Conversely, setting http.cookie-same-site-enabled=true without setting this one ends up sending None as the default which is worse than not having a SameSite setting at all. I think the docs should warn people about these issues.

I didn't end up documenting this (or testing it) but I'm happy to. Let me know.

qqmyers

The bin/asadmin work fine on an existing installation. I did suggest some doc changes to clarify that installations are not less secure now that our recommendation of going to Lax, and to stress that using None on purpose, or accidentally increases risks.

Also - not sure why the build failed - from the log, it doesn't look related to the PR to me.

qqmyers · 2025-02-06T19:24:43Z

Another note - I think Strict breaks remote login - at least ORCID which is what I tested. Lax works as expected.

pdurbin · 2025-02-11T21:20:47Z

doc/sphinx-guides/source/container/base-image.rst

      - See :ref:`:ApplicationServerSettings` ``http.request-timeout-seconds``.

        *Note:* can also be set using any other `MicroProfile Config Sources`_ available via ``dataverse.http.timeout``.
+    * - ``DATAVERSE_COOKIE_SAME_SITE_VALUE``


When I try to change DATAVERSE_COOKIE_SAME_SITE_VALUE to "Strict" in the compose file, it doesn't seem to have an effect. I still see "Lax" when I check the cookie with curl.

I opened a thread in Zulip about this: https://dataverse.zulipchat.com/#narrow/channel/375812-containers/topic/DATAVERSE_HTTP_TIMEOUT.20not.20having.20an.20effect/near/499094581

I'm not sure why it isn't working. 🤷 I'm hoping @poikilotherm will save me! ❤️

Did you also set DATAVERSE_COOKIE_SAME_SITE_ENABLED?

Well, it's already set.

Through the Dockerfile, I mean.

Hold on, I need to add something to the Dockerfile.

Bah. It doesn't work. As discussed at standup, I switched Docker to be hard coded to Lax and updated the docs to say you have to rebuild the base image if you want Strict or whatever. See d62ac2f

qqmyers

Looks good. Odd that SameSite is repeated in curl. FWIW: if I look at the first call to Dataverse in the Chrome or Edge console, the Set-Cookie entry in the response header only shows one copy (even selecting 'raw' for the response headers). Perhaps they are de-duplicating even for raw?

Also explain that we'd like it to be configurable through MPCONFIG but it doesn't seem to be working.

ofahimIQSS · 2025-03-07T18:16:21Z

Hello! Can you please update this PR with the latest from Dev. Thanks!

pdurbin · 2025-03-07T18:55:48Z

Sure, done!

ofahimIQSS · 2025-03-07T19:16:31Z

Adding Testing Evidences:

Performed smoke test, everything seems to operate as usual.

ofahimIQSS · 2025-03-07T19:17:37Z

Looks good. Odd that SameSite is repeated in curl.

What Jim mentioned, any way we can fix that or shall we proceed as is. @pdurbin

ofahimIQSS · 2025-03-09T15:53:40Z

Integration Tests are failing - please advise:

coveralls · 2025-03-09T20:35:59Z

coverage: 22.643%. remained the same
when pulling 7a9c25b on ds27-samesite-attr
into 0af33b8 on develop.

pdurbin · 2025-03-10T11:43:10Z

@ofahimIQSS I merged the latest from develop to pick up this fix:

MailTest Fix #11319

Now all tests are passing.

ofahimIQSS · 2025-03-10T12:48:59Z

Merging PR

pdurbin · 2025-03-14T20:42:21Z

Note: we suspect that the repeated SameSite attribute is a bug in Payara. We should retest after we've upgraded Payara in #11128 before reporting it upstream.

I just reported it upstream:

Set-Cookie shows duplicate SameSite=Lax;SameSite=Lax payara/Payara#7254

pdurbin added 3 commits February 3, 2025 15:47

allow SameSite cookie attribute to be configured

72b04fe

IQSS/dataverse-security#27

typo #11210

27b7fc4

add release note #11210

3c286ca

pdurbin added Size: 10 A percentage of a sprint. 7 hours. FY25 Sprint 16 FY25 Sprint 16 (2025-01-29 - 2025-02-12) labels Feb 3, 2025

qqmyers reviewed Feb 5, 2025

View reviewed changes

doc/release-notes/ds27-samesite-attr.md Outdated Show resolved Hide resolved

qqmyers reviewed Feb 5, 2025

View reviewed changes

qqmyers requested changes Feb 5, 2025

View reviewed changes

pdurbin assigned pdurbin and qqmyers Feb 6, 2025

pdurbin mentioned this pull request Feb 10, 2025

Can't build base image: Error status (125) #11239

Closed

pdurbin unassigned qqmyers Feb 11, 2025

improve release notes and docs

a81cacb

pdurbin assigned qqmyers and unassigned pdurbin Feb 11, 2025

pdurbin commented Feb 11, 2025

View reviewed changes

qqmyers approved these changes Feb 11, 2025

View reviewed changes

pdurbin assigned pdurbin and unassigned qqmyers Feb 12, 2025

pdurbin added 2 commits February 12, 2025 11:28

hard code Docker image to Lax for SameSite Cookie attribute

d62ac2f

Also explain that we'd like it to be configurable through MPCONFIG but it doesn't seem to be working.

whoops, fix link to base image doc

48de9a6

pdurbin removed their assignment Feb 12, 2025

cmbz added the FY25 Sprint 17 FY25 Sprint 17 (2025-02-12 - 2025-02-26) label Feb 12, 2025

ofahimIQSS self-assigned this Feb 19, 2025

cmbz added this to the 6.6 milestone Feb 26, 2025

cmbz added the FY25 Sprint 18 FY25 Sprint 18 (2025-02-26 - 2025-03-12) label Feb 27, 2025

pdurbin added the Component: Containers Anything related to cloudy Dataverse, shipped in containers. label Mar 6, 2025

Merge branch 'develop' into ds27-samesite-attr

cca3dcb

Merge branch 'develop' into ds27-samesite-attr

7a9c25b

ofahimIQSS merged commit bd24a24 into develop Mar 10, 2025
12 checks passed

github-project-automation bot moved this from QA ✅ to Merged 🚀 in IQSS Dataverse Project Mar 10, 2025

ofahimIQSS deleted the ds27-samesite-attr branch March 10, 2025 12:49

ofahimIQSS removed their assignment Mar 10, 2025

scolapasta moved this from Merged 🚀 to Done 🧹 in IQSS Dataverse Project Mar 10, 2025

pdurbin mentioned this pull request Mar 14, 2025

Set-Cookie shows duplicate SameSite=Lax;SameSite=Lax payara/Payara#7254

Open


		See :ref:`samesite-cookie-attribute` for context.

		The Dataverse installer configures the Payara server-config.network-config.protocols.protocol.http-listener-1.http.cookie-same-site-value setting to "Lax". From `Payara's documentation <https://docs.payara.fish/community/docs/6.2024.6/Technical%20Documentation/Payara%20Server%20Documentation/General%20Administration/Administering%20HTTP%20Connectivity.html>`_, the other possible values are "Strict" or "None". To change this to "Strict", for example, you could run the following command...

Conversation

pdurbin commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before: SameSite attribute is absent (Dataverse 6.5 and earlier)

After: SameSite attribute is present (twice) (this pull request)

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qqmyers left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qqmyers commented Feb 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qqmyers left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ofahimIQSS commented Mar 7, 2025

Uh oh!

pdurbin commented Mar 7, 2025

Uh oh!

ofahimIQSS commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ofahimIQSS commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ofahimIQSS commented Mar 9, 2025

Uh oh!

coveralls commented Mar 9, 2025

Uh oh!

pdurbin commented Mar 10, 2025

Uh oh!

ofahimIQSS commented Mar 10, 2025

Uh oh!

Uh oh!

pdurbin commented Mar 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pdurbin commented Feb 3, 2025 •

edited

Loading

qqmyers left a comment •

edited

Loading

qqmyers left a comment •

edited

Loading

ofahimIQSS commented Mar 7, 2025 •

edited

Loading

ofahimIQSS commented Mar 7, 2025 •

edited

Loading