Attention is currently required from: dexter, laforge.

Hoernchen has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/pysim/+/40464?usp=email )

Change subject: smdpp: add proper tls support, cert generation FOR TESTING ......................................................................

Patch Set 6:

(7 comments)

File contrib/generate_smdpp_certs.py:

https://gerrit.osmocom.org/c/pysim/+/40464/comment/339f62a9_c8663cc1?usp=ema... : PS5, Line 2:

To me this looks like an independent tool. […]

no, this is the tool and these are the options, these things belong together and make absolutely no sense as single commits with random mystery options and useless certs.

File osmo-smdpp.py:

https://gerrit.osmocom.org/c/pysim/+/40464/comment/4382ca7d_ea805b69?usp=ema... : PS5, Line 26: from pathlib import Path

You have moved a lot of those imports from another code location. […]

Done

https://gerrit.osmocom.org/c/pysim/+/40464/comment/bc8c1331_3407f1b1?usp=ema... : PS5, Line 53:

We now support SSL and we have means to point to different certificate sub directories. […]

not related to this patch at all, we cann add stuff if we need it later.

https://gerrit.osmocom.org/c/pysim/+/40464/comment/6ee98918_c7b38b8d?usp=ema... : PS5, Line 586: parser.add_argument("-p", "--port", help="TCP port to bind HTTP to", default=8000)

Maybe 8443, since we now support SSL?

no, because that is being used by something else on my system, we really do not want to default to a port commonly used for anything like https for a toy smdpp.

https://gerrit.osmocom.org/c/pysim/+/40464/comment/3817ef0e_da2c40d3?usp=ema... : PS5, Line 587: parser.add_argument("-c", "--certpath", help=f"cert subdir relative to {DATA_DIR}", default="certs")

I think --certpath is a bit irritating as one may thing that it is possible to provide an absolute p […]

Done .. the help line literally explains that it is relative

https://gerrit.osmocom.org/c/pysim/+/40464/comment/a7b3dcc9_fe1f3390?usp=ema... : PS5, Line 588: parser.add_argument("-s", "--nossl", help="do NOT use ssl", action='store_true', default=False)

I wonder if this may break anything when osmo-smdpp suddenly supports SSL by default. […]

The number of users is probably... low.. anyway.

https://gerrit.osmocom.org/c/pysim/+/40464/comment/f5aa1340_7306aee3?usp=ema... : PS5, Line 591: common_cert_path = os.path.join(DATA_DIR, args.certpath)

(see above) Maybe having DATA_DIR as a commandline parameter would be a good idea.

I dont't need it, and it is not related to this patch either.