-
Notifications
You must be signed in to change notification settings - Fork 162
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
doc: update timestamp spec #4300
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @JordiSubira)
doc/cryptography/drkey.rst
line 236 at r1 (raw file):
The upper bound for the epoch length is defined to 3 days, looking for a between efficiency and security.
Where does this play a role (besides in the SPAO?)
Maybe we could set a special requirement for the SPAO. DRKey usage in another context (not SPAO) might profit from a more relaxed definition.
doc/cryptography/drkey.rst
line 276 at r1 (raw file):
the CP-PKI infrastructure. The Level 1 key request message contains the ``validTime`` for which the key must be active
Would it be ok to return a key that is active only due to the grace period? I.e., the validTime is outside epoch end but within the grace period?
Otherwise:
validTime
which must be within the key's epoch and ...
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
The granularity must enable to cover the maximum epoch length for DRKey (plus the :ref:`Grace period<drkey-grace>`). At all times, at most two key
I understand what you mean here, but I think it's technically phrased wrongly.
Key-epochs can not overlap. Hence, only one key epoch can be active.
However, two keys could be active at the same time.
At all times, at most two keys can be active due to the grace period; ...
If the expressed timestamp is smaller than the epoch length of the current key, the current key has been used.
If the expressed timestamp is larger than the epoch length of the previous key but not by more than the grace period, the previous key has been used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @fstreun and @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, fstreun wrote…
I understand what you mean here, but I think it's technically phrased wrongly.
Key-epochs can not overlap. Hence, only one key epoch can be active.
However, two keys could be active at the same time.At all times, at most two keys can be active due to the grace period; ...
If the expressed timestamp is smaller than the epoch length of the current key, the current key has been used.
If the expressed timestamp is larger than the epoch length of the previous key but not by more than the grace period, the previous key has been used.
Both of these descriptions refer to the active epochs at the time of receiving the packet on the clock of the receiver. However, the timestamp refers to the clock of the sender, at the time of sending the packet.
For example, if the receiver's clock is a bit behind, and it so receives a packet "from the future", it would wrongly attribute this to the previous epoch.
We mention already that the Timestamp field should be used to filter packets outside of a sensible range around the current time. We can use this range to also help determine the matching epoch, even in the presence of some clock differences:
- The receiver defines an acceptable range of timestamps around the current time; this range must be wide enough to accommodate clock differences between the two end hosts, plus network latency from the sender to the receiver.
- The DRKey epochs (including grace period) overlapping this range are candidates; for each candidate epoch, the receiver checks whether the relative timestamp for this epoch results in a time inside the acceptable range.
- As long as the acceptable range is smaller than the min length of a DRKey epoch (6 minutes, clocks very roughly synchronized), this results in at most one matching epoch.
- If no matching candidate epoch is found, then the packet is discarded (replay).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @fstreun and @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, matzf (Matthias Frei) wrote…
Both of these descriptions refer to the active epochs at the time of receiving the packet on the clock of the receiver. However, the timestamp refers to the clock of the sender, at the time of sending the packet.
For example, if the receiver's clock is a bit behind, and it so receives a packet "from the future", it would wrongly attribute this to the previous epoch.We mention already that the Timestamp field should be used to filter packets outside of a sensible range around the current time. We can use this range to also help determine the matching epoch, even in the presence of some clock differences:
- The receiver defines an acceptable range of timestamps around the current time; this range must be wide enough to accommodate clock differences between the two end hosts, plus network latency from the sender to the receiver.
- The DRKey epochs (including grace period) overlapping this range are candidates; for each candidate epoch, the receiver checks whether the relative timestamp for this epoch results in a time inside the acceptable range.
- As long as the acceptable range is smaller than the min length of a DRKey epoch (6 minutes, clocks very roughly synchronized), this results in at most one matching epoch.
- If no matching candidate epoch is found, then the packet is discarded (replay).
Or perhaps a different way to address this issue of clock differences would be to compare the time resulting from the adding the timestamp value to the start times of the previous, current and next DRKey epochs, and pick the epoch where this result is closest to the current time.
That should lead to the same result as the procedure above, but does not require explicitly specifying a max clock difference (instead, the max acceptable clock difference would be the length of the DRKey epoch).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, matzf (Matthias Frei) wrote…
Or perhaps a different way to address this issue of clock differences would be to compare the time resulting from the adding the timestamp value to the start times of the previous, current and next DRKey epochs, and pick the epoch where this result is closest to the current time.
That should lead to the same result as the procedure above, but does not require explicitly specifying a max clock difference (instead, the max acceptable clock difference would be the length of the DRKey epoch).
I like all of your descriptions for the reconstruction of the absolute timestamp.
Especially the statement that the acceptance range must be smaller than the min length of a DRKey epoch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @fstreun)
doc/cryptography/drkey.rst
line 236 at r1 (raw file):
Previously, fstreun wrote…
Where does this play a role (besides in the SPAO?)
Maybe we could set a special requirement for the SPAO. DRKey usage in another context (not SPAO) might profit from a more relaxed definition.
In a similar vein as for the lower bound, I think it makes sense to define an upper bound. I picked it up as the same maximum validity as for CP AS certificates. We could obviously relax this, but I believe that durations longer than some days would be weird use cases for DRKey.
doc/cryptography/drkey.rst
line 276 at r1 (raw file):
Previously, fstreun wrote…
Would it be ok to return a key that is active only due to the grace period? I.e., the validTime is outside epoch end but within the grace period?
Otherwise:
validTime
which must be within the key's epoch and ...
Done.
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Or perhaps a different way to address this issue of clock differences would be to compare the time resulting from the adding the timestamp value to the start times of the previous, current and next DRKey epochs, and pick the epoch where this result is closest to the current time.
I added a "Absolute time derivation" section and try to link it to the candidate key for further packet process. This follows the last approach regarding adding the Ts to the previous, current and next epochs. Note that I consider the minimum DRKey Epoch length possible, keeping in mind that Epochs lengths can change from one Epoch to the next.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/cryptography/drkey.rst
line 236 at r1 (raw file):
Previously, JordiSubira wrote…
In a similar vein as for the lower bound, I think it makes sense to define an upper bound. I picked it up as the same maximum validity as for CP AS certificates. We could obviously relax this, but I believe that durations longer than some days would be weird use cases for DRKey.
For the use cases that we are currently thinking about, I agree with you.
What about adding a note, like for the lower bound, or combining it with the other note? Stating that for currently unknown use cases, the upper bound could be adjusted.
But I don't have a strong opinion about it.
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, JordiSubira wrote…
Or perhaps a different way to address this issue of clock differences would be to compare the time resulting from the adding the timestamp value to the start times of the previous, current and next DRKey epochs, and pick the epoch where this result is closest to the current time.
I added a "Absolute time derivation" section and try to link it to the candidate key for further packet process. This follows the last approach regarding adding the Ts to the previous, current and next epochs. Note that I consider the minimum DRKey Epoch length possible, keeping in mind that Epochs lengths can change from one Epoch to the next.
It was a bit confusing what the at meant.
Also, it should be stated here what the timestamp actually means (send time).
That at is within the epoch E_x is stated later.
For me, it's a bit weird that we use at to get the corresponding DRKey/epoch because in the derivation of at, the corresponding epoch is already derived.
Suggestion:
In turn, this timestamp MAY be used to compute the absolute time (*at*) value, which corresponds to the time at which the packet was sent.
The section:ref:`Abosulte time derivation<spao-timestamp>` describes the derivation of *at* in detail.
To retrieve the associated DRKey, if:
doc/protocols/authenticator-option.rst
line 184 at r2 (raw file):
:math:`aw := [T-a/2, T +a/2)` (i) We consider the minimum epoch length period as the upper bound for the acceptance windows.
Suggestion:
We consider the minimum DRKey epoch period as the upper bound for the acceptance windows.
doc/protocols/authenticator-option.rst
line 189 at r2 (raw file):
1. Given a time instant *T*, considering: - The current epoch (:math:`E_{i}`) as the one whose time range includes *T*. - The previous epoch (:math:`E_{i-1}`) as the prior epoch to (:math:`E_{i}`).
Suggestion:
- Epoch :math:`E_{i}` as the one whose time range includes *T*.
- Epoch :math:`E_{i-1}` as the prior epoch to :math:`E_{i}`.
- Epoch :math:`E_{i+1}` as the subsequent epoch to :math:`E_{i}`.
doc/protocols/authenticator-option.rst
line 197 at r2 (raw file):
:math:`at_{i-1}`, :math:`at_{i}` and :math:`at_{i+1}`. 3. Given (i) at most one *absolute time* will be within *aw*. *at* is then considered as the time used by the sender in the packet authentication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @fstreun)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, fstreun wrote…
It was a bit confusing what the at meant.
Also, it should be stated here what the timestamp actually means (send time).That at is within the epoch E_x is stated later.
For me, it's a bit weird that we use at to get the corresponding DRKey/epoch because in the derivation of at, the corresponding epoch is already derived.
Done.
doc/protocols/authenticator-option.rst
line 184 at r2 (raw file):
:math:`aw := [T-a/2, T +a/2)` (i) We consider the minimum epoch length period as the upper bound for the acceptance windows.
Done. Decided to keep length instead of period
doc/protocols/authenticator-option.rst
line 197 at r2 (raw file):
:math:`at_{i-1}`, :math:`at_{i}` and :math:`at_{i+1}`. 3. Given (i) at most one *absolute time* will be within *aw*. *at* is then considered as the time used by the sender in the packet authentication.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @fstreun)
doc/protocols/authenticator-option.rst
line 189 at r2 (raw file):
1. Given a time instant *T*, considering: - The current epoch (:math:`E_{i}`) as the one whose time range includes *T*. - The previous epoch (:math:`E_{i-1}`) as the prior epoch to (:math:`E_{i}`).
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, JordiSubira wrote…
Done.
Suggestion:
Assume *at* is within the epoch :math:`E_x`.
- If *Ts* is smaller than the epoch's duration ->
The candidate DRKey is the key whose epoch is :math:`E_{x}`
- Otherwise ->
The candidate DRKey is the key whose epoch is :math:`E_{x-1}`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
The granularity must enable to cover the maximum epoch length for DRKey (plus the :ref:`Grace period<drkey-grace>`). At all times, at most two key
I think this way to define this is more confusing when considering different validity periods. The grace period extension is defined and fixed. Since grace period duration is << min epoch duration, this will never be ambiguous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, JordiSubira wrote…
I think this way to define this is more confusing when considering different validity periods. The grace period extension is defined and fixed. Since grace period duration is << min epoch duration, this will never be ambiguous.
I see.
In any case, I still have some concerns:
First, the description should state that at is within the epoch of :math:E_x
. (Otherwise, the "Otherwise" can be anything)
Second, in my understanding, the grace period of :math:E_x
is within the epoch :math:E_{x+1}
.
Third, I think it's a bit unfortunate to start with the exceptional case.
Fourth, an implementation should anyways not use this derivation to obtain the correct epoch. Instead, it should obtain the epoch when deriving the absolute time at.
Side note: Now I am questioning how fixed the grace period really is. Starting to like again the flag that indicates the usages of the previous epoch's key. With that, the grace
Suggestion:
Assume *at* is within the epoch :math:`E_x`.
- If *at* is within the :ref:`Grace period<drkey-grace>` of :math:`E_{x-1}` AND
*Ts* is greater than the Grace period duration ->
The candidate DRKey is the key whose epoch is :math:`E_{x-1}`
- Otherwise ->
The candidate DRKey is the key whose epoch is :math:`E_{x}`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @fstreun and @JordiSubira)
doc/cryptography/drkey.rst
line 230 at r4 (raw file):
globally set to 6 minutes. The upper bound for the epoch length is defined to 3 days, looking for a
Missing word.
doc/cryptography/drkey.rst
line 276 at r4 (raw file):
the CP-PKI infrastructure. The Level 1 key request message contains the ``validTime`` which must be within the key's epoch
Nit, previous phrasing seemed better; the validTime
specifies the key's epoch, not vice versa.
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
For me, it's a bit weird that we use at to get the corresponding DRKey/epoch because in the derivation of at, the corresponding epoch is already derived.
Fourth, an implementation should anyways not use this derivation to obtain the correct epoch. Instead, it should obtain the epoch when deriving the absolute time at.
+1, describing both together would simplify this.
Side note: Now I am questioning how fixed the grace period really is. Starting to like again the flag that indicates the usages of the previous epoch's key. With that, the grace
Unfinished sentence?
FWIW, I think this scheme also works just fine if the grace period is not fixed. In fact, I think when addressing the concern above (i.e. changing the description to so that at and the epoch are determined in one go), the grace period will no longer be explicitly needed in the description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, matzf (Matthias Frei) wrote…
For me, it's a bit weird that we use at to get the corresponding DRKey/epoch because in the derivation of at, the corresponding epoch is already derived.
Fourth, an implementation should anyways not use this derivation to obtain the correct epoch. Instead, it should obtain the epoch when deriving the absolute time at.
+1, describing both together would simplify this.
Side note: Now I am questioning how fixed the grace period really is. Starting to like again the flag that indicates the usages of the previous epoch's key. With that, the grace
Unfinished sentence?
FWIW, I think this scheme also works just fine if the grace period is not fixed. In fact, I think when addressing the concern above (i.e. changing the description to so that at and the epoch are determined in one go), the grace period will no longer be explicitly needed in the description.
Yes, the scheme would work fine even with a different grace period.
However, the granularity of the timestamp would then only depend on the max DRKey epoch length. Potentially making the scheme a bit simpler, while requiring only one of the reserved bits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @fstreun and @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, fstreun wrote…
Yes, the scheme would work fine even with a different grace period.
However, the granularity of the timestamp would then only depend on the max DRKey epoch length. Potentially making the scheme a bit simpler, while requiring only one of the reserved bits.
How does the flag bit affect the range/granularity of the timestamp? Timestamps in the grace period must be represent-able either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @fstreun and @JordiSubira)
doc/protocols/authenticator-option.rst
line 40 at r4 (raw file):
| Algorithm | Timestamp | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | RSV | Sequence Number |
In an offline discussion some time ago, @fstreun had also suggested to re-arrange the fields in this header as the reserved byte between timestamp and sequence number is rather awkward. Now would still be a good time to change this, especially as we're also mucking around with the timestamp granularity now.
Here's a proposal:
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Timestamp / Sequence Number |
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| | RSV | Algorithm |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Either this, or the other way around, with algorithm + reserverd byte first, not sure if there is any real difference.
With this, a relatively straight forward approach to set the granularity for the timestamp for DRKey; the timestamp value is in nanoseconds, giving a range of ~78 hours. The applications can choose freely how many bits of the timestamp they set based on a clock vs from a counter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, matzf (Matthias Frei) wrote…
How does the flag bit affect the range/granularity of the timestamp? Timestamps in the grace period must be represent-able either way.
With a grace period flag, the timestamp only has to range the max DRKey epoch length.
If the flag is not set, nothing changes.
If the flag is set, simply the DRKey epoch length (of the previous epoch) must be added to the provided timestamp to obtain the epoch's relative timestamp. To derive the correct epoch (out of the three possibilities) the same approach as already described can be used.
Because all epoch starts and ends are known, also the epoch lengths are known. Hence, no new information is required for this approach.
However, with a nanosecond representation
doc/protocols/authenticator-option.rst
line 40 at r4 (raw file):
Previously, matzf (Matthias Frei) wrote…
In an offline discussion some time ago, @fstreun had also suggested to re-arrange the fields in this header as the reserved byte between timestamp and sequence number is rather awkward. Now would still be a good time to change this, especially as we're also mucking around with the timestamp granularity now.
Here's a proposal:
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Timestamp / Sequence Number | + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | | RSV | Algorithm | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Either this, or the other way around, with algorithm + reserverd byte first, not sure if there is any real difference.
With this, a relatively straight forward approach to set the granularity for the timestamp for DRKey; the timestamp value is in nanoseconds, giving a range of ~78 hours. The applications can choose freely how many bits of the timestamp they set based on a clock vs from a counter.
+1, using nanoseconds and leaving it up to the application to use a less accurate clock in combination with a counter to ensure uniqueness.
I also do not see much difference between the two representations. However, I like to give the following argument for the other representation (alg | rsv | ts):
If the timestamp is located after the other fields, to obtain the nanoseconds, only a mask (& operation) is required. If the timestamp is located before the other fields, something like a shift would be required to obtain the nanoseconds.
(Implicitly assuming the 64-bit byte order conversion)
Hence, the (alg | rsv | ts) representation seems to be more straightforward to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @fstreun and @matzf)
doc/cryptography/drkey.rst
line 230 at r4 (raw file):
Previously, matzf (Matthias Frei) wrote…
Missing word.
Done.
doc/cryptography/drkey.rst
line 276 at r4 (raw file):
Previously, matzf (Matthias Frei) wrote…
Nit, previous phrasing seemed better; the
validTime
specifies the key's epoch, not vice versa.
I modified it so that it reads in a simpler way
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, fstreun wrote…
With a grace period flag, the timestamp only has to range the max DRKey epoch length.
If the flag is not set, nothing changes.
If the flag is set, simply the DRKey epoch length (of the previous epoch) must be added to the provided timestamp to obtain the epoch's relative timestamp. To derive the correct epoch (out of the three possibilities) the same approach as already described can be used.
Because all epoch starts and ends are known, also the epoch lengths are known. Hence, no new information is required for this approach.However, with a nanosecond representation
At the moment, I don't see much benefit of having the flag, tbh. I merged both definitions and now it reads more easily and as Matthias mentioned, no need to explicitly mention the grace period, nonetheless, I added a note so that the reader is aware of the previously described situation (if it is too much noise for the reading I will remove it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @fstreun and @matzf)
doc/protocols/authenticator-option.rst
line 40 at r4 (raw file):
Previously, fstreun wrote…
+1, using nanoseconds and leaving it up to the application to use a less accurate clock in combination with a counter to ensure uniqueness.
I also do not see much difference between the two representations. However, I like to give the following argument for the other representation (alg | rsv | ts):
If the timestamp is located after the other fields, to obtain the nanoseconds, only a mask (& operation) is required. If the timestamp is located before the other fields, something like a shift would be required to obtain the nanoseconds.
(Implicitly assuming the 64-bit byte order conversion)Hence, the (alg | rsv | ts) representation seems to be more straightforward to me.
Done. I changed the layout, also merged the description. I do not think there's much difference between any of those, it will depend on the (de-)serializing implementation, both seem fine to me. Anyways I picked ( alg | rsv | ts) because I also like it better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r5.
Reviewable status: 1 of 2 files reviewed, 4 unresolved discussions (waiting on @fstreun and @JordiSubira)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, JordiSubira wrote…
At the moment, I don't see much benefit of having the flag, tbh. I merged both definitions and now it reads more easily and as Matthias mentioned, no need to explicitly mention the grace period, nonetheless, I added a note so that the reader is aware of the previously described situation (if it is too much noise for the reading I will remove it).
Ok 👍
I think to keep it short, just mention here that the timestamp granularity is 1ns for DRKey and move the discussion to the appendix at the end of the file (which now needs to be updated anyway).
doc/protocols/authenticator-option.rst
line 40 at r4 (raw file):
Previously, JordiSubira wrote…
Done. I changed the layout, also merged the description. I do not think there's much difference between any of those, it will depend on the (de-)serializing implementation, both seem fine to me. Anyways I picked ( alg | rsv | ts) because I also like it better.
Excellent 👍
doc/protocols/authenticator-option.rst
line 54 at r6 (raw file):
Timestamp (Sequence Number): Unsigned 48-bit integer value. The applications can choose freely how many bits of this field can be used to represent the Timestamp
The "freely choose" is only accurate for the DRKey timestamp interpretation. I'd rephrase this introduction; "The Timestamp/Sequence Number field contains a unique value for each packet that can be used for replay detection by the receiver.
The detailed interpretation of the Timestamp/Sequence Number field depends on the SPI."
And then discuss the different cases; DRKey (in detail) and non-DRKey (just as a filler, just keep the old text on the "wrapping counter"/"sliding window").
Mention the "choose freely how many bits" only for the DRKey timestamp (which is defined as nanoseconds, but we allow the sender to set the value be more freely).
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
the uniqueness of the combination of timestamp and sequence number. For example, the value can be chosen based on a counter, randomly or even as a constant, provided that the send rate is low enough.
This paragraph above is no longer accurate and the packet identifier tuple cannot be represented without reference to the DRKey epoch (or the absolute send timestamp).
Move this information down into the section discussion the DRKey case.
Code quote:
The sender can arbitrarily choose this value, but it SHOULD ensure
the uniqueness of the combination of timestamp and sequence number.
For example, the value can be chosen based on a counter, randomly or even as
a constant, provided that the send rate is low enough.
The receiver SHOULD drop packets with duplicate
.. math::
(\mathrm{Source\ Address, info[0].Timestamp, Timestamp, Sequence\ Number})
Moreover, the receiver SHOULD drop packets with timestamps outside of a locally chosen
range around the current time.
doc/protocols/authenticator-option.rst
line 81 at r6 (raw file):
When used with a :ref:`SPI <spao-spi>` referring to an established security association, this is used as a wrapping counter and replay detection is based on sliding window of expected counter values.
Please keep this (somewhere, either before or after the DRKey case). I think it makes sense to hint at something other than the DRKey case.
Code quote:
When used with a :ref:`SPI <spao-spi>` referring to an established
security association, this is used as a wrapping counter and replay detection
is based on sliding window of expected counter values.
This use case is not specified in detail here. Extending this specification
in the future will closely follow [`RFC 4302 <https://tools.ietf.org/html/rfc4302>`_].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 58 at r1 (raw file):
Previously, matzf (Matthias Frei) wrote…
Ok 👍
I think to keep it short, just mention here that the timestamp granularity is 1ns for DRKey and move the discussion to the appendix at the end of the file (which now needs to be updated anyway).
No idea why there is again an incomplete sentence...
I also don't see much benefit in having a flag when using the nanosecond representation.
doc/protocols/authenticator-option.rst
line 54 at r6 (raw file):
Previously, matzf (Matthias Frei) wrote…
The "freely choose" is only accurate for the DRKey timestamp interpretation. I'd rephrase this introduction; "The Timestamp/Sequence Number field contains a unique value for each packet that can be used for replay detection by the receiver.
The detailed interpretation of the Timestamp/Sequence Number field depends on the SPI."And then discuss the different cases; DRKey (in detail) and non-DRKey (just as a filler, just keep the old text on the "wrapping counter"/"sliding window").
Mention the "choose freely how many bits" only for the DRKey timestamp (which is defined as nanoseconds, but we allow the sender to set the value be more freely).
+1 for rephrasing.
However, in the case of DRKey, the value in the Timestamp/Sequence Number field is not unique. So I would just state that the value can be used for replay detection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 4 unresolved discussions (waiting on @fstreun and @matzf)
doc/protocols/authenticator-option.rst
line 54 at r6 (raw file):
Previously, fstreun wrote…
+1 for rephrasing.
However, in the case of DRKey, the value in the Timestamp/Sequence Number field is not unique. So I would just state that the value can be used for replay detection.
Done.
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, matzf (Matthias Frei) wrote…
This paragraph above is no longer accurate and the packet identifier tuple cannot be represented without reference to the DRKey epoch (or the absolute send timestamp).
Move this information down into the section discussion the DRKey case.
Done.
doc/protocols/authenticator-option.rst
line 81 at r6 (raw file):
Previously, matzf (Matthias Frei) wrote…
Please keep this (somewhere, either before or after the DRKey case). I think it makes sense to hint at something other than the DRKey case.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 54 at r6 (raw file):
Previously, JordiSubira wrote…
Done.
As mentioned, in the DRKey case, this field does not contain a unique value for each packet.
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, JordiSubira wrote…
Done.
In the DRKey case with the nanosecond timestamp, I would not mention sequence numbers as this is a different concept.
Furthermore, the nanosecond timestamp still has to be interpreted over all bits of the field (it's not like the nanosecond timestamp suddenly is represented with just 32 bits instead of 48 bits).
Only when it comes to accuracy, an application can choose to reuse the same timestamp multiple times and add it to a counter (that resets after the timestamp is updated) to ensure uniqueness.
Suggestion:
When used with a DRKey :ref:`SPI <spao-spi>`, the Timestamp/Sequence Number field contains a timestamp (*Ts) in nanoseconds relative to the :ref:`Epoch<drkey-epoch>` starting time of the associated DRKey.
Within the associated DRKey :ref:`Epoch<drkey-epoch>`, Ts must be a unique value for each packet.
Since nanosecond accuracy is not always required, an application can also opt for a less accurate timestamp and use a counter to ensure uniqueness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @matzf)
doc/protocols/authenticator-option.rst
line 54 at r6 (raw file):
Previously, fstreun wrote…
As mentioned, in the DRKey case, this field does not contain a unique value for each packet.
I think this comment applies to the discussion below about the uniqueness of the tuple. Otherwise, could you quote what part are you referring to about the uniqueness?
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, fstreun wrote…
In the DRKey case with the nanosecond timestamp, I would not mention sequence numbers as this is a different concept.
Furthermore, the nanosecond timestamp still has to be interpreted over all bits of the field (it's not like the nanosecond timestamp suddenly is represented with just 32 bits instead of 48 bits).Only when it comes to accuracy, an application can choose to reuse the same timestamp multiple times and add it to a counter (that resets after the timestamp is updated) to ensure uniqueness.
I also think this comment belongs to the previous discussion. In any case, I think in the discussion we agreed on letting the application decide on how many bits are used for each item in the case of DRKey SPI. The rational about it was moved to the appendix section as Matthias suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 54 at r6 (raw file):
Previously, JordiSubira wrote…
I think this comment applies to the discussion below about the uniqueness of the tuple. Otherwise, could you quote what part are you referring to about the uniqueness?
Yes, this one.
Code quote:
The Timestamp/Sequence Number field contains a unique value for each packet that
can be used for replay detection by the receiver.
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, JordiSubira wrote…
I also think this comment belongs to the previous discussion. In any case, I think in the discussion we agreed on letting the application decide on how many bits are used for each item in the case of DRKey SPI. The rational about it was moved to the appendix section as Matthias suggested.
As far as I understand it, we agreed on using a nanosecond timestamp (without any sequence number) in the case of DRKey SPI, and the application can choose how accurate the timestamp is.
E.g., an application could provide an accuracy of ~1024 ns and the last 10 bits of the filed are occupied by a counter to ensure uniqueness. But the timestamp is still nanoseconds over all 48 bits (and the last 10 bits are just for uniqueness).
With the current description, I would interpret it as possible:
An application can use the first 32 bits to represent the relative nanosecond timestamp and use the other bits for something else. But this is not intended as 32 bits are not enough to cover the maximum timespan of a DRKey epoch.
Overall, I guess, I disagree that, in the DRKey case, there is a free choice on splitting the bits of the timestamp/sequence number filed.
There is only a free choice on the accuracy of the timestamp, which to me is something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, fstreun wrote…
As far as I understand it, we agreed on using a nanosecond timestamp (without any sequence number) in the case of DRKey SPI, and the application can choose how accurate the timestamp is.
E.g., an application could provide an accuracy of ~1024 ns and the last 10 bits of the filed are occupied by a counter to ensure uniqueness. But the timestamp is still nanoseconds over all 48 bits (and the last 10 bits are just for uniqueness).With the current description, I would interpret it as possible:
An application can use the first 32 bits to represent the relative nanosecond timestamp and use the other bits for something else. But this is not intended as 32 bits are not enough to cover the maximum timespan of a DRKey epoch.Overall, I guess, I disagree that, in the DRKey case, there is a free choice on splitting the bits of the timestamp/sequence number filed.
There is only a free choice on the accuracy of the timestamp, which to me is something else.
The original suggestion, that I agree with, was as follows:
The applications can choose freely how many bits of the timestamp they set based on a clock vs from a counter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @matzf)
doc/protocols/authenticator-option.rst
line 54 at r6 (raw file):
Previously, fstreun wrote…
Yes, this one.
Done.
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, fstreun wrote…
The original suggestion, that I agree with, was as follows:
The applications can choose freely how many bits of the timestamp they set based on a clock vs from a counter.
I personally find both very similar. In any case, it is stated bellow that the granularity MUST enable covering the maximum epoch length. The granularity will depend on how many bits are chosen to be used for the Ts. I see the Ts as a multiple of the chosen granularity, as it is defined in the formula, not as a ns counter. So if you pick 32-bits, the granularity will be ~1ms, otherwise it violates the previous statement.
Code snippet:
When used with a DRKey SPI, the granularity must enable to cover the maximum epoch length for DRKey (plus
the :ref:`Grace period<drkey-grace>`).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r5, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, JordiSubira wrote…
I personally find both very similar. In any case, it is stated bellow that the granularity MUST enable covering the maximum epoch length. The granularity will depend on how many bits are chosen to be used for the Ts. I see the Ts as a multiple of the chosen granularity, as it is defined in the formula, not as a ns counter. So if you pick 32-bits, the granularity will be ~1ms, otherwise it violates the previous statement.
The original suggestion:
The field contains a nanosecond timestamp that is relative to ....
A few bits of the timestamp can also be set by a counter, e.g., if accuracy is not available.
Your suggestion:
The field contains a nanosecond timestamp that is relative to ...., and a sequence number.
How many bits each of the two occupies is up to the application.
These two suggestions have different meanings.
The original (48-bit nanosecond) suggestion provides enough bits to ensure uniqueness and to cover the DRKey epoch length.
With that, no granularity calculation has to be done anymore, simplifying everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @JordiSubira and @matzf)
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, fstreun wrote…
The original suggestion:
The field contains a nanosecond timestamp that is relative to ....
A few bits of the timestamp can also be set by a counter, e.g., if accuracy is not available.Your suggestion:
The field contains a nanosecond timestamp that is relative to ...., and a sequence number.
How many bits each of the two occupies is up to the application.These two suggestions have different meanings.
The original (48-bit nanosecond) suggestion provides enough bits to ensure uniqueness and to cover the DRKey epoch length.
With that, no granularity calculation has to be done anymore, simplifying everything.
Furthermore, in the original (48-bit nanosecond) suggestion, we agreed on fixing the granularity to 1ns.
While allowing the application to freely choose accuracy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @matzf)
doc/protocols/authenticator-option.rst
line 62 at r6 (raw file):
Previously, fstreun wrote…
Furthermore, in the original (48-bit nanosecond) suggestion, we agreed on fixing the granularity to 1ns.
While allowing the application to freely choose accuracy.
The discussion is only restricted to DRKey case. If I'm not wrong we decided that the application can opt for how many bits are used for each Ts and SN, we are not fixing that for DRKey 48-bit value is used. This is also what Matthias seems to suggest here: 'The "freely choose" is only accurate for the DRKey timestamp interpretation. I'd rephrase this introduction; "The Timestamp/Sequence Number field contains a unique value for each packet that can be used for replay detection by the receiver.'
aa2821b
to
50c289e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r24, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
- add missing documentation for `remote_interface_id` in `topology.json` for peering links (fix-up for #4390) - fix duplicate envvar documentation in router.rst and control.rst (envvars can only have one definition in rst), and expand the description (fix-up for #4300). Consistent formatting for envvar documentation. - move the description of the duration string format to the common section, as it's referenced by multiple components (currently control service and router). - fix references in SPAO doc - enable fail-on-warning for read-the-docs builds: this makes PR fail the the read-the-docs check if there are build warnings. Also enable the same build flags in the Makefile for local documentation builds.
This PR updates the Timestamp definition in the SPAO specification. This update follows the discussion in #4062, making the Timestamp relative to the DRKey Epoch for DRKey SPI associations. Please, use this PR to discuss details, e.g., the upper bound defined for the Epoch length, necessary to define the granularity of the timestamp.
This change is