Add crypto support to the client #166

magiconair · 2019-04-11T21:37:46Z

This patch adds initial crypto support to the client. A working example can be found in examples/crypto. We have tested Basic128Rsa15, Basic256 with Sign and SignAndEncrypt on a Siemens PLC and others (@dwhutchison pls fill in your test equipment here).

Basic256Sha256 still has issues but in order to test this we have also updated the debugging output and are finally bubbling ERRF messages up to the client.

We still need to fix the Basic256Sha256 and better configuration options for authentication tokens but this is a start.

Closes #111

dwhutchison · 2019-04-12T04:24:45Z

I've successfully tested against:

Kepware v6

Security Policy	Security Mode
None	None
Basic128Rsa15	Sign
Basic128Rsa15	SignAndEncrypt
Basic256	Sign
Basic256	SignAndEncrypt

Ignition 8 (Eclipse Milo)

Security Policy	Security Mode
None	None
Basic128Rsa15	SignAndEncrypt
Basic256	SignAndEncrypt
Basic256Sha256	SignAndEncrypt
Aes128_Sha256_RsaOaep	SignAndEncrypt
Aes256_Sha256_RsaPss	SignAndEncrypt

OPC UA ANSI C Demo Server v1.8.3
https://www.unified-automation.com/downloads/opc-ua-servers/file/download/details/opc-ua-ansi-c-demo-server-v183-windows.html

Security Policy	Security Mode
Basic256Sha256	Sign
Basic256Sha256	SignAndEncrypt
Aes128_Sha256_RsaOaep	Sign
Aes128_Sha256_RsaOaep	SignAndEncrypt
Aes256_Sha256_RsaPss	Sign
Aes256_Sha256_RsaPss	SignAndEncrypt

Prosys Simulation Server

Security Policy	Security Mode
None	None
Basic128Rsa15	Sign
Basic128Rsa15	SignAndEncrypt
Basic256	Sign
Basic256	SignAndEncrypt
Basic256Sha256	Sign
Basic256Sha256	SignAndEncrypt

dwhutchison · 2019-04-17T18:59:50Z

I've also successfully tested authentication against the following servers in the latest version

Prosys Simulation Server

Anonymous
Username
Certificate

Kepware

Anonymous
Username

Ignition 8

Anonymous
Username

OPC UA ANSI C Demo Server v1.8.3

Anonymous

magiconair · 2019-04-17T19:04:49Z

It might make sense to move this info to a Wiki page after the merge. I have someone from Siemens helping with the Basic256Sha256 issue.

magiconair · 2019-04-17T19:05:27Z

What is your take on merging this? What is still missing?

Allows for sending and receiving messages secured by the various security policies. See examples/crypto for a usage example.

…policy combinations

If an error was recevied while sending an OpenSecureChannel request, the stack would still try send a CloseSecureChannel even though the channel was never fully negotiated. This panic'd the stack as headers were not correctly initialized

Anonymous is still the only supported authorization method, however new Options were added for the remaining ones and added support to the crypto client to accept an auth type parameter

Instead of generating the next signature to send after we receive the Create/Activate response, we save the server nonce and create the signature immeditatly before sending the next ActivateSession request. The nonce is also used for encrypting user tokens so this will also allow us to support username authentication

Reorganized the client so most of the flag parsing is done outside of main and also added additional checks to make sure the user selected a supported combination of policy/mode before connecting to the server.

dwhutchison · 2019-04-17T19:12:45Z

It needs a sanity check on code quality, etc, then we should merge.

There are a couple QoL features I'll add later as smaller things but I don't want this to get too out of hand (it's already bigger than I was expecting)
eg. allowing username auth without supplying a certificate

magiconair · 2019-04-17T19:18:49Z

Is that something you want to do yourself or should I do that?

dwhutchison · 2019-04-17T19:34:48Z

I'd appreciate if someone else could do it to ensure fresh eyes. The uasc integration is the most important and that part is mostly fine. The biggest issue I can think of is that it could use some helper methods for certificate parsing. We could create an issue and I'll work on that afterwards. I also still plan on doing a refactor of the securitypolicy package to see if I can replace the closures with something else... also a future task.

The crypto client feels like a mess since it blew up with a ton of flag parsing but since that's just an example I'm not too worried about it.

magiconair · 2019-04-17T19:44:52Z

OK, I can have a look. I've rebased the branch on master and force-pushed it. I've also fixed an import cycle with the codectest which caused all binaries to include the test.* flags.

magiconair · 2019-04-17T20:05:57Z

I can also refactor the security policies to proper structs. That's somewhat straightforward.

dwhutchison · 2019-04-17T20:39:58Z

Yea, you're welcome to take a run at it.
I had some other small changes that I was going to do while in that package but those can be done separately.

uapolicy/crypto_hmac.go

uapolicy/crypto_pkcs1v15.go

uapolicy/crypto_rsapss.go

magiconair · 2019-04-18T00:20:15Z

@dwhutchison I've cleaned up the security policies as follows:

convert functions with closures to structs. EncryptionAlgorithm is still using fields but with interfaces now.
move constant values for block sizes and min padding to constants
unify SecurityPolicy constants between uasc and this package
rename securitypolicy to uapolicy

The encrypt/decrypt/signature/verifySignature fields in the EncryptionAlgorithm still bug me a bit but only b/c I think there should be a prettier solution. :)

I still get the error for Basic256Sha256 on the Siemens PLC. Basic256 and Basic128Rsa15 work with Sign and SignAndEncrypt.

dwhutchison · 2019-04-18T04:00:52Z

Nice. That works. The only thing I’d recommend is to unexport all but the EncryptionPolicy struct as the others should only used be within this package.

I know what you mean with the four interfaces; the repetition was the reason it looked the way it did to begin with. If we think of anything cleaner we can edit later.

I’ll test in the morning but I don’t see why this would change anything.

dwhutchison · 2019-04-18T14:08:04Z

Tested and everything still works fine

magiconair · 2019-04-19T10:18:05Z

Then lets merge and move on.

magiconair mentioned this pull request Apr 11, 2019

Add sign/encryption support #111

Closed

dwhutchison mentioned this pull request Apr 12, 2019

Cleanup to prep for crypto #160

Closed

dwhutchison and others added 19 commits April 17, 2019 21:07

Incorporate the crypo functions into the client transport

093c40c

Allows for sending and receiving messages secured by the various security policies. See examples/crypto for a usage example.

corrected padding removal while decrypting message

90f909c

improve debugging

a7518c8

make crypto client more configurable

dd6026d

added additional checks to avoid panics

066adc6

goify TCP in names

1a4893b

control debug output via flag for tests

3f55de0

Trim uacp package and handle errors

a78b543

Receive full uacp messages in secure channel

7f5714f

handle ERRF and bubble it up

3ff9664

drop go-cmp dependency

7beae4c

fix tests

9bbbaab

updated crypto client and secure channel to check for valid security …

b36f380

…policy combinations

fix panic on close channel

a27e98a

If an error was recevied while sending an OpenSecureChannel request, the stack would still try send a CloseSecureChannel even though the channel was never fully negotiated. This panic'd the stack as headers were not correctly initialized

added ServiceFault to service generation code

948b370

added preliminary support for additional auth policies

dcf8d46

Anonymous is still the only supported authorization method, however new Options were added for the remaining ones and added support to the crypto client to accept an auth type parameter

added support for username and certificate auth

52c0451

modified crypto client to be more robust with user input

bee654f

Reorganized the client so most of the flag parsing is done outside of main and also added additional checks to make sure the user selected a supported combination of policy/mode before connecting to the server.

magiconair added 2 commits April 17, 2019 21:42

update go.mod

e956146

break ua.CodecTest import cycle

590e473

magiconair force-pushed the feature/crypto-client branch from 7dc6d6d to 590e473 Compare April 17, 2019 19:43

update import cycle comment

086bf3b

magiconair added 5 commits April 18, 2019 01:24

refactor securitypolicy

18e1c55

unify security policy constants

a111108

rename securitypolicy to uapolicy

9635337

goimports

a30c66a

merge crypto and sign packages with uapolicy

19500d1

golangcibot reviewed Apr 18, 2019

View reviewed changes

uapolicy/crypto_hmac.go Outdated Show resolved Hide resolved

uapolicy/crypto_pkcs1v15.go Outdated Show resolved Hide resolved

uapolicy/crypto_pkcs1v15.go Outdated Show resolved Hide resolved

fix golangcibot issues

74519a0

golangcibot reviewed Apr 18, 2019

View reviewed changes

uapolicy/crypto_rsapss.go Outdated Show resolved Hide resolved

uapolicy/crypto_rsapss.go Outdated Show resolved Hide resolved

magiconair added 2 commits April 18, 2019 02:25

address golangcibot issues

6725d60

use constants for min padding

d8ecfe6

magiconair changed the title ~~WIP: Add crypto support to the client~~ Add crypto support to the client Apr 19, 2019

magiconair merged commit 0f3eb54 into master Apr 19, 2019

magiconair deleted the feature/crypto-client branch April 19, 2019 10:18

magiconair added this to the v0.1.0 milestone Jul 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add crypto support to the client #166

Add crypto support to the client #166

magiconair commented Apr 11, 2019 •

edited

Loading

dwhutchison commented Apr 12, 2019 •

edited

Loading

dwhutchison commented Apr 17, 2019

magiconair commented Apr 17, 2019

magiconair commented Apr 17, 2019

dwhutchison commented Apr 17, 2019

magiconair commented Apr 17, 2019

dwhutchison commented Apr 17, 2019

magiconair commented Apr 17, 2019

magiconair commented Apr 17, 2019

dwhutchison commented Apr 17, 2019

magiconair commented Apr 18, 2019

dwhutchison commented Apr 18, 2019

dwhutchison commented Apr 18, 2019

magiconair commented Apr 19, 2019

Add crypto support to the client #166

Add crypto support to the client #166

Conversation

magiconair commented Apr 11, 2019 • edited Loading

dwhutchison commented Apr 12, 2019 • edited Loading

dwhutchison commented Apr 17, 2019

magiconair commented Apr 17, 2019

magiconair commented Apr 17, 2019

dwhutchison commented Apr 17, 2019

magiconair commented Apr 17, 2019

dwhutchison commented Apr 17, 2019

magiconair commented Apr 17, 2019

magiconair commented Apr 17, 2019

dwhutchison commented Apr 17, 2019

magiconair commented Apr 18, 2019

dwhutchison commented Apr 18, 2019

dwhutchison commented Apr 18, 2019

magiconair commented Apr 19, 2019

magiconair commented Apr 11, 2019 •

edited

Loading

dwhutchison commented Apr 12, 2019 •

edited

Loading