Discussion on crypto support

[dwhutchison]

Creating this issue for a place for discussion outside of a PR.

While trying to implement the crypto functions for the server path, I noticed a few functions which didn't work for receiving messages. Primarily, the recv() and SendAsync() functions within the uasc package were tailored for sending client messages and not capable of receiving messages. The majority of the code is fine and the problems weren't major, but they do need some adjustment. For example, the SendAsync method would panic while trying to copy the reqHdr field into a ResponseHeader and the recv method could only handle messages with existing handlers (which are tied to client-generated requestIDs).

I'm preparing a PR for three things:

1. Generalize the above uasc functions to allow for processing both 'Request' and 'Response' types.
2. Create an initial server application layer to receive and process unsolicited messages.
3. Incorporate the crypto functions into the transport layer I'm working on all three simultaneously but if I can extract parts of each, I'll try publish them individually.

Within the server logic I'm working on a concept of a 'Channel Broker' to manage multiple secure channels on one listener. With this, I'm hoping to have a method of accepting connections, creating secure channels, and handling requests from multiple connections as well as slightly decoupling the secure channel from the uacp.Conn. This is because clients are allowed to reconnect to a server provided the secure channel hasn't expired so this should allow for a means to 'reattach' a new Conn to an existing secure channel. Having a system monitoring all active secure channels will also allow for a means to prune expired connections.

I anticipate client logic remaining the same as it is now; i.e. the application controlling the creation and use of SecureChannels. One thing I'm aware of, but unsure exactly how to implement it at this time, is that clients will also need a way to reconnect to a server utilizing an existing SecureChannel if the uacp.Conn is broken. I don't anticipate this being too difficult (try to send, fail, close the uacp.Conn, open a new one, replace the old conn reference with the new one, continue on). Also need to check the spec to see if the full HEL/ACK sequence needs to be repeated in this case.

156 was a proof of concept that the crypo functions themselves are functional, but will need more work to allow for both sending and receiving messages, decent secret management, etc. Now that I've looked closer at what it'll take to implement the server component of the logic, I should be able to clean up that sequencing.

Comments are welcome; when I have things cleaned up a bit I'll push a WIP branch for review.

[magiconair]

Based on experience with previous complex PRs I suggest to try hard to separate them. The sequence you have outlined sounds like three logical independent PRs. Keep the changes in separate branches which are based on another.

I would also like to know whether it is possible to incorporate the crypto layer into the client first since it would make the client more useful right now and is also somewhat of a pain point for myself. Otherwise, lets start with the network layer.

I know that this can be somewhat of a pain but trust me that this is a lot easier to review and you'll feel more confident that they are correct.

[dwhutchison]

I've pushed a branch which is a slight rework of #156 to incorporate the crypto into the client. Take a look and let me know what you think.
I think the config structures are a bit awkward right now so let me know if you have any thoughts on that. I've tested against Kepware and Ignition but test on other things and let me know if you run into issues.

[magiconair]

Thanks. I'll have a look and test them on Monday since the Siemens certs are also quite peculiar about their ip address.

I didn't touch the config structures yet. I thought this would come at some point. I think the session config should be part of the client config and we should use just the structs instead of the constructors. There are just too many complex arguments in this protocol and the constructors don't help IMO.

Is it OK if I try to refactor the keyring into a struct?

[magiconair]

We could try to use the Option function pattern to manage the configuration.

https://commandcenter.blogspot.com/2014/01/self-referential-functions-and-design.html https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

I'll try a PR.

[dwhutchison]

More testing would be great.

I was thinking about that method for Options, too; I like it but the rest of the code base doesn't use it so I felt that was out of scope of this change. I think that could help us as we add functionality.

Regarding the keyring, I forgot to remove the guts of that code. I ended up removing the requirement for it and only need the Thumbprint() method (which is just a simple helper method) so I'll just remove the unused code. I'll push an update.

[magiconair]

I've added #162 to add the session config to the client config and configure both with options. I just realized that we can also keep them separate with that approach. In any case this looks quite nice and extendable.

[magiconair]

Let me know if you're ok with the options and then we merge them so that we can move forward.

[dwhutchison]

Looks good in general.

I have a feeling this is going to break my crypto integration code as I’ve used the exported fields from client config that’s passed into the secure channel. That said, relying on higher level structures in lower level code is probably a good thing to get rid of so I’ll figure it out if it becomes a problem.

Edit: nevermind. Forgot that the config objects were all part of the uasc package so this’ll work.

[magiconair]

OK, I'll merge it then.

[dwhutchison]

@magiconair I've updated https://github.com/gopcua/opcua/tree/feature/crypto-client with the new config objects. I like it, this makes it a lot cleaner and much more expandable. Let me know when you've had a chance to test the crypto client. It still works against my test servers but I need a larger sample size since the the certificate stuff is really picky.

[magiconair]

I’ll try it today. On my way to the office now.

[magiconair]

This looks much cleaner indeed.

— Frank Schröder

On 9. Apr 2019, at 05:03, dwhutchison notifications@github.com wrote:

@magiconair I've updated https://github.com/gopcua/opcua/tree/feature/crypto-client with the new config objects. I like it, this makes it a lot cleaner and much more expandable. Let me know when you've had a chance to test the crypto client. It still works against my test servers but I need a larger sample size since the the certificate stuff is really picky.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[magiconair]

My cert does not work anymore on the Siemens PLC. Need some help from colleagues to get a new one installed.

[magiconair]

It does not work on the Siemens PLC. For some reason there is an unexpected EOF when decoding the CreateSessionResponse. I've captured the output and will see where it is stuck.

[magiconair]

Could you also rebase the crypto client code to master?

[dwhutchison]

Yes, I will absolutely clean the branch up and rebase to master before we finalize this; I didn't want to do that in case you had any edits but any PR will be cleaned up. If you have no edits, I can work on that

The EOF could be caused if the PLC has doesn't trust your certificate and it might be closing the connection. Also, ERR messages aren't handled correctly within the stack yet and those occasionally creep into the response decoding so that could be it, too. If you have wireshark logs, I can take a look, too.

[magiconair]

Can you find me on Keybase?

[magiconair]

We can edit it when it works. The CreateSessionResponse was received and the wireshark did not show another message. I got the wireshark dumps but since they're encrypted they won't help you much. I have added some code to dump the message after received and I'll add that as an OPCUA_DEBUG option.

[alxock]

@magiconair I was able to connect to a S7-1500 using the Basic256Sha256 security policy. Is your certificate signed with a hash that is Sha256 or stronger?

[magiconair]

No, it isn't ...

$ openssl x509 -in 192.168.254.17-urn_freeopcua_client-2048/cert.pem -text
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 10917693390298904800 (0x9783704b476578e0)
    Signature Algorithm: sha1WithRSAEncryption
        Issuer: C=SE, L=Stockholm, O=Northvolt, CN=192.168.254.17
        Validity
            Not Before: Apr 11 14:51:05 2019 GMT
            Not After : Apr  8 14:51:05 2029 GMT
        Subject: C=SE, L=Stockholm, O=Northvolt, CN=192.168.254.17
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:9f:39:8d:e0:4a:70:2b:88:50:58:78:96:ce:e9:
                    54:6d:ae:71:4a:0f:27:40:a7:9c:fd:39:fa:38:bb:
                    85:ce:c8:e2:d4:00:cf:c0:8a:d0:b0:38:e4:2b:45:
                    12:f5:e2:1a:ea:2b:1d:3e:f2:aa:b7:be:9b:9c:e3:
                    15:43:d5:e4:f8:29:4d:35:1e:6f:ff:ff:1c:62:60:
                    31:7b:86:13:5f:fa:b7:68:4d:d8:78:78:8d:ea:3c:
                    d8:93:10:34:65:e0:ad:c9:22:3e:b6:b2:db:1a:e7:
                    0c:95:a9:5e:87:fc:c0:e8:7b:13:cf:88:12:27:1f:
                    31:06:7f:c9:ff:59:3c:98:58:34:a9:26:95:a7:b4:
                    2b:39:7f:9b:d2:bc:b8:04:57:41:04:7b:24:d7:84:
                    10:44:65:d1:2f:39:c3:b3:c2:98:11:99:0e:4e:83:
                    a5:7b:50:da:37:58:43:eb:b3:ac:c9:84:ec:1a:c7:
                    59:57:c8:47:14:ab:51:8e:97:fd:cb:e1:d0:9d:8b:
                    a4:ec:f7:8f:6d:25:d6:1c:d8:44:e9:5e:28:7a:8e:
                    98:85:5d:e3:41:3d:45:ee:67:2f:70:a4:78:b6:8a:
                    48:7d:1e:62:28:f1:25:95:f3:8c:5f:e1:40:71:c4:
                    18:b7:5a:65:ed:d7:86:bc:86:30:ab:d9:af:44:ba:
                    d9:5f
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Basic Constraints:
                CA:TRUE
            X509v3 Authority Key Identifier:
                DirName:/C=SE/L=Stockholm/O=Northvolt/CN=192.168.254.17
                serial:97:83:70:4B:47:65:78:E0

            X509v3 Key Usage:
                Digital Signature, Non Repudiation, Key Encipherment, Data Encipherment, Certificate Sign, CRL Sign
            X509v3 Extended Key Usage:
                TLS Web Server Authentication, TLS Web Client Authentication
            X509v3 Subject Alternative Name:
                URI:urn:freeopcua:client, IP Address:192.168.254.17
    Signature Algorithm: sha1WithRSAEncryption
         59:c3:76:01:bc:d0:42:38:75:d2:aa:3a:57:ab:1b:5c:2b:10:
         44:d3:64:5d:1d:43:4a:e0:18:80:cc:5d:e9:91:1c:a6:f5:9d:
         c7:45:3a:c8:17:ba:8f:55:bf:f4:8e:5f:55:ed:d5:f9:96:9e:
         ac:45:7c:f3:5f:e1:dd:05:2a:40:5d:1b:16:58:2c:ff:8f:eb:
         b4:60:25:23:fb:fd:ca:d6:86:7c:b6:c2:83:87:cc:70:5f:7f:
         b2:56:9f:88:57:28:26:b2:51:17:1f:79:8d:3c:54:0c:63:3f:
         cb:83:67:b3:1f:b6:d1:32:b4:bc:8e:d5:a1:54:64:f9:59:7d:
         bc:21:0a:7e:13:94:ef:f8:05:e3:45:20:b2:d7:29:46:2e:30:
         c4:3a:13:d9:93:af:04:7f:ec:64:be:5f:ac:94:f3:db:14:ac:
         8c:08:46:69:c3:7f:66:ea:f4:d0:11:3d:62:df:3c:40:6a:3a:
         1e:76:8c:c9:09:ea:e9:4d:97:4f:36:70:72:39:fc:59:ed:a2:
         95:e7:77:f1:44:5d:cd:7a:f4:6e:b3:4e:e9:09:bf:fe:0c:82:
         b9:74:0e:e3:36:f6:d3:34:4f:e1:8d:80:de:60:77:2a:1d:f6:
         6a:d8:85:76:d3:fa:34:dc:9b:d4:39:31:f0:02:9d:2b:44:a9:
         fa:26:dd:1d

I've used this script to create the cert. It might make sense to create a helper app to create the cert like the generate_cert.go we have in the crypto example.

$ cat siemens-cert.sh
#!/bin/bash
#
# ./siemens-cert.sh <ip> <application-uri> [<subject> [<bits>]]
#
# e.g. ./siemens-cert.sh 1.2.3.4 urn:freeopcua:client "/C=SE/L=Stockholm/O=Northvolt" 2048

set -o errexit
set -o nounset
set -o pipefail

# ip address of the client, e.g. 192.168.1.2
IP=$1

# ApplicationURI of the client, e.g. urn:freeopcua:client
URI=$2

# base subject of the certificate
SUBJ=${3:-/C=US/O=Organization/OU=OrganizationUnit}

# size of the private key in bits.
# keys with more than 2048 bits don't seem to work with Siemens PLCs
BITS=${4:-2048}

base=${IP}-$(echo $URI | tr : _ )-${BITS}

(
    mkdir -p ${base} && cd ${base}

    cat > extensions.cnf <<-EOF
    basicConstraints=CA:TRUE
    authorityKeyIdentifier=keyid,issuer
    keyUsage=dataEncipherment,keyEncipherment,nonRepudiation,digitalSignature,keyCertSign,cRLSign
    extendedKeyUsage=serverAuth,clientAuth
    subjectAltName=URI:urn:freeopcua:client,IP:${IP}
    EOF

    openssl genrsa -out key.pem ${BITS}
    openssl req -new -key key.pem -out cert.csr -subj "${SUBJ}/CN=${IP}"
    openssl x509 -req -days 3650 -extfile extensions.cnf -in cert.csr -signkey key.pem -out cert.pem
    openssl x509 -in cert.pem -inform PEM -out cert.der -outform DER
    openssl rsa -in key.pem -inform PEM -out key.der -outform DER
)

[magiconair]

openssl x509 -req -sha256 ... should do the trick.

[magiconair]

@alxock yes, that works for me as well. Updated https://github.com/gopcua/opcua/wiki/Supported-Devices

[magiconair]

@dwhutchison are we done with this? I would think so.

[dwhutchison]

I think so, too. Closing.