FAQ
https://codereview.appspot.com/15520047/diff/40001/ssh/certs.go
File ssh/certs.go (right):

https://codereview.appspot.com/15520047/diff/40001/ssh/certs.go#newcode63
ssh/certs.go:63: return ct == 1<<64-1
I do not think it does. I have not gotten an overflow in separate
testing (i.e. playground). The math package defines this... MaxUint64
= 1<<64 - 1, but I did not want to import the whole math package just
for this.

I can get 1<<64 to overflow, but I did not have 1<<64 - 1 overflow.

https://codereview.appspot.com/15520047/diff/40001/ssh/keys_test.go
File ssh/keys_test.go (right):

https://codereview.appspot.com/15520047/diff/40001/ssh/keys_test.go#newcode49
ssh/keys_test.go:49: ValidBefore: 0xFFFFFFFFFFFFFFFF, //
The end of currently representable time.
On 2013/10/22 21:45:35, jpsugar wrote:
Use a constant here?
Done.

https://codereview.appspot.com/15520047/

--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Hanwen at Oct 23, 2013 at 5:03 am
    what problem is this CL solving?


    https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go
    File ssh/certs.go (right):

    https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go#newcode55
    ssh/certs.go:55: // handle time values above what a time.Time can
    represent and prevent values

    This comment is too large compared to what we're dealing with.

    time.Time goes until way beyond we all die because of petrol
    crisis/global warming/world war 3. It might even go beyond the time
    needed to brute-force RSA-2048, so it buys little additional security (I
    haven't checked though).

    https://codereview.appspot.com/15520047/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Jmpittman at Oct 23, 2013 at 11:34 am
    TLDR...
    1. Conversion from uint64 to int64 to time.Time and then back had issues
    with negative times (before unix epoch) which are not allowed in certs.
    2. Helps with parse/marshal when trying to work around issue 1.


    The field encoding is defined as a uint64, not an int64 like is becoming
    typical for representing time with 64bit values. It was done this way
    in the spec to avoid introducing a new wire encoding format for ssh
    (this was djm's reasoning when I asked). Also, valid times for
    certificates must start at unix epoch. So, a value of 0 is supposed to
    correspond to unix epoch.

    The problem this solves...

    In openssh, if a time frame/window is not specified for a certificate
    during creation, the default behavior is to set ValidAfter to all 0's
    and ValidBefore to all 1's (in binary form). This is known as the
    "forever" setting. Previously, the code was doing a direct conversion
    from uint64 to int64. Any value above 1<<63-1 (max for an int64) gets
    treated as a negative value. So, what should be considered a positive
    value (really far into the future) suddenly becomes the past (before
    unix epoch). Specifically, 0xFFFFFFFFFFFFFFFF in a uint64 is really far
    into the future, but that same value in an int64 is -1 or unix epoch - 1
    second. In the current form, when doing time validation on a cert, this
    will flip flop the window so that the certificate expires before it ever
    becomes valid.

    Introducing CertTime will clamp the time range to start at unix epoch
    and go to the max value for int64, but never before unix epoch. So, the
    time window is always positive.

    The other issue this solves is encoding. In trying to find a good way
    around this issue with just a time.Time value, everything I did would
    screw up the parse/marshal value comparison. If you want a test for
    this, we already have one. The example key that jpsugar provided has
    validafter at 0 and validbefore at all 0xFF bytes because he never set a
    time window. So, the cert is intended to always be good. With the
    current code, the certificate would have been expired one second before
    it became valid.

    Does that make sense?


    https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go
    File ssh/certs.go (right):

    https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go#newcode55
    ssh/certs.go:55: // handle time values above what a time.Time can
    represent and prevent values
    I wrote two comments. One for the reader of the code and one for
    package documentation. I was hoping the first one explained the
    reasoning for the CL.

    https://codereview.appspot.com/15520047/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedOct 23, '13 at 12:19a
activeOct 23, '13 at 11:34a
posts3
users2
websitegolang.org

2 users in discussion

Jmpittman: 2 posts Hanwen: 1 post

People

Translate

site design / logo © 2021 Grokbase