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

oqs-test/try_connection.py tests what is in the README. #103

Merged
merged 2 commits into from
Jun 9, 2021
Merged

Conversation

xvzcf
Copy link

@xvzcf xvzcf commented Jun 7, 2021

No description provided.

@xvzcf xvzcf requested review from dstebila and baentsch June 7, 2021 17:59
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

Please check the specific comments above.
Generally, using the regress data works, of course; but I'd then also suggest adding that

  • host and client keys can be separately generated using ssh-keygen
  • configuration options (like port etc.) can either be set on the command line or in the respective ssh_config and sshd_config files in the regress folder.

Also, if the whole build documentation is reduced to running the build scripts, I'd suggest adding that "PREFIX" can be set to facilitate a specific oqs-openssh install location: I can't imagine someone trying this will want to replace their system-wide ssh right away :-)

@xvzcf
Copy link
Author

xvzcf commented Jun 8, 2021

Generally, using the regress data works, of course; but I'd then also suggest adding that

* host and client keys can be separately generated using `ssh-keygen`

Added a brief note about this.

* configuration options (like port etc.) can either be set on the command line or in the respective `ssh_config` and `sshd_config` files in the `regress` folder.

Same here.

Also, if the whole build documentation is reduced to running the build scripts, I'd suggest adding that "PREFIX" can be set to facilitate a specific oqs-openssh install location: I can't imagine someone trying this will want to replace their system-wide ssh right away :-)

The oqs-scripts/build_openssh.sh script already has an INSTALL_PREFIX variable, set by default to a non-system location. The README instructions (and the test scripts) however don't use this install location, and simply use the default location of the built artifacts.

README.md Outdated Show resolved Hide resolved
Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. LGTM.

@baentsch baentsch merged commit 1e4ec38 into OQS-v8 Jun 9, 2021
@baentsch baentsch deleted the readme-fix branch June 9, 2021 08:14
@baentsch baentsch mentioned this pull request Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants