Skip to content
This repository was archived by the owner on Jul 8, 2021. It is now read-only.

Comments

Add GEX-SHA1 DH KEX to xssh scanner.#219

Closed
aaspring wants to merge 4 commits intozmap:masterfrom
aaspring:feat/xsshAddDHGroupExchange
Closed

Add GEX-SHA1 DH KEX to xssh scanner.#219
aaspring wants to merge 4 commits intozmap:masterfrom
aaspring:feat/xsshAddDHGroupExchange

Conversation

@aaspring
Copy link
Contributor

Add the diffie-hellman-group-exchange-sha1 DH KEX to the xssh scanner. This is based on the implementation by @breml with additions for customization and JSON output.

Example output --

      "algorithm_selection": {
        "dh_kex_algorithm": "diffie-hellman-group-exchange-sha1",
        "host_key_algorithm": "ssh-rsa",
        "client_to_server_alg_group": {
          "cipher": "aes128-ctr",
          "mac": "hmac-sha1",
          "compression": "none"
        },
        "server_to_client_alg_group": {
          "cipher": "aes128-ctr",
          "mac": "hmac-sha1",
          "compression": "none"
        }
      },
      "dh_key_exchange": {
        "parameters": {
          "prime": {
            "value": "///////////JD9qiIWjCNMTGYouA3BzRKQJOCIpnzHQCC76mOxObIlFKCHmONATd75UZs806QxswKwpt8l8UN0/hNW1tUcJF5IW1dmJefsb0TELppjftawv/XLb0Brft7jhr+1qJn6WunyQRfEsf5kkoZlHs5Fs9wgB8uKFjvwWY2kg2HFXTmmkWP6j9JM9fg2VdI9yjrZYcYvNWIIVSu57VKQdwlpZtZww1Tkq8mATxdGwIyhghfDKQXkYuNs474553LBgOhgObJ4Oi7Aeij7XFXfBvTFLJ3ivL9pVYFxg5lUl86pVq5RXSJhiY+gUQFXKOWoqsqmj//////////w==",
            "length": 2048
          },
          "generator": {
            "value": "Ag==",
            "length": 8
          },
          "server_public": {
            "value": "JYOBCiR4VtqQqkWnLW+PTVrQLacaTFGFKvzLuSpRHSeApn0dlinAmMZM4vOs1dKATjCN4/ULkxohbEj9OQZq8Gq8rxNnzdmiVNFNVkjI8DhrSF0JHKTsFjCVa1J4h3jB5NwFAGxsa+Hqy/L/4K6JlL8izlNUZQU7QU2UpnY25dDra75VNSEJ1mhXcr1bczSb3z+4V8TQwLsq0XtY9DA4GkjbTnJ8SRN/YnXSskRaeGsaENEON+YNoDEw3/9l7t00XMV505CtSBjsbm1WcRqQV2CFsy8ndH0KI4UxvCDP8y6spndm42exRYTKeTTUnxv7X0W1oksS6yoY4zC5YOnCqA==",
            "length": 2048
          }
        },
        "server_host_key": {
          "rsa_public_key": {
            "exponent": 65537,
            "modulus": "28tCPT/38f7AyAlrO2p3CRB16lNb8n+SVwG8TMnVAy4Q1/jQSa8tTp81CwxIXIN4f5lGXlujgnzAb7vhygpnBShvZXVFzfAu2tJFwytLy9tvEEYvRCcnRB1FWLW+pcqaOpuPxOjYx2BRmpAxRdO1b4ciuenHiiAZcDgMm7dp2wpMGAh4eP+ApSkql5ytkiZI0iwy3Co4nKupfg6fa/jBKtRxMMeWfjSO3w6I7uctpOWTkQEhwyRYZgzg+ltFGvZ/0dkPw4tBsUBpBIhUHBBl2k005eaKn5T/9ji3YsO0IOwDlae85hYviggPZJ02MFLkpAdWkWx+VL5YzMztZhorZw==",
            "length": 2048
          },
          "raw": "AAAAB3NzaC1yc2EAAAADAQABAAABAQDby0I9P/fx/sDICWs7ancJEHXqU1vyf5JXAbxMydUDLhDX+NBJry1OnzULDEhcg3h/mUZeW6OCfMBvu+HKCmcFKG9ldUXN8C7a0kXDK0vL228QRi9EJydEHUVYtb6lypo6m4/E6NjHYFGakDFF07VvhyK56ceKIBlwOAybt2nbCkwYCHh4/4ClKSqXnK2SJkjSLDLcKjicq6l+Dp9r+MEq1HEwx5Z+NI7fDoju5y2k5ZORASHDJFhmDOD6W0Ua9n/R2Q/Di0GxQGkEiFQcEGXaTTTl5oqflP/2OLdiw7Qg7AOVp7zmFi+KCA9knTYwUuSkB1aRbH5UvljMzO1mGitn",
          "algorithm": "ssh-rsa",
          "fingerprint_sha256": "043ff81fe7b1f9284c5b2a6ec31cfb26171288c2c52fc4ce0850d04be8ce35ba"
        },
        "server_signature": "AAAAB3NzaC1yc2EAAAEAYV08/cV+73VGrKuALZ/EzBXTyAQk0u71NygWDXVnXfMY/OG+f0NaHPhya88/aGBSDIxG3+/qH542foJRFsGvzihQq/fKwrauxv0UQFxR70yZKrJQOGxufioF4WaZUoXzFuZ5AsxkgNsUJGeHCL04/zZCviKiovZ8Mepqwry1x+zDEsV7ZblFVCGUGSekJEcC1/DdqXLIF1kERUcr4M6hEXuYOdBwuE3AzOQxnd5x/whRPhvdS4MnVEU3eH+b2DLyinsDCFapfeAS3K/VCRsJlA0oTlWKmpGJCACMnMRsT1UdrBBKWl21tlCLziINY15+gXwRb2703XOPkKaMXzBf0w=="
      },

The sub-grouped parameters and broken out signature fields will be added via their appropriate issues' commit.

@breml's original implementation: breml/crypto@09c4372

It is intentional that this commit does not compile. In order to
effectively track changes made from the vendor's code, this is a direct
import without fixes for the ZGrab xssh package requirements. Future
commits will fix and restore functionality.

Code is directly imported from
breml/crypto@09c4372
@zakird
Copy link
Member

zakird commented Jan 29, 2017

What's the license and copyright situation with this code?

@aaspring
Copy link
Contributor Author

The repo is a fork of the golang/crypto repo with with the exact same license.

https://github.com/breml/crypto/blob/master/LICENSE
https://github.com/golang/crypto/blob/master/LICENSE

@dadrian
Copy link
Member

dadrian commented Jan 29, 2017

It looks like this is going to conflict with #215

@aaspring
Copy link
Contributor Author

Depending on how good the merging is, the flag.go file may conflict. If it does, it will be superficial and can be manually merged with little effort.

@breml
Copy link

breml commented Jan 30, 2017

Hi there, I am the author of the mentioned code for DH GEX. I just wanted to add some notes.

  1. The license, as already mentioned, unchanged to the original license of Go.
  2. The client side code is working in production, the server side code is passing the tests, but was never used in production. Especially the part of loading the moduli is not properly implemented.
  3. There is a single file drop in version in this branch: https://github.com/breml/crypto/tree/kexalgodhgex, which makes it easy to keep track of changes either in the DH GEX or in upstream crypto/ssh. Have a look at breml/crypto@34a7d8c and breml/crypto@e2544b2 (better implementation for moduli) on the before mentioned branch.
  4. I tried to get DH GEX merged into upstream crypto/ssh (proposal: x/crypto/ssh: support Diffie-Hellman Group Exchange from RFC 4419 golang/go#17230). The issue is currently open and not yet decided.

@zakird
Copy link
Member

zakird commented Feb 4, 2017

@breml thank you for the additional information. I greatly appreciate you reaching out and providing context.

@aaspring given @breml's comments, are there any changes that you want to make to this PR or is it still good to go? Offhand it looks fine to me, but given that @breml mentions a drop in version, I wanted to double check that this is still the right approach.

@dadrian Can you please take a once over on this code when you have a moment? I think you're the other person on the ZGrab team most familiar with SSH protocol.

@zakird zakird requested a review from dadrian February 4, 2017 16:18
@aaspring
Copy link
Contributor Author

aaspring commented Feb 4, 2017

I will look into the drop-in version today and see. A self-contained approach would be better given that it may be added to the upstream package which would warrant us removing this patch and using the upstream version.

@breml Thank you very much for dropping in and letting us know there's a better approach.

@dadrian
Copy link
Member

dadrian commented Feb 4, 2017

@aaspring ping me when you want review

@aaspring
Copy link
Contributor Author

aaspring commented Feb 5, 2017

Closing PR. GEX functionality is being added via @breml's drop-in implementation in PR 232.

@aaspring aaspring closed this Feb 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants