Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

secsipid method to only validate signature without checking the rest of the header #3784

Open
whosgonna opened this issue Mar 11, 2024 · 10 comments

Comments

@whosgonna
Copy link
Contributor

Description

Currently secsipid has a method to sign arbitrary (json) data (secsipid_sign), however it has no converse method to check the signature. Currently, an attempt to check a div signature for example will yield a -303 error (SIPHdrInfo). Rather than trying to have full parsing for every possible type of Identity header (which are likely to increase in variety), it would be good to simply check "is this signature valid by trusted key", possibly validating the iat timestamp as well, but without any other opinions on the header values.

Expected behavior

A feature to check only the signature of an identity header.

Actual observed behavior

Currently the secsipid_check_ family of functions fails for non- shaken passport types.

Debugging Data

The following DIV identity header was generated by secsipid's secsipid_sign() function, so it should be possible to reverse this to validate the signature:

Identity: eyJhbGciOiJFUzI1NiIsInBwdCI6ImRpdiIsInR5cCI6InBhc3Nwb3J0IiwieDV1IjoiaHR0cHM6Ly9kLm10c2VjLm1lL2QzYTkvQmZUeGJVTlozS1FMLnBlbSJ9.eyJkZXN0Ijp7InRuIjpbIjE2MTI1NTU0MzIxIl19LCJpYXQiOiIxNzEwMTY5MzQ1Iiwib3JpZyI6eyJ0biI6IjE1NTU3MzU5MzA5In0sImRpdiI6eyJ0biI6IjE5NTI1NTU5ODc2In19.-0QF6-u6zgAQNoAhdiETuhAu7FuRDzxmFch_cTdhcbeWvUZ60NQXxdPM-JucpOtFaEdn9wnFreAZ_6vZoc_Phg;info=<https://d.mtsec.me/d3a9/BfTxbUNZ3KQL.pem>;alg=ES256;ppt=div

Possible Solutions

Because it's fairly straight forward to investigate the JWT, it's not necessary to try to account for every possible passport type, etc. The act of validating the signature is the complicated part, so a function that does only that would be convenient.

Additional Information

  • Kamailio Version - output of kamailio -v
version: kamailio 5.7.4 (x86_64/linux)
flags: USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, USE_RAW_SOCKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MMAP, PKG_MALLOC, MEM_JOIN_FREE, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLOCKLIST, HAVE_RESOLV_RES, TLS_PTHREAD_MUTEX_SHARED
ADAPTIVE_WAIT_LOOPS 1024, MAX_RECV_BUFFER_SIZE 262144, MAX_URI_SIZE 1024, BUF_SIZE 65535, DEFAULT PKG_SIZE 8MB
poll method support: poll, epoll_lt, epoll_et, sigio_rt, select.
id: unknown
compiled with gcc 12.2.0
  • Operating System:

Currently alpine linux 3.19 in a docker container, but it should be pretty reproducible everywhere.

@miconda
Copy link
Member

miconda commented Mar 11, 2024

Not sure I got exactly the kind of function you look for. One that still takes the Identity header value and verifies only the signature, with the key provided as parameter? Or the key has to be takes from the Identity header parameter? Maybe you can give the function prototype (or the list of parameters you want to provide to the function).

On the hand hand, can you look at the jwt module and see if it can already help with what you need?

@whosgonna
Copy link
Contributor Author

I'll give the JWT module a peek. Lack of caching is maybe an issue (but can be 'farmed out' to something else for caching purposes).

Perhaps this would be better considered as an error with the existing secsipid_check() function in that it will only validate shaken passport types, and the ask should be simply to eliminate this check.

secsipid_check(sIdentity, keyPath)

Check the validity of the "sIdentity" parameter using the keys stored in the file specified by "keyPath". If the keyPath parameter is empty, the function is downloading the key using the URL from "info" parameter of the sIdentity, using the value of "timeout" parameter to limit the download time. The validity of the JWT in the sIdentity value is also checked against the "expire" parameter.

The function notes, "Further checks can be done with config operations, decoding the JWT header and payload using {s.select} and {s.decode.base64t} transformations together with jansson module.", which is a very clean waay to handle this, and the function here should just be less opinionated on what is and isn't a valid Identity header?

@miconda
Copy link
Member

miconda commented Mar 14, 2024

I added the function secsipid_verify(...) that should allow disabling the checks on the jwt header attributes. I haven't tested though, report if you get any issues. You have to use the latest git versions of both kamailio and libsecsipid.

@whosgonna
Copy link
Contributor Author

whosgonna commented Mar 20, 2024

Sorry for the delayed reply - I had a few small issues compiling, but kamailio then fails to start with secsipid_verify() not found:

During startup:

 0(1) ERROR: <core> [core/cfg.y:3870]: yyparse(): cfg. parser: failed to find command secsipid_verify (params 2)

The function is called like this:

secsipid_verify("$var(identity)", "")

Versions:

[ben@NV0162~/projects/cnam_relay]$ dc exec cnam-sti-vs kamailio -v
version: kamailio 5.9.0-dev0 (x86_64/linux) 951ab1
flags: USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, USE_RAW_SOCKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MMAP, PKG_MALLOC, MEM_JOIN_FREE, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLOCKLIST, HAVE_RESOLV_RES, TLS_PTHREAD_MUTEX_SHARED
ADAPTIVE_WAIT_LOOPS 1024, MAX_RECV_BUFFER_SIZE 262144, MAX_SEND_BUFFER_SIZE 262144, MAX_URI_SIZE 1024, BUF_SIZE 65535, DEFAULT PKG_SIZE 8MB
poll method support: poll, epoll_lt, epoll_et, sigio_rt, select.
id: 951ab1
compiled on 21:42:15 Mar 19 2024 with gcc 12.2.0

[ben@NV0162~/projects/cnam_relay]$ dc exec cnam-sti-vs secsipidx -version
secsipidx v1.3.2

This is my Dockerfile in case I'm missing something in compilation:

FROM golang:1.22.1-bookworm AS secsipidbuilder
ENV GO111MODULE=off
RUN    cd / \
    && git clone https://github.com/asipto/secsipidx.git \
    && cd secsipidx \
    && make \
    && make install \
    && cd / \
    && apt update \
    && apt upgrade -y \
    && apt install -y git make automake autoconf libtool libcurl4-openssl-dev \
                      sngrep gnupg2 wget lsb-release openssl libssl-dev \
                      pkg-config uuid-dev sip-tester \
    && apt install -y pkg-config gcc bison flex g++ libssl-dev libxml2-dev \
                      libjson-c-dev libpcre3 libjansson-dev libpcre3-dev  \
                      libhiredis-dev libsqlite3-dev libpq-dev libevent-dev \
                      sqlite3 uuid-dev \
    && cd /secsipidx \
    && make install \
    && git clone \
            -b master \
            --single-branch https://github.com/kamailio/kamailio.git /kamailio \
    && cd /kamailio \
    && make include_modules="jansson json ndb_redis db_sqlite db_postgres \
                             secsipid secsipid_proc http_async_client avpops \
                             uuid" prefix="/" cfg \
    && make all \
    && make install \
    && apt clean \
    && apt-get autoremove --yes \
    && cd / \
    && rm -rf /var/lib/{apt,dpkg,cache,log}/ \
    && rm -rf /kamailio \
    && rm -rf /secsipidx

COPY etc/kamailio /etc/kamailio

@whosgonna
Copy link
Contributor Author

Ah - I see the function takes three arguments. The third argument only has "A" as an allowed value?

@whosgonna
Copy link
Contributor Author

Is it possible to use the same logic for downloading (and caching) of the key as the secsipid_check function:

If the keyPath parameter is empty, the function is downloading the key using the URL from "info" parameter of the sIdentity, using the value of "timeout" parameter to limit the download time

@miconda
Copy link
Member

miconda commented Mar 20, 2024

This function is with key value as parameter, not file path. There is also a function in the secsipid module to download:

@whosgonna
Copy link
Contributor Author

Confirmed that this is working. Will it get ported to 5.8 or will it be the next major.minor release?

Copy link

github-actions bot commented May 9, 2024

This issue is stale because it has been open 6 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.

@github-actions github-actions bot added the Stale label May 9, 2024
@whosgonna
Copy link
Contributor Author

Issue auto marked marked as "stale", but the provided fix works.

@github-actions github-actions bot removed the Stale label May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants