Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cookbook: Add "Locked-down Temporal" example #1367

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Feb 17, 2021

A partial example illustrating how to deploy a Temporal object that
doesn't leak any information about the host system's clock, time zone,
locale data, or time zone data.

Run in a separate process from the other cookbook examples.

Closes: #603

@ptomato
Copy link
Collaborator Author

ptomato commented Feb 17, 2021

@gibson042 Do you think anyone that you've been in contact with about virtualizability might be interested in this example?

@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #1367 (42a5e2d) into main (d51828d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1367   +/-   ##
=======================================
  Coverage   95.36%   95.36%           
=======================================
  Files          19       19           
  Lines       11123    11123           
  Branches     1826     1826           
=======================================
  Hits        10607    10607           
  Misses        511      511           
  Partials        5        5           
Flag Coverage Δ
test262 48.45% <ø> (ø)
tests 91.06% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d51828d...942c31a. Read the comment docs.

Comment on lines +23 to +26
// Override the Temporal.Calendar constructor and Temporal.Calendar.from to
// disallow all calendars except the iso8601 calendar, otherwise insecure code
// might be able to tell something about the version of the host system's
// locale data.
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed? if Temporal.now is locked down, then how can insecure code access the host system's data?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The version of the locale data present on the host system is a browser fingerprinting vector. If you can construct dates using non-ISO calendars and convert them to ISO, then you may be able to tell (due to bugs being fixed, or adjustments to calendars based on astronomical sightings) which version of locale data is present.

Copy link
Member

Choose a reason for hiding this comment

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

i see, so this example is primarily about avoiding fingerprinting? that's usually not something a web site author wants to prevent, since they generally want to use that data for analytics.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This example is a grab bag of modifications that are somewhat related. The blurb I wrote in cookbook-mock.md says that you wouldn't need all of them for every purpose. For example, for a test harness, you wouldn't bother with blocking access to the time zones and calendars, and for an environment like SES, you wouldn't bother with the controllable clock.

Copy link
Member

Choose a reason for hiding this comment

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

gotcha.

this is way better than nothing, to be sure :-) but it might be less confusing as separate examples, targeted to a single use case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair enough. I'll see what would be the best way to split it up after getting comments from others.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think comments like this serve the purpose of pointing out the use case of / reasons for each part - no need to further split up imo as there aren't that many things it locks down (clock precision & time, locale). Perhaps adding a bit more to the markdown intro about preventing clock & locale fingerprinting would be sufficient.

Copy link
Collaborator

@cjtenny cjtenny left a comment

Choose a reason for hiding this comment

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

Would revise some comments, but overall lgtm as an example of controlling exposed info & temporal surface for various purposes. 🙂


"Lock down" the Temporal object so that it doesn't leak any information about the host system, and the system clock is controllable, for use in security applications or for mocking in tests.

→ [Locked-down Temporal](cookbook-mock.md)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as #1370 (comment), non-blocking, just curious as it's a departure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Answered there, tl;dr: this one is longer and less general-purpose.

Not everything in this example is needed for every application.
For example, in a test harness, you would probably only need to replace `Temporal.now` with a version using a controllable clock and constant time zone, and not need to freeze the Temporal object, or replace `Function.prototype.toString`.

> **NOTE**: This is a very specialized use of Temporal and is not something you would normally need to do.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should also state that it is also not 100% guaranteed to not leak information about the host system; differences in performance of underlying temporal operations can still leak as a side channel (e.g. inform on the host VM implementation). It's a (good) example that doesn't claim to be perfectly secure or complete!

Per discussion below, should clarify that this example illustrates shadowing locale data, controllable clock time & precision, and freezing Temporal; it does not consider additional vectors.

Comment on lines +23 to +26
// Override the Temporal.Calendar constructor and Temporal.Calendar.from to
// disallow all calendars except the iso8601 calendar, otherwise insecure code
// might be able to tell something about the version of the host system's
// locale data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think comments like this serve the purpose of pointing out the use case of / reasons for each part - no need to further split up imo as there aren't that many things it locks down (clock precision & time, locale). Perhaps adding a bit more to the markdown intro about preventing clock & locale fingerprinting would be sufficient.

A partial example illustrating how to deploy a Temporal object that
doesn't leak any information about the host system's clock, time zone,
locale data, or time zone data.

Run in a separate process from the other cookbook examples.

Closes: #603
@ptomato ptomato force-pushed the 603-cookbook-locked-down-temporal branch from 54404da to 942c31a Compare February 22, 2021 18:44
@ptomato
Copy link
Collaborator Author

ptomato commented Feb 22, 2021

I revised the text in cookbook-mock.md with the intent of making the purpose of this example clearer, please take another look.

Would still be good to get a review from @gibson042 or someone else who has been considering Temporal in the context of SES.

@ptomato ptomato added the no-spec-text PR can be ignored by implementors label Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-spec-text PR can be ignored by implementors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cookbook example of locked-down Temporal object
3 participants