Make openVMS seeding less dependent of OpenVMS version [master, 3.1, 3.0] by levitte · Pull Request #18731 · openssl/openssl

levitte · 2022-07-06T04:26:38Z

SYS$GETTIM_PREC is a very new function, only available on OpenVMS v8.4.
OpenSSL binaries built on OpenVMS v8.4 become unusable on older OpenVMS
versions, but building for the older CRTL version will make the high
precision time functions unavailable.

Tests have shown that on Alpha and Itanium, the time update granularity
between SYS$GETTIM and SYS$GETTIM_PREC is marginal, so the former plus
a sequence number turns out to be better to guarantee a unique nonce.

Fixes #18727

levitte · 2022-07-06T04:27:16Z

This is the port of #18730 for higher OpenSSL versions

providers/implementations/rands/seeding/rand_vms.c

openssl-machine · 2022-08-07T00:07:47Z

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

openssl-machine · 2022-09-07T00:08:45Z

This PR is in a state where it requires action by @openssl/otc but the last update was 61 days ago

hlandau · 2022-09-09T12:32:07Z

I expect this is having difficulty getting reviews due to lack of VMS experience.

Though I have no direct experience with VMS, this patch looks right to me, and I've read the reference material for the relevant VMS system calls and think it makes sense. Moreover, there is a third party report of this working in #18727. So I'm approving this.

providers/implementations/rands/seeding/rand_vms.c

openssl-machine · 2022-10-10T00:08:39Z

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

t8m · 2022-10-24T10:53:46Z

@levitte this is waiting on update

levitte · 2022-10-24T11:01:42Z

Yes I know. I've been away on other stuff, but I actually started looking again this morning.

levitte · 2022-10-25T10:47:14Z

Rebased on a fresher master, and changes completely remade with data
from VSI forum discussions in mind.

levitte · 2022-10-25T10:48:26Z

Do note that this change assumes that ossl_pool_add_nonce_data() is called under lock. I haven't checked this myself, but have been told this by @paulidale.

hlandau

One nit, otherwise looks good.

providers/implementations/rands/seeding/rand_vms.c

levitte · 2022-10-25T19:39:36Z

Apparently, I hadn't rebase on a fresh enough master, and got some pyca related build failures.
I've rebased again now, hoping that makes things better... albeit not really relevant for this PR

levitte · 2022-10-26T05:20:58Z

@mgdaniel, if you wanna test on openssl 3.0, this is the appropriate URL for rand_vms.c.
Although, mind you, the 1.1.1 change is exactly the same in practice, so it should be sufficient for us if you try that.

https://raw.githubusercontent.com/openssl/openssl/ba0127fa7c73c7f64d2f96b214c51482256b3546/providers/implementations/rands/seeding/rand_vms.c

hlandau · 2022-10-26T08:31:25Z

@t8m Reapprove?

mgdaniel · 2022-10-26T19:10:47Z

Hi Richard, others. Grabbed the RAND_VMS.C as indicated and built under 1.1.1l which I had lurking, using OpenVMS V8.4-2L1. Made sure the new code was being employed with a simple sys$gettim((void*)&data.time); printf("++++++++++1 %llu\n", data.time); Moved the resulting OPENSSL.EXE to another V8.4-2L2 system and OK, then to OpenVMS V8.3 (the bigger test) and also OK. Looks like less version dependency and therefore greater portability has been achieved. Many thanks, Mark.

…

On Tue, 25 Oct 2022 22:21:09 -0700 ***@***.*** wrote: @mgdaniel, if you wanna test on openssl 3.0, this is the appropriate URL for rand_vms.c. Although, mind you, the 1.1.1 change is exactly the same in practice, so it should be sufficient for us if you try that. https://raw.githubusercontent.com/openssl/openssl/ba0127fa7c73c7f64d2f96b214c51482256b3546/providers/implementations/rands/seeding/rand_vms.c -- Reply to this email directly or view it on GitHub: #18731 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

levitte · 2022-10-26T21:44:49Z

Great! Thanks for the help, @mgdaniel

SYS$GETTIM_PREC is a very new function, only available on OpenVMS v8.4. OpenSSL binaries built on OpenVMS v8.4 become unusable on older OpenVM versions, but building for the older CRTL version will make the high precision time functions unavailable. Tests have shown that on Alpha and Itanium, the time update granularity between SYS$GETTIM and SYS$GETTIM_PREC is marginal, so the former plus a sequence number turns out to be better to guarantee a unique nonce. Fixes openssl#18727

levitte · 2022-10-27T10:18:56Z

#19493 removes ossl_rand_pool_add_additional_data(), so that removal could be dropped from this PR with a rebase.

SYS$GETTIM_PREC is a very new function, only available on OpenVMS v8.4. OpenSSL binaries built on OpenVMS v8.4 become unusable on older OpenVM versions, but building for the older CRTL version will make the high precision time functions unavailable. Tests have shown that on Alpha and Itanium, the time update granularity between SYS$GETTIM and SYS$GETTIM_PREC is marginal, so the former plus a sequence number turns out to be better to guarantee a unique nonce. Fixes #18727 Reviewed-by: Hugo Landau <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #18731)

SYS$GETTIM_PREC is a very new function, only available on OpenVMS v8.4. OpenSSL binaries built on OpenVMS v8.4 become unusable on older OpenVM versions, but building for the older CRTL version will make the high precision time functions unavailable. Tests have shown that on Alpha and Itanium, the time update granularity between SYS$GETTIM and SYS$GETTIM_PREC is marginal, so the former plus a sequence number turns out to be better to guarantee a unique nonce. Fixes #18727 Reviewed-by: Hugo Landau <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #18731) (cherry picked from commit 7056dc9)

levitte · 2022-10-27T10:44:39Z

Merged

master:

7056dc9 Make openVMS seeding less dependent of OpenVMS version

3.1:

868141d Make openVMS seeding less dependent of OpenVMS version

3.0:

bc84a93 Make openVMS seeding less dependent of OpenVMS version

NOTE: I kept the removal of ossl_rand_pool_add_additional_data() in
the cherry pick to 3.0, since #19493 wasn't applied in that branch.

SYS$GETTIM_PREC is a very new function, only available on OpenVMS v8.4. OpenSSL binaries built on OpenVMS v8.4 become unusable on older OpenVM versions, but building for the older CRTL version will make the high precision time functions unavailable. Tests have shown that on Alpha and Itanium, the time update granularity between SYS$GETTIM and SYS$GETTIM_PREC is marginal, so the former plus a sequence number turns out to be better to guarantee a unique nonce. Fixes openssl#18727 Reviewed-by: Hugo Landau <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#18731)

levitte added approval: otc review pending approval: review pending This pull request needs review by a committer branch: 3.0 Applies to openssl-3.0 branch branch: master Applies to master branch labels Jul 6, 2022

paulidale reviewed Jul 6, 2022

View reviewed changes

providers/implementations/rands/seeding/rand_vms.c Outdated Show resolved Hide resolved

levitte mentioned this pull request Jul 6, 2022

OpenVMS rand_vms.c dependency #18727

Closed

t8m added triaged: bug The issue/pr is/fixes a bug triaged: feature The issue/pr requests/adds a feature labels Jul 7, 2022

hlandau approved these changes Sep 9, 2022

View reviewed changes

hlandau removed the approval: review pending This pull request needs review by a committer label Sep 9, 2022

hlandau mentioned this pull request Sep 9, 2022

Make openVMS seeding less dependent of OpenVMS version [1.1.1] #18730

Closed

mattcaswell requested changes Sep 9, 2022

View reviewed changes

providers/implementations/rands/seeding/rand_vms.c Outdated Show resolved Hide resolved

t8m removed the approval: otc review pending label Oct 24, 2022

t8m added the branch: 3.1 Applies to openssl-3.1 (EOL) label Oct 24, 2022

levitte force-pushed the fix-18727 branch from 1689ecc to 1f5deab Compare October 25, 2022 10:45

levitte changed the title ~~Make openVMS seeding less dependent of OpenVMS version [master, 3.0]~~ Make openVMS seeding less dependent of OpenVMS version [master, 3.1, 3.0] Oct 25, 2022

levitte added approval: review pending This pull request needs review by a committer approval: otc review pending labels Oct 25, 2022

levitte requested a review from hlandau October 25, 2022 11:49

t8m approved these changes Oct 25, 2022

View reviewed changes

t8m removed the approval: otc review pending label Oct 25, 2022

t8m removed the triaged: feature The issue/pr requests/adds a feature label Oct 25, 2022

hlandau suggested changes Oct 25, 2022

View reviewed changes

providers/implementations/rands/seeding/rand_vms.c Outdated Show resolved Hide resolved

levitte force-pushed the fix-18727 branch from 03f2872 to ba0127f Compare October 25, 2022 19:37

hlandau approved these changes Oct 26, 2022

View reviewed changes

levitte requested a review from t8m October 26, 2022 09:16

t8m approved these changes Oct 26, 2022

View reviewed changes

t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Oct 26, 2022

levitte added 2 commits October 27, 2022 12:17

fixup! Make openVMS seeding less dependent of OpenVMS version

37daead

levitte force-pushed the fix-18727 branch from ba0127f to 37daead Compare October 27, 2022 10:17

levitte closed this Oct 27, 2022

levitte deleted the fix-18727 branch October 27, 2022 10:45

Uh oh!

Comments

Conversation

levitte commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

levitte commented Jul 6, 2022

Uh oh!

Uh oh!

openssl-machine commented Aug 7, 2022

Uh oh!

openssl-machine commented Sep 7, 2022

Uh oh!

hlandau commented Sep 9, 2022

Uh oh!

Uh oh!

openssl-machine commented Oct 10, 2022

Uh oh!

t8m commented Oct 24, 2022

Uh oh!

levitte commented Oct 24, 2022

Uh oh!

levitte commented Oct 25, 2022

Uh oh!

levitte commented Oct 25, 2022

Uh oh!

hlandau left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

levitte commented Oct 25, 2022

Uh oh!

levitte commented Oct 26, 2022

Uh oh!

hlandau commented Oct 26, 2022

Uh oh!

mgdaniel commented Oct 26, 2022 via email

Uh oh!

levitte commented Oct 26, 2022

Uh oh!

levitte commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

levitte commented Oct 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

levitte commented Jul 6, 2022 •

edited

Loading

levitte commented Oct 27, 2022 •

edited

Loading