irods / irods_capability_automated_ingest

Other
12 stars 15 forks source link

ssl_context in PRC connections should be more automatic #218

Open alanking opened 4 months ago

alanking commented 4 months ago

PRC is now able to establish an SSL context based on the user's client environment, as demonstrated in the README: https://github.com/irods/python-irodsclient/?tab=readme-ov-file#establishing-a-secure-connection

The ingest tool attempts to create an SSL context based on information in the client environment, even though not everything being used is required to establish the ssl_context. This can lead to problems connecting to the server when SSL is enabled:

https://github.com/irods/irods_capability_automated_ingest/blob/94b0efb478d922b5e1e65144d269904f795834a7/irods_capability_automated_ingest/sync_irods.py#L389-L400

alanking commented 2 months ago

Changing how the iRODSSession is instantiated will affect the as_user option. This works by adding a client_user and client_zone to the keyword args passed to the iRODSSession. Unless I'm missing something, I don't think these can be derived from the client environment file.

If there's no way to do this, we may have to just expand all the options set in the ssl_context rather than leaning on the environment file.

Reference: https://github.com/irods/irods_capability_automated_ingest/blob/9ff917c0d7e2f2a35a8539a8bdedd0790a238a56/irods_capability_automated_ingest/sync_irods.py#L369-L372

korydraughn commented 2 months ago

I don't think these can be derived from the client environment file.

You're saying client_user and client_zone cannot be derived because the env file doesn't contain properties by those names, correct?

If there's no way to do this, we may have to just expand all the options set in the ssl_context rather than leaning on the environment file.

You're saying expose new TLS options for ingest that are passed to the PRC, correct?

alanking commented 2 months ago

You're saying client_user and client_zone cannot be derived because the env file doesn't contain properties by those names, correct?

Correct.

You're saying expose new TLS options for ingest that are passed to the PRC, correct?

That's not what I had in mind, but it is a possibility. I was thinking of deriving the SSL options from the client environment file and "manually" configuring the ssl_context using those values. This is what we are doing now, but I think the current implementation is incomplete.

Now that I'm reading it again, it may be possible to do both.

korydraughn commented 2 months ago

Yeah, that sounds good. We can start with your original plan and then expand it if needed. I was just trying to make sure I was understanding the approach.