Aircloak / aircloak

This repository contains the Aircloak Air frontend as well as the code for our Cloak query and anonymization platform
2 stars 0 forks source link

Date interval alignment weirdness discussion. #4613

Open cristianberneanu opened 4 years ago

cristianberneanu commented 4 years ago

@yoid2000 @sebastian FYI:

In #4556 we limited the maximum snapping precision of dates to quarters. This should allow usage of tighter date ranges in queries. But it doesn't mean any quarter-aligned range is allowed. For example: [2020-01-01, 2023-01-01] is allowed, while [2000-01-01, 2003-01-01] isn't.

This is caused by the interval bounds being snapped to an interval size-dependent grid and relative to the 1900-01-01 origin. The behavior becomes worse the larger the range is: [2000-01-01, 9999-12-31] gets aligned to [1900-01-01, 9999-12-31].

yoid2000 commented 4 years ago

As such, this doesn't really solve TF's problem then? At least I think the intent was that any quarter could be used and aligned with the max date (which in this case is 9999-12-31?)

@sebastian ?

cristianberneanu commented 4 years ago

I am not sure what you mean above. The situation is definitely better than before. The date ranges can be tighter now and the max date is included in the range.

Some range alignment weirdness (which is not related to dates per-se) remains. I am not sure how to solve it, except by excluding large date ranges from interval alignment completely and instead only truncating the boundaries to quarters or years. But that might open some new vulnerabilities.

yoid2000 commented 4 years ago

I am not sure what you mean above. The situation is definitely better than before.

This I believe. My question is simply whether we have really solved TF's issue or not.

But with most or all of our recent changes to make date/time more flexible, we are introducing vulnerabilities, though hopefully ones that are not easy to exploit. What @cristianberneanu describes here would be one more such change. I think this change would be relatively safe.

sebastian commented 4 years ago

I am not sure how to solve it, except by excluding large date ranges from interval alignment completely and instead only truncating the boundaries to quarters or years.

What @cristianberneanu describes here would be one more such change. I think this change would be relatively safe.

@cristianberneanu you mean to the nearest whole quarter or year (where the quarters are aligned within a given year?) From a usability perspective it would of course be great.

@yoid2000 should we think this through a bit more thoroughly before jumping the ship? This will be one of those changes that is going to be hard to undo.

cristianberneanu commented 4 years ago

@cristianberneanu you mean to the nearest whole quarter or year (where the quarters are aligned within a given year?)

Yes. Or, to be safer, align the bounds to full years.

sebastian commented 4 years ago

I am fine with either. @yoid2000 let's discuss.

yoid2000 commented 4 years ago

As a special case for dates, I'm fine with aligning either edge to quarters for large ranges. I don't see an easy attack.

But we are indeed really getting into ad-hocery here.

yoid2000 commented 4 years ago

One other thing to point out. This issue has very little discussion history on github. If it was discussed in slack or the support system, then that discussion should have been copied over to github.

sebastian commented 4 years ago

I don't think there is a whole lot more discussion really

cristianberneanu commented 4 years ago

The point of this issue was to discuss a problematic behavior that wasn't discussed as such before. It was inspired by this support issue https://aircloak.atlassian.net/browse/SUP-63 and the resulting changes we made to improve the situation.

sebastian commented 4 years ago

Ok, I suggest we move this to the next release. We should give it some more thought before making a move one way or the other, and we are dangerously close to making the release now.

cristianberneanu commented 4 years ago

@yoid2000 Any new thoughts on this?

sebastian commented 3 years ago

Out of scope for Evergreen