search

FreeOpcUa / python-opcua

LGPL Pure Python OPC-UA Client and Server
http://freeopcua.github.io/
GNU Lesser General Public License v3.0
1.36k stars 658 forks source link

ExtraPaddingSize not implemented #544

Open mlgiraud opened 6 years ago

mlgiraud commented 6 years ago

The current implementation does not seem to support key sizes > 2048 bit. For keys > 2048 bit an extra padding byte is needed because the padding can be longer than 255 byte. Spec part 6 v1.03 Table 30 or Spec part 6 v1.04 Table 45

When creating padding this needs to be taken into account here and when removing padding here.

oroulet commented 6 years ago

Ok. If you know that kind of things, it would be great if you send a PR

mlgiraud commented 6 years ago

I will make a PR. I was waiting for someone to confirm this before i start changing stuff though.

oroulet commented 6 years ago

I wrote most of python-opcua code but I know nothing about encryption and code has been mainly written by @alkor. So he is probably the guy able to comment on this. Looks like very few people know cryptography, so any comments/PR are welcome

zerox1212 commented 6 years ago

If you know cryptography, please also check the public/private key situation. I think there is some problems there.

mlgiraud commented 6 years ago

Can you be a bit more specific than some problems, or are you just unsure if all is well?

I haven't run into any problems so far. I'm using the crypto modules for an OPC UA scapy plugin and everything seems to be working as intended, except for the padding situation.

zerox1212 commented 6 years ago

Last time I looked there was no public key. Both the server and and the client use the same private key for encryption. I don't know what the spec requires, but it doesn't seem like the best implementation as far as security goes.

There is some discussion here: https://github.com/FreeOpcUa/python-opcua/issues/507

oroulet commented 6 years ago

I kind of remember using same certificate for testing. But there should be no issue using 2 different certificates

oroulet commented 6 years ago

@Infinity95 interesting software you work on....

mlgiraud commented 6 years ago

The Security policies have the following parameters in their init method:

class SecurityPolicyBasic256(SecurityPolicy):
    def __init__(self, server_cert, client_cert, client_pk, mode):
        ....

The naming here is a bit misleading. IMHO it would be better to name it like this:

class SecurityPolicyBasic256(SecurityPolicy):
    def __init__(self, remote_cert, local_cert, local_pk, mode):
        ....

So when communicating with a remote (either as server or client, it doesn't matter) we need the remote certificate and our local certificate and the matching private key. The usage seems fine from the quick look i had.

oroulet commented 6 years ago

@Infinity95 i agree

zerox1212 commented 6 years ago

Since you seem knowledgeable about security. Can I ask why we do not need a pair of keys (public/private)? Instead we have a single private key and single certificate that is shared between all clients and servers.

Is the server sending these files to a client when they connect? Or must they already have the files in place?

mlgiraud commented 6 years ago

We have a pair of keys. The public key is contained in the certificates. In the asymmetric encryption case we use the public key in the certificate to encrypt the message (everybody can encrypt messages this way) and then the recipient can decrypt the message with the private key that belongs to the certificate. Since the private key is only known (or at least should be) to the recipient, he is the only one who can decrypt the message.

One can of course use the same private key on both sides but that defeats the purpose of asymmetric encryption and one could just as well use symmetric encryption.

I haven't looked at the examples yet, but the client and server (from looking at the source) can be loaded with a different private key and corresponding public key.

Concerning your edit: When the client wants to connect to a server we need to either know which endpoint (and the cert of the endpoint) we want to connect to or we could use the discovery service in order to get a list of endpoints and their certificates. So yes, the client needs to know the certificate before establishing an encrypted secure channel.

When using asymmetric encryption the sender certificate is sent along in the AsymmetricAlgorithmSecurityHeader, so that it can be used by the receiver (the server) to encrypt the messages when answering.

zerox1212 commented 6 years ago

Thanks for the detailed answer.

One can of course use the same private key on both sides but that defeats the purpose of asymmetric encryption and one could just as well use symmetric encryption.

So it could be that the implementation allows asymmetric encryption, but the example and the tests are essentially using symmetric encryption. I would really appreciate if you could show an example using asymmetric encryption. The current example only generates one key and one cert for both local and remote. I think this is where I am confused.

oroulet commented 6 years ago

Yes tests probably use one certificate to simplify things